From 102b8ff3935ceefa00aa6cc33a7fbafb8b69f94d Mon Sep 17 00:00:00 2001 From: Michael Clay Date: Sun, 24 Nov 2019 19:50:23 +0100 Subject: [PATCH 1/5] [Event Hubs] Replace constant strings with resource strings #3923 - proposal implementation for azure-messaging-eventhubs --- .../checkstyle/checkstyle-suppressions.xml | 8 +++ .../EventHubConsumerAsyncClient.java | 4 +- .../eventhubs/EventHubMessageSerializer.java | 2 +- .../EventHubPartitionAsyncConsumer.java | 6 +- .../EventHubProducerAsyncClient.java | 8 +-- .../azure/messaging/eventhubs/Messages.java | 56 +++++++++++++++++++ .../eventhubs/PartitionBasedLoadBalancer.java | 4 +- .../eventhubs/PartitionPumpManager.java | 10 ++-- .../SynchronousEventSubscriber.java | 3 +- .../SynchronousReceiveWork.java | 3 +- .../messaging/eventhubs/messages.properties | 19 +++++++ .../messaging/eventhubs/MessagesTest.java | 35 ++++++++++++ 12 files changed, 139 insertions(+), 19 deletions(-) create mode 100644 sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java create mode 100644 sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties create mode 100644 sdk/eventhubs/azure-messaging-eventhubs/src/test/java/com/azure/messaging/eventhubs/MessagesTest.java diff --git a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml index af0bc325cb4a..fa5d11072a52 100755 --- a/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml +++ b/eng/code-quality-reports/src/main/resources/checkstyle/checkstyle-suppressions.xml @@ -47,6 +47,9 @@ + + + @@ -128,6 +131,9 @@ + + + @@ -188,6 +194,8 @@ + + diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubConsumerAsyncClient.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubConsumerAsyncClient.java index 3190620dc142..2411890d9639 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubConsumerAsyncClient.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubConsumerAsyncClient.java @@ -299,7 +299,7 @@ public void close() { try { value.close(); } catch (IOException e) { - logger.warning("Exception occurred while closing consumer for partition '{}'", key, e); + logger.warning(Messages.EXCEPTION_OCCURRED_WHILE_CLOSING, key, e); } }); openPartitionConsumers.clear(); @@ -327,7 +327,7 @@ private Flux createConsumer(String linkName, String partitionId, try { consumer.close(); } catch (IOException e) { - logger.warning("Exception occurred while closing consumer {}", linkName, e); + logger.warning(Messages.EXCEPTION_CLOSING_CONSUMER, linkName, e); } } }); diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java index a652a6ace801..74a4c95821aa 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java @@ -215,7 +215,7 @@ private EventData deserializeEventData(Message message) { body = bodyData.getValue().getArray(); } else { logger.warning(String.format(Locale.US, - "Message body type is not of type Data, but type: %s. Not setting body contents.", + Messages.MESSAGE_NOT_OF_TYPE, bodySection != null ? bodySection.getType() : "null")); body = new byte[0]; diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubPartitionAsyncConsumer.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubPartitionAsyncConsumer.java index 31820c42e3a3..512fbbf5e856 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubPartitionAsyncConsumer.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubPartitionAsyncConsumer.java @@ -81,7 +81,7 @@ class EventHubPartitionAsyncConsumer implements Closeable { }); link.getErrors().subscribe(error -> { - logger.info("Error received in ReceiveLink. {}", error.toString()); + logger.info(Messages.ERROR_RECEIVED_IN_RECEIVE_LINK, error.toString()); //TODO (conniey): Surface error to EmitterProcessor. }); @@ -93,7 +93,7 @@ class EventHubPartitionAsyncConsumer implements Closeable { try { close(); } catch (IOException e) { - logger.error("Error closing consumer: {}", e.toString()); + logger.error(Messages.ERROR_CLOSING_CONSUMER, e.toString()); } }); } @@ -115,7 +115,7 @@ class EventHubPartitionAsyncConsumer implements Closeable { }) .doOnRequest(request -> { if (request < MINIMUM_REQUEST) { - logger.warning("Back pressure request value not valid. It must be between {} and {}.", + logger.warning(Messages.REQUEST_VALUE_NOT_VALID, MINIMUM_REQUEST, MAXIMUM_REQUEST); return; } diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubProducerAsyncClient.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubProducerAsyncClient.java index 91b908e2aea9..e16d34de36b9 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubProducerAsyncClient.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubProducerAsyncClient.java @@ -390,7 +390,7 @@ public Mono send(EventDataBatch batch) { if (batch == null) { return monoError(logger, new NullPointerException("'batch' cannot be null.")); } else if (batch.getEvents().isEmpty()) { - logger.warning("Cannot send an EventBatch that is empty."); + logger.warning(Messages.CANNOT_SEND_EVENT_BATCH_EMPTY); return Mono.empty(); } @@ -476,7 +476,7 @@ private Mono sendInternal(Flux eventBatches) { .flatMap(this::send) .then() .doOnError(error -> { - logger.error("Error sending batch.", error); + logger.error(Messages.ERROR_SENDING_BATCH, error); }); } @@ -514,7 +514,7 @@ public void close() { try { value.close(); } catch (IOException e) { - logger.warning("Error closing link for partition: {}", key, e); + logger.warning(Messages.ERROR_CLOSING_LINK_FOR_PARTITION, key, e); } }); openLinks.clear(); @@ -572,7 +572,7 @@ public BiConsumer, EventData> accumulator() { if (maxNumberOfBatches != null && list.size() == maxNumberOfBatches) { final String message = String.format(Locale.US, - "EventData does not fit into maximum number of batches. '%s'", maxNumberOfBatches); + Messages.EVENT_DATA_DOES_NOT_FIT, maxNumberOfBatches); throw new AmqpException(false, AmqpErrorCondition.LINK_PAYLOAD_SIZE_EXCEEDED, message, contextProvider.getErrorContext()); diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java new file mode 100644 index 000000000000..7758ef00ec16 --- /dev/null +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java @@ -0,0 +1,56 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.messaging.eventhubs; + +import com.azure.core.util.logging.ClientLogger; + +import java.io.IOException; +import java.io.InputStream; +import java.util.Properties; + +/** + * I18n messages loaded from the messages.properties file located within the same package. + */ +public enum Messages { + ; + private static final ClientLogger LOGGER = new ClientLogger(Messages.class); + private static final Properties PROPERTIES = loadProperties(); + public static final String PROCESS_SPAN_SCOPE_TYPE_ERROR = getMessage("PROCESS_SPAN_SCOPE_TYPE_ERROR"); + public static final String EXCEPTION_CLOSING_CONSUMER = getMessage("EXCEPTION_CLOSING_CONSUMER"); + public static final String MESSAGE_NOT_OF_TYPE = getMessage("MESSAGE_NOT_OF_TYPE"); + public static final String ERROR_RECEIVED_IN_RECEIVE_LINK = getMessage("ERROR_RECEIVED_IN_RECEIVE_LINK"); + public static final String ERROR_CLOSING_CONSUMER = getMessage("ERROR_CLOSING_CONSUMER"); + public static final String REQUEST_VALUE_NOT_VALID = getMessage("REQUEST_VALUE_NOT_VALID"); + public static final String ERROR_CLOSING_LINK_FOR_PARTITION = getMessage("ERROR_CLOSING_LINK_FOR_PARTITION"); + public static final String EVENT_DATA_DOES_NOT_FIT = getMessage("EVENT_DATA_DOES_NOT_FIT"); + public static final String CANNOT_SEND_EVENT_BATCH_EMPTY = getMessage("CANNOT_SEND_EVENT_BATCH_EMPTY"); + public static final String ERROR_SENDING_BATCH = getMessage("ERROR_SENDING_BATCH"); + public static final String FAILED_TO_CLAIM_OWNERSHIP = getMessage("FAILED_TO_CLAIM_OWNERSHIP"); + public static final String LOAD_BALANCING_FAILED = getMessage("LOAD_BALANCING_FAILED"); + public static final String EVENT_PROCESSOR_RUN_END = getMessage("EVENT_PROCESSOR_RUN_END"); + public static final String FAILED_PROCESSING_ERROR_RECEIVE = getMessage("FAILED_PROCESSING_ERROR_RECEIVE"); + public static final String FAILED_WHILE_PROCESSING_ERROR = getMessage("FAILED_WHILE_PROCESSING_ERROR"); + public static final String FAILED_CLOSE_CONSUMER_PARTITION = getMessage("FAILED_CLOSE_CONSUMER_PARTITION"); + public static final String ERROR_OCCURRED_IN_SUBSCRIBER_ERROR = getMessage("ERROR_OCCURRED_IN_SUBSCRIBER_ERROR"); + public static final String EXCEPTION_OCCURRED_WHILE_EMITTING = getMessage("EXCEPTION_OCCURRED_WHILE_EMITTING"); + public static final String EXCEPTION_OCCURRED_WHILE_CLOSING = getMessage("EXCEPTION_OCCURRED_WHILE_CLOSING"); + + private static Properties loadProperties() { + final Properties properties = new Properties(); + try (InputStream inputStream = Messages.class.getClassLoader().getResourceAsStream("messages.properties")) { + properties.load(inputStream); + } catch (IOException exception) { + LOGGER.error("Error loading message properties {}", Messages.class, exception); + } + return properties; + } + + /** + * @param key the key of the message to retrieve + * @return the message matching the given key + */ + public static String getMessage(String key) { + return String.valueOf(PROPERTIES.getOrDefault(key, key)); + } +} diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/PartitionBasedLoadBalancer.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/PartitionBasedLoadBalancer.java index 99e3534c21d4..7744ff243bd2 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/PartitionBasedLoadBalancer.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/PartitionBasedLoadBalancer.java @@ -107,7 +107,7 @@ void loadBalance() { Mono.zip(partitionOwnershipMono, partitionsMono) .flatMap(this::loadBalance) // if there was an error, log warning and TODO: call user provided error handler - .doOnError(ex -> logger.warning("Load balancing for event processor failed - {}", ex.getMessage())) + .doOnError(ex -> logger.warning(Messages.LOAD_BALANCING_FAILED, ex.getMessage())) .subscribe(); } @@ -332,7 +332,7 @@ private void claimOwnership(final Map partitionOwner .doOnNext(partitionOwnership -> logger.info("Successfully claimed ownership of partition {}", partitionOwnership.getPartitionId())) .doOnError(ex -> logger - .warning("Failed to claim ownership of partition {} - {}", ownershipRequest.getPartitionId(), + .warning(Messages.FAILED_TO_CLAIM_OWNERSHIP, ownershipRequest.getPartitionId(), ex.getMessage(), ex)) .collectList() .zipWith(checkpointStore.listCheckpoints(fullyQualifiedNamespace, eventHubName, consumerGroupName) diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/PartitionPumpManager.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/PartitionPumpManager.java index 8c7f82e81182..5f662aa47e4f 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/PartitionPumpManager.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/PartitionPumpManager.java @@ -90,7 +90,7 @@ void stopAllPartitionPumps() { try { eventHubConsumer.close(); } catch (Exception ex) { - logger.warning("Failed to close consumer for partition {}", partitionId, ex); + logger.warning(Messages.FAILED_CLOSE_CONSUMER_PARTITION, partitionId, ex); } finally { partitionPumps.remove(partitionId); } @@ -168,7 +168,7 @@ private void handleProcessingError(PartitionOwnership claimedOwnership, Partitio // also fails just log and continue partitionProcessor.processError(new ErrorContext(partitionContext, error)); } catch (Exception ex) { - logger.warning("Failed while processing error {}", claimedOwnership.getPartitionId(), ex); + logger.warning(Messages.FAILED_WHILE_PROCESSING_ERROR, claimedOwnership.getPartitionId(), ex); } } @@ -186,7 +186,7 @@ private void handleReceiveError(PartitionOwnership claimedOwnership, EventHubCon } partitionProcessor.close(new CloseContext(partitionContext, closeReason)); } catch (Exception ex) { - logger.warning("Failed while processing error on receive {}", claimedOwnership.getPartitionId(), ex); + logger.warning(Messages.FAILED_PROCESSING_ERROR_RECEIVE, claimedOwnership.getPartitionId(), ex); } finally { try { // close the consumer @@ -225,12 +225,12 @@ private void endProcessTracingSpan(Context processSpanContext, Signal sign close.close(); tracerProvider.endSpan(processSpanContext, signal); } catch (IOException ioException) { - logger.error("EventProcessor.run() endTracingSpan().close() failed with an error %s", ioException); + logger.error(Messages.EVENT_PROCESSOR_RUN_END, ioException); } } else { logger.warning(String.format(Locale.US, - "Process span scope type is not of type Closeable, but type: %s. Not closing the scope and span", + Messages.PROCESS_SPAN_SCOPE_TYPE_ERROR, spanScope.get() != null ? spanScope.getClass() : "null")); } } diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/SynchronousEventSubscriber.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/SynchronousEventSubscriber.java index 5b5c1779977d..b588d7107d9e 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/SynchronousEventSubscriber.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/SynchronousEventSubscriber.java @@ -4,6 +4,7 @@ package com.azure.messaging.eventhubs.implementation; import com.azure.core.util.logging.ClientLogger; +import com.azure.messaging.eventhubs.Messages; import com.azure.messaging.eventhubs.models.PartitionEvent; import org.reactivestreams.Subscription; import reactor.core.publisher.BaseSubscriber; @@ -69,7 +70,7 @@ protected void hookOnComplete() { */ @Override protected void hookOnError(Throwable throwable) { - logger.error("Error occurred in subscriber. Error: {}", throwable); + logger.error(Messages.ERROR_OCCURRED_IN_SUBSCRIBER_ERROR, throwable); work.error(throwable); dispose(); } diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/SynchronousReceiveWork.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/SynchronousReceiveWork.java index 8637fca6c24b..1ec6fd0ddee2 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/SynchronousReceiveWork.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/implementation/SynchronousReceiveWork.java @@ -4,6 +4,7 @@ package com.azure.messaging.eventhubs.implementation; import com.azure.core.util.logging.ClientLogger; +import com.azure.messaging.eventhubs.Messages; import com.azure.messaging.eventhubs.models.PartitionEvent; import reactor.core.publisher.FluxSink; @@ -88,7 +89,7 @@ public void next(PartitionEvent event) { emitter.next(event); remaining.decrementAndGet(); } catch (Exception e) { - logger.warning("Exception occurred while emitting next received event.", e); + logger.warning(Messages.EXCEPTION_OCCURRED_WHILE_EMITTING, e); isTerminal = true; emitter.error(e); } diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties b/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties new file mode 100644 index 000000000000..3d8d7a9033c5 --- /dev/null +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties @@ -0,0 +1,19 @@ +PROCESS_SPAN_SCOPE_TYPE_ERROR=Process span scope type is not of type Closeable, but type: %s. Not closing the scope and span +EXCEPTION_CLOSING_CONSUMER=Exception occurred while closing consumer {} +MESSAGE_NOT_OF_TYPE=Message body type is not of type Data, but type: %s. Not setting body contents. +ERROR_RECEIVED_IN_RECEIVE_LINK=Error received in ReceiveLink. {} +ERROR_CLOSING_CONSUMER=Error closing consumer: {} +REQUEST_VALUE_NOT_VALID=Back pressure request value not valid. It must be between {} and {}. +ERROR_CLOSING_LINK_FOR_PARTITION=Error closing link for partition: {} +EVENT_DATA_DOES_NOT_FIT=EventData does not fit into maximum number of batches. '%s' +CANNOT_SEND_EVENT_BATCH_EMPTY=Cannot send an EventBatch that is empty. +ERROR_SENDING_BATCH=Error sending batch. +FAILED_TO_CLAIM_OWNERSHIP=Failed to claim ownership of partition {} - {} +LOAD_BALANCING_FAILED=Load balancing for event processor failed - {} +EVENT_PROCESSOR_RUN_END=EventProcessor.run() endTracingSpan().close() failed with an error %s +FAILED_PROCESSING_ERROR_RECEIVE=Failed while processing error on receive {} +FAILED_WHILE_PROCESSING_ERROR=Failed while processing error {} +FAILED_CLOSE_CONSUMER_PARTITION=Failed to close consumer for partition {} +ERROR_OCCURRED_IN_SUBSCRIBER_ERROR=Error occurred in subscriber. Error: {} +EXCEPTION_OCCURRED_WHILE_EMITTING=Exception occurred while emitting next received event. +EXCEPTION_OCCURRED_WHILE_CLOSING=Exception occurred while closing consumer for partition '{}' diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/test/java/com/azure/messaging/eventhubs/MessagesTest.java b/sdk/eventhubs/azure-messaging-eventhubs/src/test/java/com/azure/messaging/eventhubs/MessagesTest.java new file mode 100644 index 000000000000..afb6a9b716c6 --- /dev/null +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/test/java/com/azure/messaging/eventhubs/MessagesTest.java @@ -0,0 +1,35 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.messaging.eventhubs; + +import org.junit.jupiter.params.ParameterizedTest; +import org.junit.jupiter.params.provider.MethodSource; + +import java.lang.reflect.Field; +import java.util.Objects; +import java.util.stream.Stream; + +import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.junit.jupiter.api.Assertions.assertNotEquals; + +class MessagesTest { + + static Stream keys() { + return Stream.of(Messages.class.getFields()).map(Field::getName); + } + + @ParameterizedTest + @MethodSource("keys") + void getMessage(String messageKey) { + assertNotEquals(messageKey, Messages.getMessage(messageKey)); + } + + @ParameterizedTest + @MethodSource("keys") + void messageField(String messageKey) throws NoSuchFieldException, IllegalAccessException { + Field field = Messages.class.getField(messageKey); + field.setAccessible(true); + assertEquals(Messages.getMessage(messageKey), Objects.toString(field.get(Messages.class))); + } +} From acd3d98c3111071e3daa59bcce2e917e3bc19887 Mon Sep 17 00:00:00 2001 From: Michael Clay Date: Sun, 24 Nov 2019 20:43:46 +0100 Subject: [PATCH 2/5] Switch to JUnit version 5 for unit tests #4955 - migrate unit tests in sdk/storage/* --- .../azure/messaging/eventhubs/Messages.java | 18 +++++++++++------- 1 file changed, 11 insertions(+), 7 deletions(-) diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java index 7758ef00ec16..63c70c58791a 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java @@ -15,7 +15,8 @@ public enum Messages { ; private static final ClientLogger LOGGER = new ClientLogger(Messages.class); - private static final Properties PROPERTIES = loadProperties(); + private static Properties PROPERTIES; + private static final String MESSAGES_PROPERTIES_PATH = "com/azure/messaging/eventhubs/messages.properties"; public static final String PROCESS_SPAN_SCOPE_TYPE_ERROR = getMessage("PROCESS_SPAN_SCOPE_TYPE_ERROR"); public static final String EXCEPTION_CLOSING_CONSUMER = getMessage("EXCEPTION_CLOSING_CONSUMER"); public static final String MESSAGE_NOT_OF_TYPE = getMessage("MESSAGE_NOT_OF_TYPE"); @@ -36,14 +37,17 @@ public enum Messages { public static final String EXCEPTION_OCCURRED_WHILE_EMITTING = getMessage("EXCEPTION_OCCURRED_WHILE_EMITTING"); public static final String EXCEPTION_OCCURRED_WHILE_CLOSING = getMessage("EXCEPTION_OCCURRED_WHILE_CLOSING"); - private static Properties loadProperties() { - final Properties properties = new Properties(); - try (InputStream inputStream = Messages.class.getClassLoader().getResourceAsStream("messages.properties")) { - properties.load(inputStream); + private static synchronized Properties getProperties() { + if (PROPERTIES != null) { + return PROPERTIES; + } + PROPERTIES = new Properties(); + try (InputStream inputStream = Thread.currentThread().getContextClassLoader().getResourceAsStream(MESSAGES_PROPERTIES_PATH)) { + PROPERTIES.load(inputStream); } catch (IOException exception) { LOGGER.error("Error loading message properties {}", Messages.class, exception); } - return properties; + return PROPERTIES; } /** @@ -51,6 +55,6 @@ private static Properties loadProperties() { * @return the message matching the given key */ public static String getMessage(String key) { - return String.valueOf(PROPERTIES.getOrDefault(key, key)); + return String.valueOf(getProperties().getOrDefault(key, key)); } } From 7ec47c6c337ee346769b5444ba87e89b26c1fbfa Mon Sep 17 00:00:00 2001 From: Michael Clay Date: Sun, 24 Nov 2019 22:15:17 +0100 Subject: [PATCH 3/5] [Event Hubs] Replace constant strings with resource strings #3923 - checkstyle fix --- .../com/azure/messaging/eventhubs/Messages.java | 15 ++++++++------- 1 file changed, 8 insertions(+), 7 deletions(-) diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java index 63c70c58791a..9462d5a5af9d 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java @@ -15,7 +15,7 @@ public enum Messages { ; private static final ClientLogger LOGGER = new ClientLogger(Messages.class); - private static Properties PROPERTIES; + private static Properties properties; private static final String MESSAGES_PROPERTIES_PATH = "com/azure/messaging/eventhubs/messages.properties"; public static final String PROCESS_SPAN_SCOPE_TYPE_ERROR = getMessage("PROCESS_SPAN_SCOPE_TYPE_ERROR"); public static final String EXCEPTION_CLOSING_CONSUMER = getMessage("EXCEPTION_CLOSING_CONSUMER"); @@ -38,16 +38,17 @@ public enum Messages { public static final String EXCEPTION_OCCURRED_WHILE_CLOSING = getMessage("EXCEPTION_OCCURRED_WHILE_CLOSING"); private static synchronized Properties getProperties() { - if (PROPERTIES != null) { - return PROPERTIES; + if (properties != null) { + return properties; } - PROPERTIES = new Properties(); - try (InputStream inputStream = Thread.currentThread().getContextClassLoader().getResourceAsStream(MESSAGES_PROPERTIES_PATH)) { - PROPERTIES.load(inputStream); + properties = new Properties(); + try (InputStream inputStream = + Thread.currentThread().getContextClassLoader().getResourceAsStream(MESSAGES_PROPERTIES_PATH)) { + properties.load(inputStream); } catch (IOException exception) { LOGGER.error("Error loading message properties {}", Messages.class, exception); } - return PROPERTIES; + return properties; } /** From 222c08fdb4f5206c323af0b7c3adc10b3d7c27ad Mon Sep 17 00:00:00 2001 From: Michael Clay Date: Mon, 25 Nov 2019 21:10:20 +0100 Subject: [PATCH 4/5] [Event Hubs] Replace constant strings with resource strings #3923 - proposal implementation for azure-messaging-eventhubs - incorporated first round of review issues - resolve merge conflicts - remove unused message keys --- .../messaging/eventhubs/EventHubMessageSerializer.java | 9 +++------ .../java/com/azure/messaging/eventhubs/Messages.java | 7 +------ .../com/azure/messaging/eventhubs/messages.properties | 5 ----- 3 files changed, 4 insertions(+), 17 deletions(-) diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java index 74a4c95821aa..d028f8a80fce 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java @@ -24,7 +24,6 @@ import java.time.Instant; import java.util.Date; import java.util.HashMap; -import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -155,7 +154,7 @@ private T deserializeManagementResponse(Message message, Class deserializ } else if (deserializedType == EventHubProperties.class) { return (T) toEventHubProperties(amqpBody); } else { - throw logger.logExceptionAsError(new IllegalArgumentException(String.format(Locale.US, + throw logger.logExceptionAsError(new IllegalArgumentException(String.format( "Class '%s' is not a supported deserializable type.", deserializedType))); } } @@ -214,8 +213,7 @@ private EventData deserializeEventData(Message message) { Data bodyData = (Data) bodySection; body = bodyData.getValue().getArray(); } else { - logger.warning(String.format(Locale.US, - Messages.MESSAGE_NOT_OF_TYPE, + logger.warning(String.format(Messages.MESSAGE_NOT_OF_TYPE, bodySection != null ? bodySection.getType() : "null")); body = new byte[0]; @@ -347,7 +345,6 @@ private static void setSystemProperties(EventData eventData, Message message) { default: throw new IllegalArgumentException( String.format( - Locale.US, "Property is not a recognized reserved property name: %s", key)); } @@ -426,7 +423,7 @@ private static int sizeof(Object obj) { return Double.BYTES; } - throw new IllegalArgumentException(String.format(Locale.US, "Encoding Type: %s is not supported", + throw new IllegalArgumentException(String.format("Encoding Type: %s is not supported", obj.getClass())); } diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java index 9462d5a5af9d..7e5bb4b48b48 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java @@ -18,12 +18,8 @@ public enum Messages { private static Properties properties; private static final String MESSAGES_PROPERTIES_PATH = "com/azure/messaging/eventhubs/messages.properties"; public static final String PROCESS_SPAN_SCOPE_TYPE_ERROR = getMessage("PROCESS_SPAN_SCOPE_TYPE_ERROR"); - public static final String EXCEPTION_CLOSING_CONSUMER = getMessage("EXCEPTION_CLOSING_CONSUMER"); public static final String MESSAGE_NOT_OF_TYPE = getMessage("MESSAGE_NOT_OF_TYPE"); - public static final String ERROR_RECEIVED_IN_RECEIVE_LINK = getMessage("ERROR_RECEIVED_IN_RECEIVE_LINK"); - public static final String ERROR_CLOSING_CONSUMER = getMessage("ERROR_CLOSING_CONSUMER"); public static final String REQUEST_VALUE_NOT_VALID = getMessage("REQUEST_VALUE_NOT_VALID"); - public static final String ERROR_CLOSING_LINK_FOR_PARTITION = getMessage("ERROR_CLOSING_LINK_FOR_PARTITION"); public static final String EVENT_DATA_DOES_NOT_FIT = getMessage("EVENT_DATA_DOES_NOT_FIT"); public static final String CANNOT_SEND_EVENT_BATCH_EMPTY = getMessage("CANNOT_SEND_EVENT_BATCH_EMPTY"); public static final String ERROR_SENDING_BATCH = getMessage("ERROR_SENDING_BATCH"); @@ -35,7 +31,6 @@ public enum Messages { public static final String FAILED_CLOSE_CONSUMER_PARTITION = getMessage("FAILED_CLOSE_CONSUMER_PARTITION"); public static final String ERROR_OCCURRED_IN_SUBSCRIBER_ERROR = getMessage("ERROR_OCCURRED_IN_SUBSCRIBER_ERROR"); public static final String EXCEPTION_OCCURRED_WHILE_EMITTING = getMessage("EXCEPTION_OCCURRED_WHILE_EMITTING"); - public static final String EXCEPTION_OCCURRED_WHILE_CLOSING = getMessage("EXCEPTION_OCCURRED_WHILE_CLOSING"); private static synchronized Properties getProperties() { if (properties != null) { @@ -46,7 +41,7 @@ private static synchronized Properties getProperties() { Thread.currentThread().getContextClassLoader().getResourceAsStream(MESSAGES_PROPERTIES_PATH)) { properties.load(inputStream); } catch (IOException exception) { - LOGGER.error("Error loading message properties {}", Messages.class, exception); + LOGGER.error("Error loading message properties {}", Messages.class, exception); //NON-NLS } return properties; } diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties b/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties index 3d8d7a9033c5..e20933d06bdc 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties @@ -1,10 +1,6 @@ PROCESS_SPAN_SCOPE_TYPE_ERROR=Process span scope type is not of type Closeable, but type: %s. Not closing the scope and span -EXCEPTION_CLOSING_CONSUMER=Exception occurred while closing consumer {} MESSAGE_NOT_OF_TYPE=Message body type is not of type Data, but type: %s. Not setting body contents. -ERROR_RECEIVED_IN_RECEIVE_LINK=Error received in ReceiveLink. {} -ERROR_CLOSING_CONSUMER=Error closing consumer: {} REQUEST_VALUE_NOT_VALID=Back pressure request value not valid. It must be between {} and {}. -ERROR_CLOSING_LINK_FOR_PARTITION=Error closing link for partition: {} EVENT_DATA_DOES_NOT_FIT=EventData does not fit into maximum number of batches. '%s' CANNOT_SEND_EVENT_BATCH_EMPTY=Cannot send an EventBatch that is empty. ERROR_SENDING_BATCH=Error sending batch. @@ -16,4 +12,3 @@ FAILED_WHILE_PROCESSING_ERROR=Failed while processing error {} FAILED_CLOSE_CONSUMER_PARTITION=Failed to close consumer for partition {} ERROR_OCCURRED_IN_SUBSCRIBER_ERROR=Error occurred in subscriber. Error: {} EXCEPTION_OCCURRED_WHILE_EMITTING=Exception occurred while emitting next received event. -EXCEPTION_OCCURRED_WHILE_CLOSING=Exception occurred while closing consumer for partition '{}' From dfd54cda234e864d7f90e6f0b1b9404652d35c33 Mon Sep 17 00:00:00 2001 From: Michael Clay Date: Tue, 26 Nov 2019 20:42:48 +0100 Subject: [PATCH 5/5] [Event Hubs] Replace constant strings with resource strings #3923 - incorporated review issues --- .../messaging/eventhubs/EventHubMessageSerializer.java | 4 ++-- .../java/com/azure/messaging/eventhubs/Messages.java | 10 ++++++++-- .../com/azure/messaging/eventhubs/messages.properties | 2 ++ 3 files changed, 12 insertions(+), 4 deletions(-) diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java index d028f8a80fce..e6a8a056f7ff 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/EventHubMessageSerializer.java @@ -155,7 +155,7 @@ private T deserializeManagementResponse(Message message, Class deserializ return (T) toEventHubProperties(amqpBody); } else { throw logger.logExceptionAsError(new IllegalArgumentException(String.format( - "Class '%s' is not a supported deserializable type.", deserializedType))); + Messages.CLASS_NOT_A_SUPPORTED_TYPE, deserializedType))); } } @@ -423,7 +423,7 @@ private static int sizeof(Object obj) { return Double.BYTES; } - throw new IllegalArgumentException(String.format("Encoding Type: %s is not supported", + throw new IllegalArgumentException(String.format(Messages.ENCODING_TYPE_NOT_SUPPORTED, obj.getClass())); } diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java index 7e5bb4b48b48..0580ea3468bc 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/java/com/azure/messaging/eventhubs/Messages.java @@ -17,6 +17,8 @@ public enum Messages { private static final ClientLogger LOGGER = new ClientLogger(Messages.class); private static Properties properties; private static final String MESSAGES_PROPERTIES_PATH = "com/azure/messaging/eventhubs/messages.properties"; + public static final String CLASS_NOT_A_SUPPORTED_TYPE = getMessage("CLASS_NOT_A_SUPPORTED_TYPE"); + public static final String ENCODING_TYPE_NOT_SUPPORTED = getMessage("ENCODING_TYPE_NOT_SUPPORTED"); public static final String PROCESS_SPAN_SCOPE_TYPE_ERROR = getMessage("PROCESS_SPAN_SCOPE_TYPE_ERROR"); public static final String MESSAGE_NOT_OF_TYPE = getMessage("MESSAGE_NOT_OF_TYPE"); public static final String REQUEST_VALUE_NOT_VALID = getMessage("REQUEST_VALUE_NOT_VALID"); @@ -39,9 +41,13 @@ private static synchronized Properties getProperties() { properties = new Properties(); try (InputStream inputStream = Thread.currentThread().getContextClassLoader().getResourceAsStream(MESSAGES_PROPERTIES_PATH)) { - properties.load(inputStream); + if (inputStream != null) { + properties.load(inputStream); + } else { + LOGGER.error("Message properties [{}] not found", MESSAGES_PROPERTIES_PATH); //NON-NLS + } } catch (IOException exception) { - LOGGER.error("Error loading message properties {}", Messages.class, exception); //NON-NLS + LOGGER.error("Error loading message properties [{}]", MESSAGES_PROPERTIES_PATH, exception); //NON-NLS } return properties; } diff --git a/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties b/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties index e20933d06bdc..c3f1ffcbf7cb 100644 --- a/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties +++ b/sdk/eventhubs/azure-messaging-eventhubs/src/main/resources/com/azure/messaging/eventhubs/messages.properties @@ -12,3 +12,5 @@ FAILED_WHILE_PROCESSING_ERROR=Failed while processing error {} FAILED_CLOSE_CONSUMER_PARTITION=Failed to close consumer for partition {} ERROR_OCCURRED_IN_SUBSCRIBER_ERROR=Error occurred in subscriber. Error: {} EXCEPTION_OCCURRED_WHILE_EMITTING=Exception occurred while emitting next received event. +CLASS_NOT_A_SUPPORTED_TYPE=Class '%s' is not a supported deserializable type. +ENCODING_TYPE_NOT_SUPPORTED=Encoding Type: %s is not supported