Review (NS-37): Settings additional with focus on Channels

Author	Eld Zierau
Moderator	Eld Zierau
State	Closed

Objectives

Include channel part of review NS-33.
This main review is mainly concerns changes and clean up of setting, and implementing: Assignment B.2.2a0 - Separate the meaning of replica and physical location. All files not mentioned in the below lists only concerns:
- Move of comments from settings into javadoc
- Refactoring (replica instead of location, Id instead of name)
Channels has new basis in form of applicationId instead of http port and new meaning of replica, the main changes are made in the following files:
/trunk/src/dk/netarkivet/common/Constants.java Move language settings from constants to settings.
/trunk/src/dk/netarkivet/common/CommonSettings.java all (include JMX, web interface language)
/trunk/src/dk/netarkivet/common/webinterface/HTMLUtils.java line 205-220 language
/trunk/src/dk/netarkivet/common/distribute/Channels.java all (see diff 447 - 727)
/trunk/src/dk/netarkivet/common/distribute/ChannelID.java  all  (see diff 447 - 727)
/trunk/src/dk/netarkivet/common/distribute/arcrepository/Replica.java  all (see diff 526 - 626)
/trunk/src/dk/netarkivet/common/distribute/arcrepository/ReplicaType.java all new
/trunk/src/dk/netarkivet/common/settings.xml all - new replica settings
/trunk/conf/settings_example.xml all

Total Time Used (Coding,Documentation,Review):

Time use (Coding,Documentation,Review)
ELZI: 10 MD
SVC: 5 MD
Earlier reviews concerning the Settings rewrite is included in these numbers

General comments:

Description

Classification

Status

Comments on file 'trunk/src/dk/netarkivet/common/webinterface/HTMLUtils.java', revision 637

Lines	Description	Classification	Status
General	Badly in need of argument checking of public methods. Some methods will throw NullPointerExceptions of null arguments	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/common/distribute/Channels.java', revision 727

Lines	Description	Classification	Status
General	The channel-prefixes ALL_BA, ANY_BA, THE_BAMON, THE_SCHED, ANY_LOWPRIORITY_HACO ANY_HIGHPRIORITY_HACO, THIS_REPOS_CLIENT, THE_REPOS, ERROR, INDEX_SERVER, THIS_INDEX_CLIENT, MONITOR should all be made as final static String constants	Cosmetic	OK
74	Missing javadoc for the Channels constructor	Cosmetic	OK
98-99	Remove this comment, or add TODO before "Sort in .."	Cosmetic	OK

Comments on file 'trunk/conf/settings_example.xml', revision 626

Lines	Description	Classification	Status
General	Run this file through deploy validation	Cosmetic	OK
527	Comment "applicationsInstanceId" here	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/common/distribute/arcrepository/Replica.java', revision 591

Lines	Description	Classification	Status
57	Consider renaming "known" as "knownReplicas"	Cosmetic	OK
105	Null value for parameter id seems to be allowed; Mention in the javadoc, that null id is allowed, or add a CheckNotNull for id parameter Add to javadoc: throws UnknownId if null id or unknown replica Write to webarkivering-teknik: In need of policy for whether or not add throws of unchecked exception to the method signatures	Cosmetic	OK
214	The type that => The type of	Cosmetic	OK
222	Language: "The id that this replica" => "the id of this replica"	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/common/distribute/ChannelID.java', revision 727

Lines	Description	Classification	Status
124	Have a constant for the "_" string.	Cosmetic	OK
131	id.equals("") => id.isEmpty()	Cosmetic	OK
244	Code style violation: i+1 => i + 1	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/common/Constants.java', revision 637

Lines

Description

Classification

Status

Comments on file 'trunk/src/dk/netarkivet/common/distribute/arcrepository/ReplicaType.java', revision 592

Lines	Description	Classification	Status
10, 14, 18	Add missing dot	Cosmetic	OK
31	The integer rt will never be null. Maybe replace with a checkNotNegative instead; or remove	Cosmetic	OK
51, 54	Replace the strings with String constants: perhaps BITARCHIVE_REPLICATYPE = "bitarchive" and CHECKSUM_REPLICATYPE = "checksum"	Cosmetic	OK