From 8dff688b3e925079e826231d2f5d5e2549c272a9 Mon Sep 17 00:00:00 2001 From: bohlski Date: Mon, 27 Jun 2022 13:40:51 +0200 Subject: [PATCH] More context added on message response logging --- .../GeneralConversationState.java | 8 +++---- .../selector/ContributorResponseStatus.java | 24 +++++++++++++------ .../putfile/PutFileClientComponentTest.java | 2 +- .../protocol/utils/MessageUtils.java | 11 +++++---- 4 files changed, 29 insertions(+), 16 deletions(-) diff --git a/bitrepository-client/src/main/java/org/bitrepository/client/conversation/GeneralConversationState.java b/bitrepository-client/src/main/java/org/bitrepository/client/conversation/GeneralConversationState.java index 96d9b939d..c6b3a4bf8 100644 --- a/bitrepository-client/src/main/java/org/bitrepository/client/conversation/GeneralConversationState.java +++ b/bitrepository-client/src/main/java/org/bitrepository/client/conversation/GeneralConversationState.java @@ -49,7 +49,7 @@ public abstract class GeneralConversationState implements ConversationState { private final Logger log = LoggerFactory.getLogger(getClass()); /** The scheduler used for timeout checks. */ private static final ScheduledExecutorService timer = Executors.newScheduledThreadPool(1, - new DefaultThreadFactory("ConversationState-Timeout-", Thread.NORM_PRIORITY)); + new DefaultThreadFactory("ConversationState-Timeout-", Thread.NORM_PRIORITY)); private ScheduledFuture scheduledTimeout; /** For response bookkeeping */ private final ContributorResponseStatus responseStatus; @@ -63,7 +63,7 @@ protected GeneralConversationState(Collection expectedContributors) { } /** - * Startes the state by:
    + * Starts the state by:
      *
    1. Scheduling a timeout.
      2. *
    2. Sends the request which triggers the responses for this state.
      3. *
    @@ -107,9 +107,9 @@ public final void handleMessage(Message message) { } try { - if(processMessage(response)) { + if (processMessage(response)) { responseStatus.responseReceived(response); - if (responseStatus.haveAllComponentsResponded()) { + if (responseStatus.haveAllComponentsResponded(response)) { scheduledTimeout.cancel(true); changeState(); } diff --git a/bitrepository-client/src/main/java/org/bitrepository/client/conversation/selector/ContributorResponseStatus.java b/bitrepository-client/src/main/java/org/bitrepository/client/conversation/selector/ContributorResponseStatus.java index 88d3d825b..bd073f71d 100644 --- a/bitrepository-client/src/main/java/org/bitrepository/client/conversation/selector/ContributorResponseStatus.java +++ b/bitrepository-client/src/main/java/org/bitrepository/client/conversation/selector/ContributorResponseStatus.java @@ -60,13 +60,14 @@ public ContributorResponseStatus(Collection componentsWhichShouldRespond public final void responseReceived(MessageResponse response) { if (MessageUtils.isEndMessageForPrimitive(response)) { String componentID = response.getFrom(); - log.debug("Received response from: " + componentID); + log.debug("Received response from: {} ({})", componentID, MessageUtils.createMessageIdentifier(response)); if (componentsWithOutstandingResponse.contains(componentID)) { componentsWithOutstandingResponse.remove(componentID); - } else if (!componentsWithOutstandingResponse.contains(componentID) && componentsWhichShouldRespond.contains(componentID)) { - log.debug("Received more than one response from component " + componentID); + } else if (!componentsWithOutstandingResponse.contains(componentID) && + componentsWhichShouldRespond.contains(componentID)) { + log.debug("Received more than one response from component {}", componentID); } else { - log.debug("Received response from irrelevant component " + componentID); + log.debug("Received response from irrelevant component {}", componentID); } } } @@ -83,12 +84,21 @@ public Collection getOutstandingComponents() { /** * @return true if all components have responded. False otherwise */ - public final boolean haveAllComponentsResponded() { - log.debug("Expected contributors: " + componentsWhichShouldRespond + ", components that have not answered: " + - componentsWithOutstandingResponse); + public final boolean haveAllComponentsResponded(MessageResponse response) { + logContributorsStatus(response); return componentsWithOutstandingResponse.isEmpty(); } + /** + * Logs status of expected- and outstanding contributors. + * @param response The received response. + */ + private void logContributorsStatus(MessageResponse response) { + log.debug("Expected contributors: {}, components that have not answered: {} ({})", + componentsWhichShouldRespond, componentsWithOutstandingResponse, + MessageUtils.createMessageIdentifier(response)); + } + /** * @return The set of components which should respond to the identification request. */ diff --git a/bitrepository-client/src/test/java/org/bitrepository/modify/putfile/PutFileClientComponentTest.java b/bitrepository-client/src/test/java/org/bitrepository/modify/putfile/PutFileClientComponentTest.java index 0335e6a1f..8a61ce8fb 100644 --- a/bitrepository-client/src/test/java/org/bitrepository/modify/putfile/PutFileClientComponentTest.java +++ b/bitrepository-client/src/test/java/org/bitrepository/modify/putfile/PutFileClientComponentTest.java @@ -407,7 +407,7 @@ public void fileExistsOnPillarDifferentChecksumFromPillar() throws Exception { @Test(groups={"regressiontest"}) public void sameFileExistsOnOnePillar() throws Exception { addDescription("Tests that PutClient handles the presence of a file correctly, when the pillar " + - "returns a checksum equal the file being put (idempotent). "); + "returns a checksum equal the file being put (idempotent)."); TestEventHandler testEventHandler = new TestEventHandler(testEventManager); PutFileClient putClient = createPutFileClient(); diff --git a/bitrepository-core/src/main/java/org/bitrepository/protocol/utils/MessageUtils.java b/bitrepository-core/src/main/java/org/bitrepository/protocol/utils/MessageUtils.java index fdd432b9e..ab556a707 100644 --- a/bitrepository-core/src/main/java/org/bitrepository/protocol/utils/MessageUtils.java +++ b/bitrepository-core/src/main/java/org/bitrepository/protocol/utils/MessageUtils.java @@ -54,7 +54,8 @@ public static boolean isPositiveIdentifyResponse(MessageResponse response) { */ public static boolean isPositiveProgressResponse(MessageResponse response) { ResponseCode responseCode = response.getResponseInfo().getResponseCode(); - return responseCode.equals(ResponseCode.IDENTIFICATION_POSITIVE) || responseCode.equals(ResponseCode.OPERATION_ACCEPTED_PROGRESS) || + return responseCode.equals(ResponseCode.IDENTIFICATION_POSITIVE) || + responseCode.equals(ResponseCode.OPERATION_ACCEPTED_PROGRESS) || responseCode.equals(ResponseCode.OPERATION_PROGRESS); } @@ -66,7 +67,8 @@ public static boolean isPositiveProgressResponse(MessageResponse response) { */ public static boolean isIdentifyResponse(MessageResponse response) { ResponseCode responseCode = response.getResponseInfo().getResponseCode(); - return responseCode.equals(ResponseCode.IDENTIFICATION_POSITIVE) || responseCode.equals(ResponseCode.IDENTIFICATION_NEGATIVE); + return responseCode.equals(ResponseCode.IDENTIFICATION_POSITIVE) || + responseCode.equals(ResponseCode.IDENTIFICATION_NEGATIVE); } /** @@ -82,12 +84,13 @@ public static boolean isIdentifyRequest(Message message) { /** * @param response the supplied message - * @return whether the supplied message can be considered a end response for a primitive, emg. ends a series of + * @return whether the supplied message can be considered an end response for a primitive, i.e. it ends a series of * identify or operation responses. */ public static boolean isEndMessageForPrimitive(MessageResponse response) { ResponseCode responseCode = response.getResponseInfo().getResponseCode(); - return !(responseCode.equals(ResponseCode.OPERATION_PROGRESS) || responseCode.equals(ResponseCode.OPERATION_ACCEPTED_PROGRESS)); + return !(responseCode.equals(ResponseCode.OPERATION_PROGRESS) || + responseCode.equals(ResponseCode.OPERATION_ACCEPTED_PROGRESS)); } /**