Review (NS-65): Bug 555 and other related bugs

Author

Søren

Moderator

Søren

State

Closed

Objectives

Review the changes introduced by KFC to JMSConnection, and related classes.
Note that a problem has already been identifed in the JMSConnection code. One of our unittests provokes this NPE:
Exception in thread "Thread-285" java.lang.NullPointerException
        at dk.netarkivet.common.distribute.JMSConnection.getProducer(JMSConnection.java:445)
        at dk.netarkivet.common.distribute.JMSConnection.doSend(JMSConnection.java:550)
        at dk.netarkivet.common.distribute.JMSConnection.sendMessage(JMSConnection.java:367)
        at dk.netarkivet.common.distribute.JMSConnection.send(JMSConnection.java:200)
        at dk.netarkivet.archive.bitarchive.distribute.BitarchiveMonitorServer.doBatchReply(BitarchiveMonitorServer.java:252)
        at dk.netarkivet.archive.bitarchive.distribute.BitarchiveMonitorServer.access$1(BitarchiveMonitorServer.java:232)
        at dk.netarkivet.archive.bitarchive.distribute.BitarchiveMonitorServer$2.run(BitarchiveMonitorServer.java:218)

Summary

Have now reviewed this code. Probably needs to be reviewed again when the author returns from vacation.
Have found problems with the code, that needs to be patched.
Followup by SVC

General comments:

Description

Classification

Status

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/distribute/ArcRepositoryServer.java', revision 889

Lines

Description

Classification

Status

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/distribute/JMSArcRepositoryClient.java', revision 889

Lines

Description

Classification

Status

Comments on file 'trunk/src/dk/netarkivet/common/distribute/JMSConnection.java', revision 890

Lines

Description

Classification

Status

General

Exception in thread "Thread-285" java.lang.NullPointerException at dk.netarkivet.common.distribute.JMSConnection.getProducer(JMSConnection.java:445) at dk.netarkivet.common.distribute.JMSConnection.doSend(JMSConnection.java:550) at dk.netarkivet.common.distribute.JMSConnection.sendMessage(JMSConnection.java:367) at dk.netarkivet.common.distribute.JMSConnection.send(JMSConnection.java:200) at dk.netarkivet.archive.bitarchive.distribute.BitarchiveMonitorServer.doBatchReply(BitarchiveMonitorServer.java:252) at dk.netarkivet.archive.bitarchive.distribute.BitarchiveMonitorServer.access$1(BitarchiveMonitorServer.java:232) at dk.netarkivet.archive.bitarchive.distribute.BitarchiveMonitorServer$2.run(BitarchiveMonitorServer.java:218)

Cosmetic

OK

65

consumer key (two words rigtht?)

Cosmetic

OK

71

JMX_MAX_TRIES

Cosmetic

REJECTED

199

log -> + ". To:" + msg.getTo()

Cosmetic

OK

213

log -> " To : " + to

Cosmetic

OK

261

Missing javadoc for method removeListener()

Cosmetic

OK

269

Add todo to the javadoc, that this method sets the producers and consumers datastructures to null without initializing them again elsewhere This is problematic when cleanup() is called as part of the reconnection procedure

Cosmetic

OK

442

This method currently throws an NPE in one of our unittests. See general comment above for details

Cosmetic

OK

478

Add missing javadoc to explain this helper method

Cosmetic

OK

487

Add missing javadoc to explain this helper method

Cosmetic

OK

Comments on file 'trunk/src/dk/netarkivet/archive/arcrepository/ArcRepository.java', revision 889

Lines

Description

Classification

Status

Comments on file 'trunk/src/dk/netarkivet/common/utils/JMXUtils.java', revision 889

Lines

Description

Classification

Status

255-256

Move "+"' at end of line to next line

Cosmetic

OK

374-375

Move

at end of line to next line

Cosmetic

OK

387, 389

Too long line

Cosmetic

OK

IssuesFromNs65 (last edited 2010-08-16 10:24:28 by localhost)