[
https://issues.apache.org/jira/browse/QPID-5138?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13778913#comment-13778913
]
Keith Wall commented on QPID-5138:
----------------------------------
Hi Alex, This is my review up to r1526182.
PreferencesProvider#setPreferences/FileSystemPreferencesProvider#setPreferences
javadoc on interface disagrees with impl (specifically javadoc says old prefs
are returned)
FileSystemPreferenceProvider#setState - exception text refers to authentication
provider
FileSystemPreferenceProvider#setState - what if File.delete() fails?
FileSystemPreferenceProvider#DEFAULTS - under what circumstances is the default
path used ~/.qpid/preferences.json
FileSystemPreferenceProvider#setState - how will the user recover if the lock
file is not removed if owing to JVM failure? Maybe exception message should say
words to the effect of (if you are sure there is no other instance of Qpid
running resolve this issue by deleting...)
FileSystemPreferenceProvider - file locking - why the two levels of locking?
FileSystemPreferenceProvider - file locking - adopt better naming for the two
locks better (perhaps saveLock and overallPrefLock)
FileSystemPreferenceProvider - spelling - a(c)quire.
BrokerRecoverer#BrokerRecoverer - formal parameter named incorrectly
preferencesProviderFactory => preferencesProviderCreator
AuthenticationProviderRecoverer#recoverType - why the different recovery
approach to that taken elsewhere in the Broker?
PreferencesProviderCreator - no licence
PreferencesProviderCreator - why have we introduced a creator concept?
PreferencesProviderCreator#createPreferencesProvider - wrong text in exception
message
PreferencesProviderCreator#create - duplicates recover
PreferencesProviderCreator#createPreferencesProvider - implementation always
uses first factory
PreferencesProviderFactory - no licence
PreferencesProviderRecoverer - no licence
PreferencesProviderRecoverer#create - avoid boiler plate by extracting
RecovererHelper.verifyOnlyParentIsA(AuthenticationProvider.class,
ConfiguredObject... parents)
MemoryConfigurationEntryStore#toEntry - comment and/or extract method to
explain what is going on.
MemoryConfigurationEntryStoreTest - needs extended to cover the new functionaly
of MemoryConfigurationEntryStore#toEntry.
PreferencesServlet/UserPreferencesServlet - these names are very close. Can we
name them to better allude to their functions?
PreferencesServlet#doPostWithSubjectAndActor - inconsistent error handing. the
method throws ISE for pp == null whereas doGet, getPut send NOT_FOUND
UserPreferencesServlet#getUserPreferences - typo View(e)ing
UserPreferencesServlet#userPreferencesOperationAuthorized - surely we don't
want the servlet to be making authorising decisions? This should be pushed down.
UserPreferencesServlet#doGetWithSubjectAndActor - copy/pasted 'path' code.
UserPreferencesServlet#doDeleteWithSubjectAndActor - in the case where we are
deleting many, if authorisation check fails part way through we will have
deleted some of the users and returned a 403. I think PreferencesProvider
should probably take a list.
typo in html id: delete(Pre)eferencesButton
Other observations:
UI - Think the logout link looks odd next to the cog button. Perhaps logout
should be moved into the cog menu, or the link changed to a button?
UI - We should support UTC. Currently if you select no timezone I would expect
UTC, I see BST (is this based on my browser's locale)?
UI - City names should be sorted alphabetically
Out of the box, the default preference file created is called "passwordFile",
which seems odd. I think this will confuse users who will think it contains
passwords.
Should the preference file have a version or model number embedded within?
Defect:
If I make an update that causes the preference file to be made shorter,
trailing garbage is left in the file.
> [Java Broker] Implement web management console preferences
> ----------------------------------------------------------
>
> Key: QPID-5138
> URL: https://issues.apache.org/jira/browse/QPID-5138
> Project: Qpid
> Issue Type: New Feature
> Components: Java Broker
> Reporter: Alex Rudyy
> Assignee: Alex Rudyy
> Fix For: 0.25
>
>
> Add ability to set user preferences in web management console as specified
> [here|http://cwiki.apache.org/confluence/display/qpid/Java+Broker+Design+-+Preferences]
--
This message is automatically generated by JIRA.
If you think it was sent incorrectly, please contact your JIRA administrators
For more information on JIRA, see: http://www.atlassian.com/software/jira
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
