NetarchiveSuite
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 |
NOTOK |
Remove @author and @since tags in all the code-files. We don't use these tags in NetarchiveSuite |
Cosmetic |
NOTOK |
|| 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 |
NOTOK |
39-64 |
Add javadoc before each enum value |
Cosmetic |
NOTOK |
Comments on file 'trunk/webpages/HarvestDefinition/traplist-download-element.jspf', revision 1239
Lines |
Description |
Classification |
Status |
General |
Add NetarchiveSuite header-file |
Cosmetic |
NOTOK |
24 |
long line; make newline before 'value=..' |
Cosmetic |
NOTOK |
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 |
NOTOK |
45 |
Complete the @return statement |
Cosmetic |
NOTOK |
51 |
Complete the @return statement |
Cosmetic |
NOTOK |
58 |
Complete the @return statement |
Cosmetic |
NOTOK |
75-80 |
Missing javadoc for these methods |
Cosmetic |
NOTOK |
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 |
NOTOK |
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 |
NOTOK |
115 |
This could be written more concisely like: if (fileName != null && !fileName.isEmpty()) |
Cosmetic |
NOTOK |
Comments on file 'trunk/src/dk/netarkivet/harvester/datamodel/Job.java', revision 1226
Lines |
Description |
Classification |
Status |
394 |
Missing javadoc for this method |
Cosmetic |
NOTOK |
401 |
Remove redundant line |
Cosmetic |
NOTOK |
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 |
NOTOK |
56 |
say which table it is unique in. |
Cosmetic |
NOTOK |
58-60 |
Missing javadoc |
Cosmetic |
NOTOK |
62-64 |
Missing javadoc Missing argument validation (not null and not empty string) |
Cosmetic |
NOTOK |
67 |
Missing '.' at end of line. |
NA |
NOTOK |
69 |
say which table it is unique in. |
Cosmetic |
NOTOK |
132-133 |
Missing '.' at end of line. |
Cosmetic |
NOTOK |
136 |
Missing argument validation of argument 'is' |
Cosmetic |
NOTOK |
149-151 |
Missing javadoc |
Cosmetic |
NOTOK |
153 |
Missing javadoc |
NA |
NOTOK |
157 |
Missing javadoc |
NA |
NOTOK |
161 |
Missing javadoc Missing argument validation |
Cosmetic |
NOTOK |
165 |
Missing javadoc |
Cosmetic |
NOTOK |
169 |
Missing javadoc Missing argument validation |
Cosmetic |
NOTOK |
173 |
Missing javadoc |
Cosmetic |
NOTOK |
177-179 |
Missing javadoc |
Cosmetic |
NOTOK |
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 |
NOTOK |
42-43 |
Remove @author and @since tags. We don't use these tags in NetarchiveSuite |
Cosmetic |
NOTOK |
58 |
Breaks checkstyle rule: move "+" at end of line to next line. |
Cosmetic |
NOTOK |
64 |
Insert whitespace around ":" |
Cosmetic |
NOTOK |
65 |
replace (trap+"\n") with (trap + "\n") |
Cosmetic |
NOTOK |
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 |
NOTOK |
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 |
NOTOK |
110 |
Missing javadoc |
Cosmetic |
NOTOK |
113 |
Missing javadoc; add new line before method |
Cosmetic |
NOTOK |
127 |
Move "+" to next line |
Cosmetic |
NOTOK |
129 |
Is it really necessary to log here? The exception is rethrown just after the logentry |
Cosmetic |
NOTOK |
144 |
Add missing javadoc |
Cosmetic |
NOTOK |
149 |
add newline |
Cosmetic |
NOTOK |
168 |
Add missing javadoc Remove long line |
Cosmetic |
NOTOK |
214-219 |
Add javadoc |
Cosmetic |
NOTOK |
226 |
Add comment that here we delete all entries from global_crawler_trap_lists where .... |
Cosmetic |
NOTOK |
229 |
Add comment that here we delete all entries from global_crawler_trap_expressions where .... |
Cosmetic |
NOTOK |
244 |
Add javadoc |
Cosmetic |
NOTOK |
249 |
Add missing argument validation |
Cosmetic |
NOTOK |
254 |
Add comment to what is done here |
Cosmetic |
NOTOK |
260 |
Add comment to what is done here |
Cosmetic |
NOTOK |
263 |
Add comment to what is done here |
Cosmetic |
NOTOK |
271 |
Move "+" to next line |
Cosmetic |
NOTOK |
281-286 |
Add javadoc |
Cosmetic |
NOTOK |
296 |
Move "+" to next line |
Cosmetic |
NOTOK |
312 |
Move "+" to next line |
Cosmetic |
NOTOK |
Comments on file 'trunk/webpages/HarvestDefinition/traplist_createorupdate_element.jspf', revision 1239
Lines |
Description |
Classification |
Status |
General |
Add NetarchiveSuite fileheader |
Cosmetic |
NOTOK |
Comments on file 'trunk/webpages/HarvestDefinition/traplist_activation_element.jspf', revision 1238
Lines |
Description |
Classification |
Status |
General |
Add NetarchiveSuite fileheader |
Cosmetic |
NOTOK |
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 |
NOTOK |
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 |
NOTOK |
320 |
Is the case not wrong here? |
Cosmetic |
NOTOK |
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 |
NOTOK |
70, 77 |
Replace "".equals(requestType) with requestType.isEmpty() |
Cosmetic |
NOTOK |
Comments on file 'trunk/webpages/HarvestDefinition/traplist_delete_element.jspf', revision 1238
Lines |
Description |
Classification |
Status |
General |
Add NetarchiveSuite fileheader |
Cosmetic |
NOTOK |
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 |
NOTOK |
57 |
Shouldn't we put some of this code inside a try..catch, so that we can intercept any database errors? |
Cosmetic |
NOTOK |
Comments on file 'trunk/src/dk/netarkivet/harvester/webinterface/Constants.java', revision 1227
Lines |
Description |
Classification |
Status |
86-87 |
add missing "." |
Cosmetic |
NOTOK |
89-99 |
Consider adding separate javadoc for each constant |
Cosmetic |
NOTOK |
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 |
NOTOK |
44-45 |
Put into a try-catch, intercept any SQL-exceptions, and forward an error the user |
Cosmetic |
NOTOK |