Review (NS-143): FR1116: Global Crawler Traps

Author	Colin
Moderator	Colin
State	Closed

Objectives

A component to allow lists of crawler traps to be appended to all scheduled jobs. There are
three sub-components
i. Backend: datamodel + DAO
ii. Business logic: (small) modifications to class Job which actually add the global crawler traps to all new Jobs
iii. GUI: a single page with corresponding backend for carrying out all the necessary operations for managing lists of global crawler traps  - create, read, update, delete and activate/deactivate.

General comments:

Description	Classification	Status
Append the english versions of the new properties to the other language files (Translations_de.properties, Translations_fr.properties, Translations_it.properties)	Cosmetic	OK
Remove @author and @since tags in all the code-files. We don't use these tags in NetarchiveSuite	Cosmetic	OK

Time Used svc: 1md csr: 12md

Cosmetic

NOTOK

Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapActionEnum.java', revision 1226

Lines	Description	Classification	Status
General	Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id)	Cosmetic	OK
39-64	Add javadoc before each enum value	Cosmetic	OK

Comments on file 'trunk/webpages/HarvestDefinition/traplist-download-element.jspf', revision 1239

Lines	Description	Classification	Status
General	Add NetarchiveSuite header-file	Cosmetic	OK
24	long line; make newline before 'value=..'	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/GlobalCrawlerTrapListDAO.java', revision 1226

Lines	Description	Classification	Status
General	Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id)	Cosmetic	OK
45	Complete the @return statement	Cosmetic	OK
51	Complete the @return statement	Cosmetic	OK
58	Complete the @return statement	Cosmetic	OK
75-80	Missing javadoc for these methods	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapCreateOrUpdateAction.java', revision 1227

Lines	Description	Classification	Status
General	Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id)	Cosmetic	OK
108-111, 113-114, 117-125	Shouldn't line 108-125 be put into a try .. catch so that any database errors are caught?	Cosmetic	OK
115	This could be written more concisely like: if (fileName != null && !fileName.isEmpty())	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/Job.java', revision 1226

Lines	Description	Classification	Status
394	Missing javadoc for this method	Cosmetic	OK
401	Remove redundant line	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/GlobalCrawlerTrapList.java', revision 1226

Lines	Description	Classification	Status
General	Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id)	Cosmetic	OK
56	say which table it is unique in.	Cosmetic	OK
58-60	Missing javadoc	Cosmetic	OK
62-64	Missing javadoc Missing argument validation (not null and not empty string)	Cosmetic	OK
67	Missing '.' at end of line.	NA	OK
69	say which table it is unique in.	Cosmetic	OK
132-133	Missing '.' at end of line.	Cosmetic	OK
136	Missing argument validation of argument 'is'	Cosmetic	OK
149-151	Missing javadoc	Cosmetic	OK
153	Missing javadoc	NA	OK
157	Missing javadoc	NA	OK
161	Missing javadoc Missing argument validation	Cosmetic	OK
165	Missing javadoc	Cosmetic	OK
169	Missing javadoc Missing argument validation	Cosmetic	OK
173	Missing javadoc	Cosmetic	OK
177-179	Missing javadoc	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapReadAction.java', revision 1227

Lines	Description	Classification	Status
General	Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id)	Cosmetic	OK
42-43	Remove @author and @since tags. We don't use these tags in NetarchiveSuite	Cosmetic	OK
58	Breaks checkstyle rule: move "+" at end of line to next line.	Cosmetic	OK
64	Insert whitespace around ":"	Cosmetic	OK
65	replace (trap+"\n") with (trap + "\n")	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/MySQLSpecifics.java', revision 1238

Lines	Description	Classification	Status
General	Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions()	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/GlobalCrawlerTrapListDBDAO.java', revision 1226

Lines	Description	Classification	Status
General	Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id)	Cosmetic	OK
110	Missing javadoc	Cosmetic	OK
113	Missing javadoc; add new line before method	Cosmetic	OK
127	Move "+" to next line	Cosmetic	OK
129	Is it really necessary to log here? The exception is rethrown just after the logentry	Cosmetic	OK
144	Add missing javadoc	Cosmetic	OK
149	add newline	Cosmetic	OK
168	Add missing javadoc Remove long line	Cosmetic	OK
214-219	Add javadoc	Cosmetic	OK
226	Add comment that here we delete all entries from global_crawler_trap_lists where ....	Cosmetic	OK
229	Add comment that here we delete all entries from global_crawler_trap_expressions where ....	Cosmetic	OK
244	Add javadoc	Cosmetic	OK
249	Add missing argument validation	Cosmetic	OK
254	Add comment to what is done here	Cosmetic	OK
260	Add comment to what is done here	Cosmetic	OK
263	Add comment to what is done here	Cosmetic	OK
271	Move "+" to next line	Cosmetic	OK
281-286	Add javadoc	Cosmetic	OK
296	Move "+" to next line	Cosmetic	OK
312	Move "+" to next line	Cosmetic	OK

Comments on file 'trunk/webpages/HarvestDefinition/traplist_createorupdate_element.jspf', revision 1239

Lines	Description	Classification	Status
General	Add NetarchiveSuite fileheader	Cosmetic	OK

Comments on file 'trunk/webpages/HarvestDefinition/traplist_activation_element.jspf', revision 1238

Lines	Description	Classification	Status
General	Add NetarchiveSuite fileheader	Cosmetic	OK

Comments on file 'trunk/webpages/HarvestDefinition/Definitions-edit-global-crawler-traps.jsp', revision 1239

Lines	Description	Classification	Status
General	Missing standard NetarchiveSuite header-file	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/DerbySpecifics.java', revision 1233

Lines	Description	Classification	Status
General	Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions()	Cosmetic	OK
320	Is the case not wrong here?	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapAction.java', revision 1226

Lines	Description	Classification	Status
General	Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id)	Cosmetic	OK
70, 77	Replace "".equals(requestType) with requestType.isEmpty()	Cosmetic	OK

Comments on file 'trunk/webpages/HarvestDefinition/traplist_delete_element.jspf', revision 1238

Lines	Description	Classification	Status
General	Add NetarchiveSuite fileheader	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapActivationAction.java', revision 1227

Lines	Description	Classification	Status
General	Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id)	Cosmetic	OK
57	Shouldn't we put some of this code inside a try..catch, so that we can intercept any database errors?	Cosmetic	OK

Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/Constants.java', revision 1227

Lines	Description	Classification	Status
86-87	add missing "."	Cosmetic	OK
89-99	Consider adding separate javadoc for each constant	Cosmetic	Rejected

Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/TrapDeleteAction.java', revision 1209

Lines	Description	Classification	Status
General	Wrong revision shown; add SVN property svn:keywords with value (URL Revision Author Date Id)	Cosmetic	OK
44-45	Put into a try-catch, intercept any SQL-exceptions, and forward an error the user	Cosmetic	OK