= Review (NS-145): Assignment B.2.2b: Make bitarchive replica use new messages. =
|| Author || Jonas ||
|| Moderator || Jonas ||
|| State || Review ||
== Objectives ==
{{{
See FR 1823.
}}}
'''Total Time Used (Coding,Documentation,Review)''':

{{{
Time use (Coding,Documentation,Review)
SVC: 1.0
JOLF: 5.0
}}}
'''General comments''':
|| '''Description''' || '''Classification''' || '''Status''' ||
|| Use different termilogy: wrong/bad => corrupt || Cosmetic || OK ||


=== Comments on file 'trunk/src/dk/netarkivet/archive/checksum/distribute/ChecksumFileServer.java', revision 1246 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 263 || Should we validate the badFile before sending it back? || Cosmetic || REJECT ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/distribute/JMSArcRepositoryClient.java', revision 1246 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 684 || Replace 'wrong' with 'corrupt' || Cosmetic || OK ||
|| 717, 721 || This should be 'removedFile' instead of retrieving the variable again. || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/checksum/distribute/CorrectMessage.java', revision 1246 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 79 || "String checksum" => "String badChecksum" || Cosmetic || OK ||
|| 173-176 || Issue warning, and return null. || Minor || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/checksum/FileChecksumArchive.java', revision 1246 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 723-724 || Language: Change to "Make the file containing the corrupt entry be returned in the CorrectMessage || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/bitpreservation/DatabaseBasedActiveBitPreservation.java', revision 1254 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 85 || Remove. This will not be implemented. || Cosmetic || OK ||
|| 138-139 || Split up into multiple commands || Cosmetic || OK ||
|| 159-160 || Split up into multiple commands || Cosmetic || OK ||
|| 446 || Describe, when this will be implemented || Cosmetic || OK ||
|| 469 || Describe, when this will be implemented || Cosmetic || OK ||
|| 515, 519 || Does not throw an UnknownID (or IOFailure? - perhaps indirectly) || Cosmetic || OK ||
|| 601 || Add when this will be implemented || Cosmetic || OK ||
|| 641 || typo:  teh => the || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/ArcRepository.java', revision 1242 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 358-359 || Change to "Checksum job message submitted for file '" + filename + "' with message id '" + msg.getID + "' || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/checksum/ChecksumArchive.java', revision 1246 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 72 || Does "incorrectChecksum" refer to an undefined variable? || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/RemoveAndGetFileMessage.java', revision 1246 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 60-61 || What prevents you from removing this constructor, now? || Cosmetic || OK ||
|| 98 || Typo: "fopr .." => "for the file" || Cosmetic || OK ||
|| 101-102 || Add missing argument-check || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/checksum/distribute/ChecksumClient.java', revision 1247 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 66 || change "theCR" to "theChecksumChannel" or "theChecksumQueue" || Cosmetic || OK ||
|| 192 || What is this TODO about? I don't understand it || Cosmetic || OK ||
|| 192 || Remove! The todo is fixed || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/BitarchiveMonitorServer.java', revision 1251 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 107, 110, 116 || "First" => "In the first stage" "Second" => In the second stage" "Third" => "In the third stage" || Cosmetic || OK ||
|| 112 || an UploadMessage || Cosmetic || OK ||
|| 240-251 || This description here is redundant || Cosmetic || OK ||
|| 258 || Typo: Recieving =>  Receiving || Cosmetic || OK ||
|| 258, 298-299 || Receiving || Cosmetic || OK ||
|| 280-281 || Redundant description || Cosmetic || OK ||
|| 289-291 || Redundant description || Cosmetic || OK ||
|| 308 || Typo: filed => failed || Cosmetic || OK ||
|| 333-340 || redundant description || Cosmetic || OK ||
|| 368, 371 || Finish the two sentences. || NA || OK ||
|| 373 || argument validation || Cosmetic || OK ||
|| 389 || argument validation + javadoc || Cosmetic || OK ||
|| 405 || argument validation + javadoc. || Cosmetic || OK ||
|| 444 || Trouble while handling batch request => Unable to handle batch request due to unexpected exception || Cosmetic || OK ||
|| 561 || Which other exceptions are foreseen here besides the IOFailure thrown in case of unhandled message type?   The called method handle their own exceptions, so none. The 'try - catch' is redundant if the reply is put into the last 'else' clause. || Cosmetic || OK ||
|| 640 || Change to: Fetch the contents of the batchresultfile || Cosmetic || OK ||
|| 657 || Include the others in the log entry || Cosmetic || OK ||
|| 664 || explain what valid means here || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/bitpreservation/FileBasedActiveBitPreservation.java', revision 1254 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 296 || This method is redundant: the method only forwards to the getChecksum method || Cosmetic || OK ||
|| 347 || Wrong spelling of 'successful' || Cosmetic || OK ||
|| 363 || Wrong checkNotNull message || Cosmetic || OK ||
|| 402 || Replace log message with "Finding missing files in directory '" || Cosmetic || OK ||
|| 413 || Add to comment what difference set 1 is || Cosmetic || OK ||
|| 431 || Add to comment what difference set 2 is || Cosmetic || OK ||
|| 449 || Replace log message with "Finished finding missing files" || Cosmetic || OK ||
|| 472-474 || Make an external variable for value  of "ArcRepositoryClientFactory.getPreservationInstance().getAllFilenames(replica.getId))" so this value can be checked before calling the copyFile method || Cosmetic || OK ||
|| 478 || Don't use the word 'wrong'. Use the word 'corrupt' instead. || Cosmetic || OK ||
|| 619-620 || Make an external variable for value  of "ArcRepositoryClientFactory.getPreservationInstance().getAllChecksums(replica.getId))" so this value can be checked before calling the copyFile method || Cosmetic || OK ||
|| 718-720 || Please change to: Check that the files we want to restore are indeed missing on the replica on which we are restoring them, and presnt in admin data and the reference replica. If so, upload the missing files from the reference replica to this replica. || Cosmetic || OK ||
|| 873 || Add that: We don't wait for an acknowledgement that admin data has indeed been updated. || Cosmetic || OK ||
|| 916 || change "a bad entry" to "an entry" || Cosmetic || OK ||
|| 962 || When will this be implemented? || Cosmetic || OK ||
|| 975 || When will this be implemented? || Cosmetic || OK ||
|| 988 || Change to "String... filenames" || Cosmetic || OK ||
|| 990 || When will this be implemented? || Cosmetic || OK ||
|| 1023 || When will this be implemented? || Cosmetic || OK ||
=== Comments on file 'trunk/src/dk/netarkivet/archive/bitarchive/distribute/BitarchiveClient.java', revision 1247 ===
|| '''Lines''' || '''Description''' || '''Classification''' || '''Status''' ||
|| 229-230 || Remove this @throws clause || Cosmetic || OK ||
|| 243 || why was this line added? || Cosmetic || OK ||
|| 247 || Typo: be send => be sent || Cosmetic || OK ||
|| 286-288 || Document that a Argument not valid is thrown || Cosmetic || OK ||
|| 313 || remove || Cosmetic || OK ||