Skip to content

BITMAG-1276-refactor JMS resource - #93

Open
Knud-Aage wants to merge 2 commits into
masterfrom
BITMAG-1276-refactor-to-try-with-resources
Open

BITMAG-1276-refactor JMS resource#93
Knud-Aage wants to merge 2 commits into
masterfrom
BITMAG-1276-refactor-to-try-with-resources

Conversation

@Knud-Aage

Copy link
Copy Markdown
Contributor

JVM now guarantees they're closed fully and safely — even if closing one resource throws, the others are still attempted, and the original exception isn't masked.

…r-to-try-with-resources

# Conflicts:
#	bitrepository-core/src/test/java/org/bitrepository/protocol/bus/RawMessagebus.java
@Knud-Aage
Knud-Aage requested review from Thommelise and ole-v-v July 16, 2026 09:55

@ole-v-v ole-v-v left a comment

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Overall looks fine. Please see my detail comments.

}
} No newline at end of file

@Test

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nice tests.

dedicatedMessageBus.close();

addStep("Attempt to send a message on the closed message bus",
"The send should fail, since the underlying JMS session and connection have been closed.");

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking, English usually hasn’t got any comma before “since”. Here and in line 217.

}

/**
* Closes the producer session, consumer session and connection. Declared in reverse close order so

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Nitpicking: The declaration order is an implementation detail and does not belong in the Javadoc. Put it in a comment inside the method instead. The guarantee that all are attempted regardless of exceptions does belong here.

* Closes the connection, suppressing any JMSException, so it can be safely used for cleanup after a
* failed initialisation without masking the original error.
*/
private void closeQuietly(Connection connection) {

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You have this method in two places, here and in RawMessagebus. Can we have it only once and reuse it?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants