Differences between revisions 1 and 2
⇤ ← Revision 1 as of 2010-02-05 13:52:25 →
Size: 10335
Editor: ColinRosenthal
Comment:
← Revision 2 as of 2010-02-09 12:22:27 → ⇥
Size: 10121
Editor: ColinRosenthal
Comment:
Deletions are marked like this. Additions are marked like this.
Line 7: Line 7:
A component to allow lists of crawler traps to be appended to all scheduled jobs. There are  A component to allow lists of crawler traps to be appended to all scheduled jobs. There are
Line 15: Line 15:
|| 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 ||		 || 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 ||
Line 21: Line 18:

|| Time Used svc: 1md csr: 12md || Cosmetic || NOTOK ||
Line 24: Line 23:
|| 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 ||		 || 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 ||
Line 28: Line 27:
|| General || Add NetarchiveSuite header-file || Cosmetic || NOTOK ||
|| 24 || long line; make newline before 'value=..' || Cosmetic || NOTOK ||		 || General || Add NetarchiveSuite header-file || Cosmetic || OK ||
|| 24 || long line; make newline before 'value=..' || Cosmetic || OK ||
Line 32: Line 31:
|| 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 ||		 || 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 ||
Line 39: Line 38:
|| 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 ||		 || 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 ||
Line 44: Line 43:
|| 394 || Missing javadoc for this method || Cosmetic || NOTOK ||
|| 401 || Remove redundant line || Cosmetic || NOTOK ||		 || 394 || Missing javadoc for this method || Cosmetic || OK ||
|| 401 || Remove redundant line || Cosmetic || OK ||
Line 48: Line 47:
|| 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 ||		 || 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 ||
Line 66: Line 65:
|| 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 ||		 || 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 ||
Line 73: Line 72:
|| General || Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() || Cosmetic || NOTOK || || General || Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() || Cosmetic || OK ||
Line 76: Line 75:
|| 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 ||		 || 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 ||
Line 98: Line 97:
|| General || Add NetarchiveSuite fileheader || Cosmetic || NOTOK || || General || Add NetarchiveSuite fileheader || Cosmetic || OK ||
Line 101: Line 100:
|| General || Add NetarchiveSuite fileheader || Cosmetic || NOTOK || || General || Add NetarchiveSuite fileheader || Cosmetic || OK ||
Line 104: Line 103:
|| General || Missing standard NetarchiveSuite header-file || Cosmetic || NOTOK || || General || Missing standard NetarchiveSuite header-file || Cosmetic || OK ||
Line 107: Line 106:
|| General || Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() || Cosmetic || NOTOK ||
|| 320 || Is the case not wrong here? || Cosmetic || NOTOK ||		 || General || Remove long lines in methods createGlobalcrawlerTrapLists() and createGlobalcrawlerTrapExpressions() || Cosmetic || OK ||
|| 320 || Is the case not wrong here? || Cosmetic || OK ||
Line 111: Line 110:
|| 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 ||		 || 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 ||
Line 115: Line 114:
|| General || Add NetarchiveSuite fileheader || Cosmetic || NOTOK || || General || Add NetarchiveSuite fileheader || Cosmetic || OK ||
Line 118: Line 117:
|| 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 ||		 || 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 ||
Line 122: Line 121:
|| 86-87 || add missing "." || Cosmetic || NOTOK ||
|| 89-99 || Consider adding separate javadoc for each constant || Cosmetic || NOTOK ||		 || 86-87 || add missing "." || Cosmetic || OK ||
|| 89-99 || Consider adding separate javadoc for each constant || Cosmetic ||Rejected||
Line 126: Line 125:
|| 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 ||		 || 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 ||

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

IssuesFromNs143 (last edited 2010-08-16 10:24:44 by localhost)