From b23438371045c9a85e90d68dd8cfe8b93e0e8bed Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 10 Nov 2020 21:10:37 -0800 Subject: [PATCH 01/23] Amqp Message Update - API Change --- .../checkstyle/checkstyle-suppressions.xml | 1 + .../src/main/resources/revapi/revapi.json | 70 ++++++++++++++ .../azure/core/amqp/models/AmqpAddress.java | 62 +++++++++++++ .../amqp/models/AmqpAnnotatedMessage.java | 21 ++++- .../azure/core/amqp/models/AmqpBodyType.java | 10 +- .../azure/core/amqp/models/AmqpDataBody.java | 41 -------- .../core/amqp/models/AmqpMessageBody.java | 93 ++++++++++++++++++- .../azure/core/amqp/models/AmqpMessageId.java | 64 +++++++++++++ .../amqp/models/AmqpMessageProperties.java | 24 ++--- .../amqp/models/AmqpAnnotatedMessageTest.java | 22 ++--- ...BodyTest.java => AmqpMessageBodyTest.java} | 10 +- .../azure-messaging-servicebus/pom.xml | 2 +- .../servicebus/ServiceBusMessage.java | 63 ++++++++++--- .../ServiceBusMessageSerializer.java | 16 +++- .../servicebus/ServiceBusReceivedMessage.java | 63 ++++++++++--- ...BusReceiverAsyncClientIntegrationTest.java | 12 ++- 16 files changed, 457 insertions(+), 117 deletions(-) create mode 100644 sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java delete mode 100644 sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpDataBody.java create mode 100644 sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java rename sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/{AmqpDataBodyTest.java => AmqpMessageBodyTest.java} (83%) 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 42252c13d1b0..c8816fe48fa6 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 @@ -269,6 +269,7 @@ + diff --git a/eng/code-quality-reports/src/main/resources/revapi/revapi.json b/eng/code-quality-reports/src/main/resources/revapi/revapi.json index 26e55d5d88a7..efd1dd748636 100644 --- a/eng/code-quality-reports/src/main/resources/revapi/revapi.json +++ b/eng/code-quality-reports/src/main/resources/revapi/revapi.json @@ -269,6 +269,76 @@ "code": "java.annotation.added", "new": "class com.azure.storage.blob.models.PageList", "justification": "Annotation required to resolve deserialization bug." + },{ + "code": "java.method.returnTypeChanged", + "old": "method com.azure.core.util.IterableStream com.azure.core.amqp.models.AmqpDataBody::getData()", + "new": "method java.util.List com.azure.core.amqp.models.AmqpDataBody::getData()", + "justification": "Updated " + }, + { + "code": "java.class.removed", + "old": "class com.azure.core.amqp.models.AmqpDataBody", + "justification": "Renamed as AmqpMessageBody." + }, + { + "code": "java.class.kindChanged", + "old": "interface com.azure.core.amqp.models.AmqpMessageBody", + "new": "class com.azure.core.amqp.models.AmqpMessageBody", + "justification": "AmqpMessageBody is class representing all the new AMQP data types." + }, + { + "code": "java.class.nowFinal", + "old": "interface com.azure.core.amqp.models.AmqpMessageBody", + "new": "class com.azure.core.amqp.models.AmqpMessageBody", + "justification": "Made it final." + }, + { + "code": "java.method.returnTypeChanged", + "old": "method java.lang.String com.azure.core.amqp.models.AmqpMessageProperties::getCorrelationId()", + "new": "method com.azure.core.amqp.models.AmqpMessageId com.azure.core.amqp.models.AmqpMessageProperties::getCorrelationId()", + "justification": "New return type AmqpMessageId." + }, + { + "code": "java.method.returnTypeChanged", + "old": "method java.lang.String com.azure.core.amqp.models.AmqpMessageProperties::getMessageId()", + "new": "method com.azure.core.amqp.models.AmqpMessageId com.azure.core.amqp.models.AmqpMessageProperties::getMessageId()", + "justification": "New return type." + }, + { + "code": "java.method.returnTypeChanged", + "old": "method java.lang.String com.azure.core.amqp.models.AmqpMessageProperties::getReplyTo()", + "new": "method com.azure.core.amqp.models.AmqpAddress com.azure.core.amqp.models.AmqpMessageProperties::getReplyTo()", + "justification": "New return type." + }, + { + "code": "java.method.returnTypeChanged", + "old": "method java.lang.String com.azure.core.amqp.models.AmqpMessageProperties::getTo()", + "new": "method com.azure.core.amqp.models.AmqpAddress com.azure.core.amqp.models.AmqpMessageProperties::getTo()", + "justification": "New return type." + }, + { + "code": "java.method.parameterTypeChanged", + "old": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setCorrelationId(===java.lang.String===)", + "new": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setCorrelationId(===com.azure.core.amqp.models.AmqpMessageId===)", + "justification": "Introduced new type AmqpMessageId." + }, + { + "code": "java.method.parameterTypeChanged", + "old": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setMessageId(===java.lang.String===)", + "new": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setMessageId(===com.azure.core.amqp.models.AmqpMessageId===)", + "justification":"Introduced new type AmqpMessageId." + }, + { + "code": "java.method.parameterTypeChanged", + "old": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setReplyTo(===java.lang.String===)", + "new": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setReplyTo(===com.azure.core.amqp.models.AmqpAddress===)", + "justification": "Introduced new type AmqpAddress." + }, + { + "code": "java.method.parameterTypeChanged", + "old": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setTo(===java.lang.String===)", + "new": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setTo(===com.azure.core.amqp.models.AmqpAddress===)", + "justification": "Introduced new type AmqpAddress." } ] } diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java new file mode 100644 index 000000000000..4497573bb6b1 --- /dev/null +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java @@ -0,0 +1,62 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.amqp.models; + +import java.util.Objects; + +/** + * This represents amqp address information. + */ +public class AmqpAddress { + + private final String address; + + /** + * Creates the {@link AmqpAddress} with given {@code address}. + * + * @param address to use. + */ + public AmqpAddress(String address) { + this.address = Objects.requireNonNull(address, "'address' cannot be null."); + } + + @Override + public int hashCode() { + return address.hashCode(); + } + + /** + * {@inheritDoc} + */ + @Override + public boolean equals(Object other) { + if (other == null) { + return false; + } + + if (this.getClass() != other.getClass()) { + return false; + } + + if (this == other) { + return true; + } + + if (!address.equals(((AmqpAddress) other).toString())) { + return false; + } + + return true; + } + + /** + * Gets string representation of the address. + * + * @return string representation of the address. + */ + @Override + public String toString() { + return this.address; + } +} diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index a07e93012318..9ecc161bd97f 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -3,6 +3,8 @@ package com.azure.core.amqp.models; +import com.azure.core.util.logging.ClientLogger; + import java.util.HashMap; import java.util.Map; import java.util.Objects; @@ -14,6 +16,7 @@ * Amqp Message Format. */ public final class AmqpAnnotatedMessage { + private final ClientLogger logger = new ClientLogger(AmqpAnnotatedMessage.class); private final AmqpMessageBody amqpMessageBody; private final Map applicationProperties; private final Map deliveryAnnotations; @@ -49,7 +52,22 @@ public AmqpAnnotatedMessage(AmqpMessageBody body) { */ public AmqpAnnotatedMessage(AmqpAnnotatedMessage message) { Objects.requireNonNull(message, "'message' cannot be null."); - amqpMessageBody = Objects.requireNonNull(message.getBody(), "'message.body' cannot be null."); + Objects.requireNonNull(message.getBody(), "'message.body' cannot be null."); + + AmqpBodyType bodyType = message.getBody().getBodyType(); + switch (bodyType) { + case DATA: + amqpMessageBody = AmqpMessageBody.fromData(message.getBody().getData()); + break; + case SEQUENCE: + case VALUE: + throw logger.logExceptionAsError(new UnsupportedOperationException("Body type not supported yet " + + bodyType.toString())); + default: + throw logger.logExceptionAsError(new IllegalStateException("Body type not valid " + + bodyType.toString())); + } + applicationProperties = new HashMap<>(message.getApplicationProperties()); deliveryAnnotations = new HashMap<>(message.getDeliveryAnnotations()); messageAnnotations = new HashMap<>(message.getMessageAnnotations()); @@ -58,6 +76,7 @@ public AmqpAnnotatedMessage(AmqpAnnotatedMessage message) { properties = new AmqpMessageProperties(message.getProperties()); } + /** * Gets the {@link Map} of application properties. * diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpBodyType.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpBodyType.java index 80c66af35608..39b398729a0a 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpBodyType.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpBodyType.java @@ -8,16 +8,16 @@ */ public enum AmqpBodyType { /** - * Represent Amqp Data type + * Message content is byte array, equivalent to AMQP Data. */ DATA, /** - * Represent Amqp Value type + * Message content is a single object, equivalent to AMQP Value. The object must be of a type supported by AMQP. */ VALUE, /** - * Represent Amqp Sequence type + * Message content is a list of objects, equivalent to AMQP Sequence. Each object must be of a type supported + * by AMQP. */ - SEQUENCE; - + SEQUENCE } diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpDataBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpDataBody.java deleted file mode 100644 index c26eb2f1e09a..000000000000 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpDataBody.java +++ /dev/null @@ -1,41 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.azure.core.amqp.models; - -import com.azure.core.util.IterableStream; - -import java.util.Objects; - -/** - * This is amqp message body which represents {@link AmqpBodyType#DATA} type. - */ -public final class AmqpDataBody implements AmqpMessageBody { - private final IterableStream data; - - /** - * Creates instance of {@link AmqpDataBody} with given {@link Iterable} of byte array. - * - * @param data to be set on amqp body. - * - * @throws NullPointerException if {@code data} is null. - */ - public AmqpDataBody(Iterable data) { - Objects.requireNonNull(data, "'data' cannot be null."); - this.data = new IterableStream<>(data); - } - - @Override - public AmqpBodyType getBodyType() { - return AmqpBodyType.DATA; - } - - /** - * Gets byte array set on this {@link AmqpDataBody}. - * - * @return data set on {@link AmqpDataBody}. - */ - public IterableStream getData() { - return data; - } -} diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index a984fb566720..89e8e6cd02fd 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -3,14 +3,97 @@ package com.azure.core.amqp.models; +import com.azure.core.util.logging.ClientLogger; + +import java.util.Collections; +import java.util.List; +import java.util.Objects; + /** - * Interface representing Amqp Message Body. + * This class encapsulates the body of a message. The {@link AmqpBodyType} map to AMQP specification message body types. + * Current implementation only support {@link AmqpBodyType#DATA DATA}. Other types will be supported in future releases. + * Client should test for {@link AmqpBodyType} before calling corresponding get method. + * Get methods not corresponding to the type of the body throw exception. + * + * @see AmqpBodyType */ -public interface AmqpMessageBody { +public final class AmqpMessageBody { + private static final ClientLogger LOGGER = new ClientLogger(AmqpMessageBody.class); + private AmqpBodyType bodyType; + + private byte[] data; + + /** + * Creates instance of {@link AmqpMessageBody} with given {@link Iterable} of byte array. Please note that this + * version of the SDK supports only one element in given {@link Iterable}. + * + * @param data used to create another instance of {@link AmqpMessageBody}. + * + * @return AmqpMessageBody Newly created instance. + * + * @throws NullPointerException if {@code data} is null. + * + * @throws IllegalArgumentException if size of 'data' is zero or greater than one. + */ + public static AmqpMessageBody fromData(Iterable data) { + Objects.requireNonNull(data, "'data' cannot be null."); + byte[] payload = null; + for (byte[] binaryData : data) { + if (payload != null) { + throw LOGGER.logExceptionAsError(new IllegalArgumentException( + "Only one instance of byte array is allowed in 'data'.")); + } + payload = binaryData; + } + + if (payload == null) { + throw LOGGER.logExceptionAsError(new IllegalArgumentException("'data' can not be empty.")); + } + + AmqpMessageBody body = new AmqpMessageBody(); + body.bodyType = AmqpBodyType.DATA; + body.data = payload; + return body; + } + + /** + * Creates instance of {@link AmqpMessageBody} with given byte array. + * + * @param data used to create another instance of {@link AmqpMessageBody}. + * + * @return AmqpMessageBody Newly created instance. + * + * @throws NullPointerException if {@code data} is null. + */ + public static AmqpMessageBody fromData(byte[] data) { + Objects.requireNonNull(data, "'data' cannot be null."); + AmqpMessageBody body = new AmqpMessageBody(); + body.bodyType = AmqpBodyType.DATA; + body.data = data; + return body; + } + + /** + * Gets the {@link AmqpBodyType} of the message. + * + * @return AmqpBodyType type of the message. + */ + public AmqpBodyType getBodyType() { + return bodyType; + } + /** - * Type representing various supported amqp body types. + * Gets an immutable list containing only first byte array set on this {@link AmqpMessageBody}. + * + * @return data set on {@link AmqpMessageBody}. * - * @return The {@link AmqpBodyType}. + * @throws IllegalArgumentException If {@link AmqpBodyType} is not {@link AmqpBodyType#DATA DATA}. */ - AmqpBodyType getBodyType(); + public List getData() { + if (bodyType != AmqpBodyType.DATA) { + throw LOGGER.logExceptionAsError(new IllegalArgumentException(String.format("Can not return data for a " + + "message which is of type %s.", getBodyType().toString()))); + } + return Collections.singletonList(data); + } } diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java new file mode 100644 index 000000000000..6bf0a6ddee04 --- /dev/null +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java @@ -0,0 +1,64 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.amqp.models; + +import java.util.Objects; + +/** + * This represents Message id in various forms. + * + * @see + * Amqp Message Id. + */ +public final class AmqpMessageId { + private final String messageId; + + /** + * Creates the {@link AmqpMessageId} with given {@code messageId}. + * + * @param messageId representing id of the message. + */ + public AmqpMessageId(String messageId) { + this.messageId = Objects.requireNonNull(messageId, "'messageId' cannot be null."); + } + + @Override + public int hashCode() { + return messageId.hashCode(); + } + + /** + * {@inheritDoc} + */ + @Override + public boolean equals(Object other) { + if (other == null) { + return false; + } + + if (this.getClass() != other.getClass()) { + return false; + } + + if (this == other) { + return true; + } + + if (!messageId.equals(((AmqpMessageId) other).toString())) { + return false; + } + + return true; + } + + /** + * Gets string representation of the message id. + * + * @return string representation of the message id. + */ + @Override + public String toString() { + return this.messageId; + } +} diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java index 29e7595e5b41..77c248e407e9 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java @@ -21,14 +21,14 @@ public class AmqpMessageProperties { private OffsetDateTime absoluteExpiryTime; private String contentEncoding; private String contentType; - private String correlationId; + private AmqpMessageId correlationId; private OffsetDateTime creationTime; private String groupId; private Long groupSequence; - private String messageId; + private AmqpMessageId messageId; private String replyToGroupId; - private String replyTo; - private String to; + private AmqpAddress replyTo; + private AmqpAddress to; private String subject; private byte[] userId; @@ -124,7 +124,7 @@ public AmqpMessageProperties setContentType(String contentType) { * * @return the {@code correlationId} value. */ - public String getCorrelationId() { + public AmqpMessageId getCorrelationId() { return correlationId; } @@ -135,7 +135,7 @@ public String getCorrelationId() { * * @return updated {@link AmqpMessageProperties} object. */ - public AmqpMessageProperties setCorrelationId(String correlationId) { + public AmqpMessageProperties setCorrelationId(AmqpMessageId correlationId) { this.correlationId = correlationId; return this; } @@ -208,7 +208,7 @@ public AmqpMessageProperties setGroupSequence(Long groupSequence) { * * @return the {@code messageId} value. */ - public String getMessageId() { + public AmqpMessageId getMessageId() { return messageId; } @@ -219,7 +219,7 @@ public String getMessageId() { * * @return updated {@link AmqpMessageProperties} object. */ - public AmqpMessageProperties setMessageId(String messageId) { + public AmqpMessageProperties setMessageId(AmqpMessageId messageId) { this.messageId = messageId; return this; } @@ -229,7 +229,7 @@ public AmqpMessageProperties setMessageId(String messageId) { * * @return The {@code replyTo} value. */ - public String getReplyTo() { + public AmqpAddress getReplyTo() { return replyTo; } @@ -240,7 +240,7 @@ public String getReplyTo() { * * @return updated {@link AmqpMessageProperties} object. */ - public AmqpMessageProperties setReplyTo(String replyTo) { + public AmqpMessageProperties setReplyTo(AmqpAddress replyTo) { this.replyTo = replyTo; return this; } @@ -292,7 +292,7 @@ public AmqpMessageProperties setSubject(String subject) { * * @return the {@code to} value. */ - public String getTo() { + public AmqpAddress getTo() { return to; } @@ -303,7 +303,7 @@ public String getTo() { * * @return updated {@link AmqpMessageProperties} object. */ - public AmqpMessageProperties setTo(String to) { + public AmqpMessageProperties setTo(AmqpAddress to) { this.to = to; return this; } diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java index ce6f84bba3ea..f0e46f12b03d 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java @@ -39,8 +39,8 @@ public void copyConstructorTest() { List expectedBinaryData = new ArrayList<>(); expectedBinaryData.add(CONTENTS_BYTES); - final AmqpDataBody amqpDataBody = new AmqpDataBody(expectedBinaryData); - final AmqpAnnotatedMessage expected = new AmqpAnnotatedMessage(amqpDataBody); + final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(expectedBinaryData); + final AmqpAnnotatedMessage expected = new AmqpAnnotatedMessage(amqpMessageBody); final Map expectedMessageAnnotations = expected.getMessageAnnotations(); expectedMessageAnnotations.put("ma-1", "ma-value1"); @@ -57,10 +57,10 @@ public void copyConstructorTest() { expectedMessageProperties.setGroupSequence(2L); expectedMessageProperties.setContentEncoding("content-enc"); expectedMessageProperties.setReplyToGroupId("a"); - expectedMessageProperties.setReplyTo("b"); - expectedMessageProperties.setCorrelationId("c"); + expectedMessageProperties.setReplyTo(new AmqpAddress("b")); + expectedMessageProperties.setCorrelationId(new AmqpMessageId("c")); expectedMessageProperties.setSubject("d"); - expectedMessageProperties.setMessageId("id"); + expectedMessageProperties.setMessageId(new AmqpMessageId("id")); final AmqpMessageHeader expectedMessageHeader = expected.getHeader(); expectedMessageHeader.setDeliveryCount(5L); @@ -113,10 +113,10 @@ public void copyConstructorTest() { public void constructorValidValues() { // Arrange final List expectedBinaryData = Collections.singletonList(CONTENTS_BYTES); - final AmqpDataBody amqpDataBody = new AmqpDataBody(expectedBinaryData); + final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(expectedBinaryData); // Act - final AmqpAnnotatedMessage actual = new AmqpAnnotatedMessage(amqpDataBody); + final AmqpAnnotatedMessage actual = new AmqpAnnotatedMessage(amqpMessageBody); // Assert assertMessageCreation(AmqpBodyType.DATA, expectedBinaryData.size(), actual); @@ -129,8 +129,8 @@ public void constructorValidValues() { public void constructorAmqpValidValues() { // Arrange final List expectedBinaryData = Collections.singletonList(CONTENTS_BYTES); - final AmqpDataBody amqpDataBody = new AmqpDataBody(expectedBinaryData); - final AmqpAnnotatedMessage expected = new AmqpAnnotatedMessage(amqpDataBody); + final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(expectedBinaryData); + final AmqpAnnotatedMessage expected = new AmqpAnnotatedMessage(amqpMessageBody); // Act final AmqpAnnotatedMessage actual = new AmqpAnnotatedMessage(expected); @@ -145,7 +145,7 @@ public void constructorAmqpValidValues() { @Test public void constructorNullValidValues() { // Arrange - final AmqpDataBody body = null; + final AmqpMessageBody body = null; // Act & Assert Assertions.assertThrows(NullPointerException.class, () -> new AmqpAnnotatedMessage(body)); @@ -170,7 +170,7 @@ private void assertMessageBody(int expectedMessageSize, byte[] expectedbody, Amq final AmqpBodyType actualType = actual.getBody().getBodyType(); switch (actualType) { case DATA: - List actualData = ((AmqpDataBody) actual.getBody()).getData().stream().collect(Collectors.toList()); + List actualData = actual.getBody().getData().stream().collect(Collectors.toList()); assertEquals(expectedMessageSize, actualData.size()); assertArrayEquals(expectedbody, actualData.get(0)); break; diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpDataBodyTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java similarity index 83% rename from sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpDataBodyTest.java rename to sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java index e5d196c1c1d2..b9cce90b3ea9 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpDataBodyTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java @@ -14,9 +14,9 @@ import static org.junit.jupiter.api.Assertions.assertEquals; /** - * Test for {@link AmqpDataBody}. + * Test for {@link AmqpMessageBody}. */ -public class AmqpDataBodyTest { +public class AmqpMessageBodyTest { /** * Verifies we correctly set values via constructor for {@link AmqpAnnotatedMessage}. @@ -29,7 +29,7 @@ public void constructorValidValues() { expectedDataList.add("some data 2".getBytes()); // Act - final AmqpDataBody actual = new AmqpDataBody(expectedDataList); + final AmqpMessageBody actual = AmqpMessageBody.fromData(expectedDataList); // Assert assertEquals(AmqpBodyType.DATA, actual.getBodyType()); @@ -41,7 +41,7 @@ public void constructorValidValues() { } /** - * Verifies {@link AmqpDataBody} constructor for null values. + * Verifies {@link AmqpMessageBody} constructor for null values. */ @Test public void constructorNullValidValues() { @@ -49,6 +49,6 @@ public void constructorNullValidValues() { final List listBinaryData = null; // Act & Assert - Assertions.assertThrows(NullPointerException.class, () -> new AmqpDataBody(listBinaryData)); + Assertions.assertThrows(NullPointerException.class, () -> AmqpMessageBody.fromData(listBinaryData)); } } diff --git a/sdk/servicebus/azure-messaging-servicebus/pom.xml b/sdk/servicebus/azure-messaging-servicebus/pom.xml index 8852a04531fd..42626aa360d3 100644 --- a/sdk/servicebus/azure-messaging-servicebus/pom.xml +++ b/sdk/servicebus/azure-messaging-servicebus/pom.xml @@ -52,7 +52,7 @@ com.azure azure-core-amqp - 1.7.0-beta.1 + 1.7.0-beta.2 com.azure diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java index 5227285e5a68..6740265c2fad 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java @@ -4,9 +4,11 @@ package com.azure.messaging.servicebus; import com.azure.core.amqp.AmqpMessageConstant; +import com.azure.core.amqp.models.AmqpAddress; import com.azure.core.amqp.models.AmqpAnnotatedMessage; import com.azure.core.amqp.models.AmqpBodyType; -import com.azure.core.amqp.models.AmqpDataBody; +import com.azure.core.amqp.models.AmqpMessageBody; +import com.azure.core.amqp.models.AmqpMessageId; import com.azure.core.util.Context; import com.azure.core.util.logging.ClientLogger; import com.azure.core.experimental.util.BinaryData; @@ -14,7 +16,6 @@ import java.time.Duration; import java.time.OffsetDateTime; import java.time.ZoneOffset; -import java.util.Collections; import java.util.Map; import java.util.Objects; import java.util.Optional; @@ -93,8 +94,7 @@ public ServiceBusMessage(String body) { public ServiceBusMessage(BinaryData body) { Objects.requireNonNull(body, "'body' cannot be null."); this.context = Context.NONE; - this.amqpAnnotatedMessage = new AmqpAnnotatedMessage( - new AmqpDataBody(Collections.singletonList(body.toBytes()))); + this.amqpAnnotatedMessage = new AmqpAnnotatedMessage(AmqpMessageBody.fromData(body.toBytes())); } /** @@ -161,8 +161,7 @@ public BinaryData getBody() { final AmqpBodyType type = amqpAnnotatedMessage.getBody().getBodyType(); switch (type) { case DATA: - Optional byteArrayData = ((AmqpDataBody) amqpAnnotatedMessage.getBody()).getData().stream() - .findFirst(); + Optional byteArrayData = amqpAnnotatedMessage.getBody().getData().stream().findFirst(); final byte[] bytes; if (byteArrayData.isPresent()) { @@ -215,7 +214,12 @@ public ServiceBusMessage setContentType(String contentType) { * Routing and Correlation */ public String getCorrelationId() { - return amqpAnnotatedMessage.getProperties().getCorrelationId(); + String correlationId = null; + AmqpMessageId amqpCorrelationId = amqpAnnotatedMessage.getProperties().getCorrelationId(); + if (amqpCorrelationId != null) { + correlationId = amqpCorrelationId.toString(); + } + return correlationId; } /** @@ -227,7 +231,11 @@ public String getCorrelationId() { * @see #getCorrelationId() */ public ServiceBusMessage setCorrelationId(String correlationId) { - amqpAnnotatedMessage.getProperties().setCorrelationId(correlationId); + AmqpMessageId id = null; + if (correlationId != null) { + id = new AmqpMessageId(correlationId); + } + amqpAnnotatedMessage.getProperties().setCorrelationId(id); return this; } @@ -256,7 +264,12 @@ public ServiceBusMessage setSubject(String subject) { * @return Id of the {@link ServiceBusMessage}. */ public String getMessageId() { - return amqpAnnotatedMessage.getProperties().getMessageId(); + String messageId = null; + AmqpMessageId amqpMessageId = amqpAnnotatedMessage.getProperties().getMessageId(); + if (amqpMessageId != null) { + messageId = amqpMessageId.toString(); + } + return messageId; } /** @@ -267,7 +280,11 @@ public String getMessageId() { * @return The updated {@link ServiceBusMessage}. */ public ServiceBusMessage setMessageId(String messageId) { - amqpAnnotatedMessage.getProperties().setMessageId(messageId); + AmqpMessageId id = null; + if (messageId != null) { + id = new AmqpMessageId(messageId); + } + amqpAnnotatedMessage.getProperties().setMessageId(id); return this; } @@ -313,7 +330,12 @@ public ServiceBusMessage setPartitionKey(String partitionKey) { * Routing and Correlation */ public String getReplyTo() { - return amqpAnnotatedMessage.getProperties().getReplyTo(); + String replyTo = null; + AmqpAddress amqpAddress = amqpAnnotatedMessage.getProperties().getReplyTo(); + if (amqpAddress != null) { + replyTo = amqpAddress.toString(); + } + return replyTo; } /** @@ -325,7 +347,11 @@ public String getReplyTo() { * @see #getReplyTo() */ public ServiceBusMessage setReplyTo(String replyTo) { - amqpAnnotatedMessage.getProperties().setReplyTo(replyTo); + AmqpAddress replyToAddress = null; + if (replyTo != null) { + replyToAddress = new AmqpAddress(replyTo); + } + amqpAnnotatedMessage.getProperties().setReplyTo(replyToAddress); return this; } @@ -335,7 +361,12 @@ public ServiceBusMessage setReplyTo(String replyTo) { * @return "To" property value of this message */ public String getTo() { - return amqpAnnotatedMessage.getProperties().getTo(); + String to = null; + AmqpAddress amqpAddress = amqpAnnotatedMessage.getProperties().getTo(); + if (amqpAddress != null) { + to = amqpAddress.toString(); + } + return to; } /** @@ -351,7 +382,11 @@ public String getTo() { * @return The updated {@link ServiceBusMessage}. */ public ServiceBusMessage setTo(String to) { - amqpAnnotatedMessage.getProperties().setTo(to); + AmqpAddress toAddress = null; + if (to != null) { + toAddress = new AmqpAddress(to); + } + amqpAnnotatedMessage.getProperties().setTo(toAddress); return this; } diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessageSerializer.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessageSerializer.java index 5edc881140ca..a49dc1dc00d3 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessageSerializer.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessageSerializer.java @@ -9,8 +9,10 @@ import com.azure.core.amqp.exception.AmqpResponseCode; import com.azure.core.amqp.implementation.MessageSerializer; import com.azure.core.amqp.implementation.RequestResponseUtils; +import com.azure.core.amqp.models.AmqpAddress; import com.azure.core.amqp.models.AmqpAnnotatedMessage; import com.azure.core.amqp.models.AmqpMessageHeader; +import com.azure.core.amqp.models.AmqpMessageId; import com.azure.core.amqp.models.AmqpMessageProperties; import com.azure.core.experimental.util.BinaryData; import com.azure.core.util.logging.ClientLogger; @@ -387,21 +389,27 @@ private ServiceBusReceivedMessage deserializeMessage(Message amqpMessage) { // Properties final AmqpMessageProperties brokeredProperties = brokeredAmqpAnnotatedMessage.getProperties(); brokeredProperties.setReplyToGroupId(amqpMessage.getReplyToGroupId()); - brokeredProperties.setReplyTo(amqpMessage.getReplyTo()); + final String replyTo = amqpMessage.getReplyTo(); + if (replyTo != null) { + brokeredProperties.setReplyTo(new AmqpAddress(amqpMessage.getReplyTo())); + } final Object messageId = amqpMessage.getMessageId(); if (messageId != null) { - brokeredProperties.setMessageId(messageId.toString()); + brokeredProperties.setMessageId(new AmqpMessageId(messageId.toString())); } brokeredProperties.setContentType(amqpMessage.getContentType()); final Object correlationId = amqpMessage.getCorrelationId(); if (correlationId != null) { - brokeredProperties.setCorrelationId(correlationId.toString()); + brokeredProperties.setCorrelationId(new AmqpMessageId(correlationId.toString())); } final Properties amqpProperties = amqpMessage.getProperties(); if (amqpProperties != null) { - brokeredProperties.setTo(amqpProperties.getTo()); + final String to = amqpProperties.getTo(); + if (to != null) { + brokeredProperties.setTo(new AmqpAddress(amqpProperties.getTo())); + } if (amqpProperties.getAbsoluteExpiryTime() != null) { brokeredProperties.setAbsoluteExpiryTime(amqpProperties.getAbsoluteExpiryTime().toInstant() diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java index 74c6df584c82..9b983fceb045 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java @@ -4,9 +4,11 @@ package com.azure.messaging.servicebus; import com.azure.core.amqp.AmqpMessageConstant; +import com.azure.core.amqp.models.AmqpAddress; import com.azure.core.amqp.models.AmqpAnnotatedMessage; import com.azure.core.amqp.models.AmqpBodyType; -import com.azure.core.amqp.models.AmqpDataBody; +import com.azure.core.amqp.models.AmqpMessageBody; +import com.azure.core.amqp.models.AmqpMessageId; import com.azure.core.experimental.util.BinaryData; import com.azure.core.util.logging.ClientLogger; import com.azure.messaging.servicebus.models.ReceiveMode; @@ -14,7 +16,6 @@ import java.time.Duration; import java.time.OffsetDateTime; import java.time.ZoneOffset; -import java.util.Collections; import java.util.Date; import java.util.Map; import java.util.Objects; @@ -43,7 +44,7 @@ public final class ServiceBusReceivedMessage { ServiceBusReceivedMessage(BinaryData body) { Objects.requireNonNull(body, "'body' cannot be null."); - amqpAnnotatedMessage = new AmqpAnnotatedMessage(new AmqpDataBody(Collections.singletonList(body.toBytes()))); + amqpAnnotatedMessage = new AmqpAnnotatedMessage(AmqpMessageBody.fromData(body.toBytes())); } /** @@ -75,8 +76,7 @@ public BinaryData getBody() { final AmqpBodyType bodyType = amqpAnnotatedMessage.getBody().getBodyType(); switch (bodyType) { case DATA: - return BinaryData.fromBytes(((AmqpDataBody) amqpAnnotatedMessage.getBody()) - .getData().stream().findFirst().get()); + return BinaryData.fromBytes(amqpAnnotatedMessage.getBody().getData().stream().findFirst().get()); case SEQUENCE: case VALUE: throw logger.logExceptionAsError(new UnsupportedOperationException("Body type not supported yet " @@ -118,7 +118,12 @@ public String getContentType() { * Routing and Correlation */ public String getCorrelationId() { - return amqpAnnotatedMessage.getProperties().getCorrelationId(); + String correlationId = null; + AmqpMessageId amqpCorrelationId = amqpAnnotatedMessage.getProperties().getCorrelationId(); + if (amqpCorrelationId != null) { + correlationId = amqpCorrelationId.toString(); + } + return correlationId; } /** @@ -280,7 +285,12 @@ public OffsetDateTime getLockedUntil() { * @return Id of the {@link ServiceBusReceivedMessage}. */ public String getMessageId() { - return amqpAnnotatedMessage.getProperties().getMessageId(); + String messageId = null; + AmqpMessageId amqpMessageId = amqpAnnotatedMessage.getProperties().getMessageId(); + if (amqpMessageId != null) { + messageId = amqpMessageId.toString(); + } + return messageId; } /** @@ -327,7 +337,12 @@ public Map getApplicationProperties() { * Routing and Correlation */ public String getReplyTo() { - return amqpAnnotatedMessage.getProperties().getReplyTo(); + String replyTo = null; + AmqpAddress amqpAddress = amqpAnnotatedMessage.getProperties().getReplyTo(); + if (amqpAddress != null) { + replyTo = amqpAddress.toString(); + } + return replyTo; } /** @@ -414,7 +429,12 @@ public Duration getTimeToLive() { * @return "To" property value of this message */ public String getTo() { - return amqpAnnotatedMessage.getProperties().getTo(); + String to = null; + AmqpAddress amqpAddress = amqpAnnotatedMessage.getProperties().getTo(); + if (amqpAddress != null) { + to = amqpAddress.toString(); + } + return to; } /** @@ -450,7 +470,11 @@ boolean isSettled() { * @see #getCorrelationId() */ void setCorrelationId(String correlationId) { - amqpAnnotatedMessage.getProperties().setCorrelationId(correlationId); + AmqpMessageId id = null; + if (correlationId != null) { + id = new AmqpMessageId(correlationId); + } + amqpAnnotatedMessage.getProperties().setCorrelationId(id); } /** @@ -554,7 +578,11 @@ void setLockedUntil(OffsetDateTime lockedUntil) { * @param messageId to be set. */ void setMessageId(String messageId) { - amqpAnnotatedMessage.getProperties().setMessageId(messageId); + AmqpMessageId id = null; + if (messageId != null) { + id = new AmqpMessageId(messageId); + } + amqpAnnotatedMessage.getProperties().setMessageId(id); } /** @@ -625,7 +653,12 @@ void setTimeToLive(Duration timeToLive) { * @see #getReplyTo() */ void setReplyTo(String replyTo) { - amqpAnnotatedMessage.getProperties().setReplyTo(replyTo); + AmqpAddress replyToAddress = null; + if (replyTo != null) { + replyToAddress = new AmqpAddress(replyTo); + } + amqpAnnotatedMessage.getProperties().setReplyTo(replyToAddress); + } /** @@ -648,7 +681,11 @@ void setReplyToSessionId(String replyToSessionId) { * @param to To property value of this message */ void setTo(String to) { - amqpAnnotatedMessage.getProperties().setTo(to); + AmqpAddress toAddress = null; + if (to != null) { + toAddress = new AmqpAddress(to); + } + amqpAnnotatedMessage.getProperties().setTo(toAddress); } /** diff --git a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusReceiverAsyncClientIntegrationTest.java b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusReceiverAsyncClientIntegrationTest.java index 75e2405380ff..86bca3652b76 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusReceiverAsyncClientIntegrationTest.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusReceiverAsyncClientIntegrationTest.java @@ -3,9 +3,11 @@ package com.azure.messaging.servicebus; +import com.azure.core.amqp.models.AmqpAddress; import com.azure.core.amqp.models.AmqpAnnotatedMessage; -import com.azure.core.amqp.models.AmqpDataBody; +import com.azure.core.amqp.models.AmqpMessageBody; import com.azure.core.amqp.models.AmqpMessageHeader; +import com.azure.core.amqp.models.AmqpMessageId; import com.azure.core.amqp.models.AmqpMessageProperties; import com.azure.core.util.logging.ClientLogger; import com.azure.messaging.servicebus.implementation.DispositionStatus; @@ -1013,13 +1015,13 @@ void receiveAndValidateProperties(MessagingEntityType entityType) { final String messageId = UUID.randomUUID().toString(); final AmqpAnnotatedMessage expectedAmqpProperties = new AmqpAnnotatedMessage( - new AmqpDataBody(Collections.singletonList(CONTENTS_BYTES))); + AmqpMessageBody.fromData(CONTENTS_BYTES)); expectedAmqpProperties.getProperties().setSubject(subject); expectedAmqpProperties.getProperties().setReplyToGroupId("r-gid"); - expectedAmqpProperties.getProperties().setReplyTo("reply-to"); + expectedAmqpProperties.getProperties().setReplyTo(new AmqpAddress("reply-to")); expectedAmqpProperties.getProperties().setContentType("content-type"); - expectedAmqpProperties.getProperties().setCorrelationId("correlation-id"); - expectedAmqpProperties.getProperties().setTo("to"); + expectedAmqpProperties.getProperties().setCorrelationId(new AmqpMessageId("correlation-id")); + expectedAmqpProperties.getProperties().setTo(new AmqpAddress("to")); expectedAmqpProperties.getProperties().setAbsoluteExpiryTime(OffsetDateTime.now().plusSeconds(60)); expectedAmqpProperties.getProperties().setUserId("user-id-1".getBytes()); expectedAmqpProperties.getProperties().setContentEncoding("string"); From c4c6b2535ed28b53274d8e23ce5c14782a47d9e3 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 10 Nov 2020 23:54:50 -0800 Subject: [PATCH 02/23] Adding test --- .../core/amqp/models/AmqpAddressTest.java | 52 +++++++++++++++++++ .../core/amqp/models/AmqpMessageBodyTest.java | 15 +++++- .../core/amqp/models/AmqpMessageIdTest.java | 49 +++++++++++++++++ 3 files changed, 115 insertions(+), 1 deletion(-) create mode 100644 sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAddressTest.java create mode 100644 sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAddressTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAddressTest.java new file mode 100644 index 000000000000..bc0512196ff2 --- /dev/null +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAddressTest.java @@ -0,0 +1,52 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.amqp.models; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.List; + +/** + * Test for {@link AmqpAddress}. + */ +public class AmqpAddressTest { + /** + * Verifies {@link AmqpAddress} constructor for null values. + */ + @Test + public void constructorNullValue() { + // Arrange + final List listBinaryData = null; + + // Act & Assert + Assertions.assertThrows(NullPointerException.class, () -> new AmqpAddress(null)); + } + + /** + * Verifies {@link AmqpAddress} with same value makes instances equal. + */ + @Test + public void equalityTest() { + // Arrange + final AmqpAddress address1 = new AmqpAddress("a"); + final AmqpAddress address2 = new AmqpAddress("a"); + + // Act & Assert + Assertions.assertEquals(address1, address2); + } + + /** + * Verifies {@link AmqpAddress} , two different values are not equal. + */ + @Test + public void nonEqualityTest() { + // Arrange + final AmqpAddress address1 = new AmqpAddress("a"); + final AmqpAddress address2 = new AmqpAddress("a1"); + + // Act & Assert + Assertions.assertNotEquals(address1, address2); + } +} diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java index b9cce90b3ea9..372d4fbcabcf 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java @@ -26,7 +26,6 @@ public void constructorValidValues() { // Arrange final List expectedDataList = new ArrayList<>(); expectedDataList.add("some data 1".getBytes()); - expectedDataList.add("some data 2".getBytes()); // Act final AmqpMessageBody actual = AmqpMessageBody.fromData(expectedDataList); @@ -51,4 +50,18 @@ public void constructorNullValidValues() { // Act & Assert Assertions.assertThrows(NullPointerException.class, () -> AmqpMessageBody.fromData(listBinaryData)); } + + /** + * Verifies that only one byte array is alllowed in {@link AmqpMessageBody}. + */ + @Test + public void constructorSupportOneByteArray() { + // Arrange + final List expectedDataList = new ArrayList<>(); + expectedDataList.add("some data 1".getBytes()); + expectedDataList.add("some data 2".getBytes()); + + // Act & Assert + Assertions.assertThrows(IllegalArgumentException.class, () -> AmqpMessageBody.fromData(expectedDataList)); + } } diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java new file mode 100644 index 000000000000..cad24cf29ce1 --- /dev/null +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java @@ -0,0 +1,49 @@ +package com.azure.core.amqp.models; + +import org.junit.jupiter.api.Assertions; +import org.junit.jupiter.api.Test; + +import java.util.List; + +/** + * Test for {@link AmqpMessageId}. + */ +public class AmqpMessageIdTest { + /** + * Verifies {@link AmqpMessageId} constructor for null values. + */ + @Test + public void constructorNullValue() { + // Arrange + final List listBinaryData = null; + + // Act & Assert + Assertions.assertThrows(NullPointerException.class, () -> new AmqpMessageId(null)); + } + + /** + * Verifies {@link AmqpMessageId} with same value makes instances equal. + */ + @Test + public void equalityTest() { + // Arrange + final AmqpMessageId id1 = new AmqpMessageId("a"); + final AmqpMessageId id2 = new AmqpMessageId("a"); + + // Act & Assert + Assertions.assertEquals(id1, id2); + } + + /** + * Verifies {@link AmqpMessageId} , two different values are not equal. + */ + @Test + public void nonEqualityTest() { + // Arrange + final AmqpMessageId id1 = new AmqpMessageId("a"); + final AmqpMessageId id2 = new AmqpMessageId("a1"); + + // Act & Assert + Assertions.assertNotEquals(id1, id2); + } +} From 66b1fb18be26f239e1cacbfccd829dba9c6394d1 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Wed, 11 Nov 2020 17:45:06 -0800 Subject: [PATCH 03/23] Removing Iterable constructor --- .../azure/core/amqp/models/AmqpAddress.java | 2 +- .../amqp/models/AmqpAnnotatedMessage.java | 4 ++- .../core/amqp/models/AmqpMessageBody.java | 33 ------------------- 3 files changed, 4 insertions(+), 35 deletions(-) diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java index 4497573bb6b1..3aec41f824b0 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java @@ -8,7 +8,7 @@ /** * This represents amqp address information. */ -public class AmqpAddress { +public final class AmqpAddress { private final String address; diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index 9ecc161bd97f..1a80d348b7fd 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -49,10 +49,12 @@ public AmqpAnnotatedMessage(AmqpMessageBody body) { * @param message used to create another instance of {@link AmqpAnnotatedMessage}. * * @throws NullPointerException if {@code message} or {@link AmqpAnnotatedMessage#getBody() body} is null. + * @throws UnsupportedOperationException if {@link AmqpBodyType} is {@link AmqpBodyType#SEQUENCE} or + * {@link AmqpBodyType#VALUE}. + * @throws IllegalStateException for invalid {@link AmqpBodyType}. */ public AmqpAnnotatedMessage(AmqpAnnotatedMessage message) { Objects.requireNonNull(message, "'message' cannot be null."); - Objects.requireNonNull(message.getBody(), "'message.body' cannot be null."); AmqpBodyType bodyType = message.getBody().getBodyType(); switch (bodyType) { diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index 89e8e6cd02fd..f747046e4d38 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -23,39 +23,6 @@ public final class AmqpMessageBody { private byte[] data; - /** - * Creates instance of {@link AmqpMessageBody} with given {@link Iterable} of byte array. Please note that this - * version of the SDK supports only one element in given {@link Iterable}. - * - * @param data used to create another instance of {@link AmqpMessageBody}. - * - * @return AmqpMessageBody Newly created instance. - * - * @throws NullPointerException if {@code data} is null. - * - * @throws IllegalArgumentException if size of 'data' is zero or greater than one. - */ - public static AmqpMessageBody fromData(Iterable data) { - Objects.requireNonNull(data, "'data' cannot be null."); - byte[] payload = null; - for (byte[] binaryData : data) { - if (payload != null) { - throw LOGGER.logExceptionAsError(new IllegalArgumentException( - "Only one instance of byte array is allowed in 'data'.")); - } - payload = binaryData; - } - - if (payload == null) { - throw LOGGER.logExceptionAsError(new IllegalArgumentException("'data' can not be empty.")); - } - - AmqpMessageBody body = new AmqpMessageBody(); - body.bodyType = AmqpBodyType.DATA; - body.data = payload; - return body; - } - /** * Creates instance of {@link AmqpMessageBody} with given byte array. * From 888217b2ee268d62b4dfcb1024f63922eb9735fb Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Wed, 11 Nov 2020 23:56:03 -0800 Subject: [PATCH 04/23] Adding java doc --- .../amqp/models/AmqpAnnotatedMessage.java | 6 ++-- .../core/amqp/models/AmqpMessageBody.java | 24 +++++++++++--- ...mqpAnnotatedMessageJavaDocCodeSamples.java | 30 +++++++++++++++++ .../amqp/models/AmqpAnnotatedMessageTest.java | 32 +++++++------------ .../core/amqp/models/AmqpMessageBodyTest.java | 31 +++--------------- .../core/amqp/models/AmqpMessageIdTest.java | 3 ++ .../servicebus/ServiceBusMessageTest.java | 6 ++-- 7 files changed, 75 insertions(+), 57 deletions(-) create mode 100644 sdk/core/azure-core-amqp/src/sampels/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index 1a80d348b7fd..9356c719542e 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -59,7 +59,7 @@ public AmqpAnnotatedMessage(AmqpAnnotatedMessage message) { AmqpBodyType bodyType = message.getBody().getBodyType(); switch (bodyType) { case DATA: - amqpMessageBody = AmqpMessageBody.fromData(message.getBody().getData()); + amqpMessageBody = AmqpMessageBody.fromData(message.getBody().getData().get(0)); break; case SEQUENCE: case VALUE: @@ -90,7 +90,9 @@ public Map getApplicationProperties() { /** * Gets the {@link AmqpMessageBody} of an amqp message. - * + * Client should test for {@link AmqpBodyType} before calling corresponding get method on {@link AmqpMessageBody} + *

How to check for {@link AmqpBodyType}

+ * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @return the {@link AmqpMessageBody} object. */ public AmqpMessageBody getBody() { diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index f747046e4d38..33a6de04bede 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -12,8 +12,16 @@ /** * This class encapsulates the body of a message. The {@link AmqpBodyType} map to AMQP specification message body types. * Current implementation only support {@link AmqpBodyType#DATA DATA}. Other types will be supported in future releases. - * Client should test for {@link AmqpBodyType} before calling corresponding get method. - * Get methods not corresponding to the type of the body throw exception. + * Amqp message body types: + * + * Client should test for {@link AmqpBodyType} before calling corresponding get method.Get methods not corresponding + * to the type of the body throw exception. + *

How to check for {@link AmqpBodyType}

+ * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * * @see AmqpBodyType */ @@ -42,7 +50,8 @@ public static AmqpMessageBody fromData(byte[] data) { /** * Gets the {@link AmqpBodyType} of the message. - * + *

How to check for {@link AmqpBodyType}

+ * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @return AmqpBodyType type of the message. */ public AmqpBodyType getBodyType() { @@ -50,8 +59,13 @@ public AmqpBodyType getBodyType() { } /** - * Gets an immutable list containing only first byte array set on this {@link AmqpMessageBody}. - * + * Gets an immutable list containing only first byte array set on this {@link AmqpMessageBody}. The proton-j + * library used only support one byte array, so the returned list will have only one element. Look for future + * releases where we will support multiple byte array. + * Client should test for {@link AmqpBodyType} before calling corresponding get method.Get methods not corresponding + * to the type of the body throw exception. + *

How to check for {@link AmqpBodyType}

+ * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @return data set on {@link AmqpMessageBody}. * * @throws IllegalArgumentException If {@link AmqpBodyType} is not {@link AmqpBodyType#DATA DATA}. diff --git a/sdk/core/azure-core-amqp/src/sampels/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/sampels/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java new file mode 100644 index 000000000000..9af22d3ed4af --- /dev/null +++ b/sdk/core/azure-core-amqp/src/sampels/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java @@ -0,0 +1,30 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.amqp.models; + +/** + * Class contains sample code snippets that will be used in javadocs. + */ +public class AmqpAnnotatedMessageJavaDocCodeSamples { + /** + * Get message body from {@link AmqpAnnotatedMessage}. + */ + public void checkBodyType() { + AmqpAnnotatedMessage amqpAnnotatedMessage = null; + // BEGIN: com.azure.core.amqp.models.AmqpBodyType.checkBodyType + AmqpBodyType bodyType = amqpAnnotatedMessage.getBody().getBodyType(); + AmqpMessageBody messageBody = null; + switch (bodyType) { + case DATA: + messageBody = AmqpMessageBody.fromData(amqpAnnotatedMessage.getBody().getData().get(0)); + break; + case SEQUENCE: + case VALUE: + throw new RuntimeException("Body type not supported yet."); + default: + throw new RuntimeException("Body type not valid."); + } + // END: com.azure.core.amqp.models.AmqpBodyType.checkBodyType + } +} diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java index f0e46f12b03d..4fe024527b2c 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java @@ -9,11 +9,7 @@ import java.nio.charset.StandardCharsets; import java.time.Duration; -import java.util.ArrayList; -import java.util.Collections; -import java.util.List; import java.util.Map; -import java.util.stream.Collectors; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -36,10 +32,8 @@ public class AmqpAnnotatedMessageTest { public void copyConstructorTest() { // Arrange final int expectedBinaryDataSize = 1; - List expectedBinaryData = new ArrayList<>(); - expectedBinaryData.add(CONTENTS_BYTES); - final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(expectedBinaryData); + final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(CONTENTS_BYTES); final AmqpAnnotatedMessage expected = new AmqpAnnotatedMessage(amqpMessageBody); final Map expectedMessageAnnotations = expected.getMessageAnnotations(); expectedMessageAnnotations.put("ma-1", "ma-value1"); @@ -75,7 +69,6 @@ public void copyConstructorTest() { expectedApplicationProperties.put("ap-2", "ap-value2"); expectedFooter.remove("foo-1"); expected.getHeader().setDeliveryCount(Long.valueOf(100)); - expectedBinaryData = new ArrayList<>(); // Assert // Ensure the memory references are not same. @@ -103,7 +96,7 @@ public void copyConstructorTest() { assertEquals(expectedMessageHeader.getTimeToLive(), actual.getHeader().getTimeToLive()); assertEquals(expectedMessageHeader.getPriority(), actual.getHeader().getPriority()); - assertMessageBody(expectedBinaryDataSize, CONTENTS_BYTES, actual); + assertMessageBody(CONTENTS_BYTES, actual); } /** @@ -112,14 +105,13 @@ public void copyConstructorTest() { @Test public void constructorValidValues() { // Arrange - final List expectedBinaryData = Collections.singletonList(CONTENTS_BYTES); - final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(expectedBinaryData); + final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(CONTENTS_BYTES); // Act final AmqpAnnotatedMessage actual = new AmqpAnnotatedMessage(amqpMessageBody); // Assert - assertMessageCreation(AmqpBodyType.DATA, expectedBinaryData.size(), actual); + assertMessageCreation(AmqpBodyType.DATA, actual); } /** @@ -128,15 +120,14 @@ public void constructorValidValues() { @Test public void constructorAmqpValidValues() { // Arrange - final List expectedBinaryData = Collections.singletonList(CONTENTS_BYTES); - final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(expectedBinaryData); + final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(CONTENTS_BYTES); final AmqpAnnotatedMessage expected = new AmqpAnnotatedMessage(amqpMessageBody); // Act final AmqpAnnotatedMessage actual = new AmqpAnnotatedMessage(expected); // Assert - assertMessageCreation(AmqpBodyType.DATA, expectedBinaryData.size(), actual); + assertMessageCreation(AmqpBodyType.DATA, actual); } /** @@ -151,7 +142,7 @@ public void constructorNullValidValues() { Assertions.assertThrows(NullPointerException.class, () -> new AmqpAnnotatedMessage(body)); } - private void assertMessageCreation(AmqpBodyType expectedType, int expectedMessageSize, AmqpAnnotatedMessage actual) { + private void assertMessageCreation(AmqpBodyType expectedType, AmqpAnnotatedMessage actual) { assertEquals(expectedType, actual.getBody().getBodyType()); assertNotNull(actual.getProperties()); assertNotNull(actual.getHeader()); @@ -163,16 +154,15 @@ private void assertMessageCreation(AmqpBodyType expectedType, int expectedMessag // Validate Message Body assertNotNull(actual.getBody()); - assertMessageBody(expectedMessageSize, CONTENTS_BYTES, actual); + assertMessageBody(CONTENTS_BYTES, actual); } - private void assertMessageBody(int expectedMessageSize, byte[] expectedbody, AmqpAnnotatedMessage actual) { + private void assertMessageBody(byte[] expectedbody, AmqpAnnotatedMessage actual) { final AmqpBodyType actualType = actual.getBody().getBodyType(); switch (actualType) { case DATA: - List actualData = actual.getBody().getData().stream().collect(Collectors.toList()); - assertEquals(expectedMessageSize, actualData.size()); - assertArrayEquals(expectedbody, actualData.get(0)); + byte[] actualData = actual.getBody().getData().get(0); + assertArrayEquals(expectedbody, actualData); break; case VALUE: case SEQUENCE: diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java index 372d4fbcabcf..431f0113f24a 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java @@ -6,10 +6,6 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import java.util.ArrayList; -import java.util.List; -import java.util.stream.Collectors; - import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; @@ -24,19 +20,16 @@ public class AmqpMessageBodyTest { @Test public void constructorValidValues() { // Arrange - final List expectedDataList = new ArrayList<>(); - expectedDataList.add("some data 1".getBytes()); + final byte[] expectedData = "some data 1".getBytes(); // Act - final AmqpMessageBody actual = AmqpMessageBody.fromData(expectedDataList); + final AmqpMessageBody actual = AmqpMessageBody.fromData(expectedData); // Assert assertEquals(AmqpBodyType.DATA, actual.getBodyType()); // Validate Message Body - final List dataList = actual.getData().stream().collect(Collectors.toList()); - assertEquals(expectedDataList.size(), dataList.size()); - assertArrayEquals(expectedDataList.toArray(), dataList.toArray()); + assertArrayEquals(expectedData, actual.getData().get(0)); } /** @@ -45,23 +38,9 @@ public void constructorValidValues() { @Test public void constructorNullValidValues() { // Arrange - final List listBinaryData = null; - - // Act & Assert - Assertions.assertThrows(NullPointerException.class, () -> AmqpMessageBody.fromData(listBinaryData)); - } - - /** - * Verifies that only one byte array is alllowed in {@link AmqpMessageBody}. - */ - @Test - public void constructorSupportOneByteArray() { - // Arrange - final List expectedDataList = new ArrayList<>(); - expectedDataList.add("some data 1".getBytes()); - expectedDataList.add("some data 2".getBytes()); + final byte[] binaryData = null; // Act & Assert - Assertions.assertThrows(IllegalArgumentException.class, () -> AmqpMessageBody.fromData(expectedDataList)); + Assertions.assertThrows(NullPointerException.class, () -> AmqpMessageBody.fromData(binaryData)); } } diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java index cad24cf29ce1..3f7dabb04cb7 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java @@ -1,3 +1,6 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + package com.azure.core.amqp.models; import org.junit.jupiter.api.Assertions; diff --git a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java index f0a809d55be7..5920a34c9797 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java @@ -146,10 +146,10 @@ public void copyConstructorModifyAfterCopyTest() { // Assert // Validate updated values assertEquals(expectedSubject, originalMessage.getAmqpAnnotatedMessage().getProperties().getSubject()); - assertEquals(expectedTo, originalMessage.getAmqpAnnotatedMessage().getProperties().getTo()); - assertEquals(expectedReplyTo, originalMessage.getAmqpAnnotatedMessage().getProperties().getReplyTo()); + assertEquals(expectedTo, originalMessage.getAmqpAnnotatedMessage().getProperties().getTo().toString()); + assertEquals(expectedReplyTo, originalMessage.getAmqpAnnotatedMessage().getProperties().getReplyTo().toString()); assertEquals(expectedReplyToSessionId, originalMessage.getAmqpAnnotatedMessage().getProperties().getReplyToGroupId()); - assertEquals(expectedCorrelationId, originalMessage.getAmqpAnnotatedMessage().getProperties().getCorrelationId()); + assertEquals(expectedCorrelationId, originalMessage.getAmqpAnnotatedMessage().getProperties().getCorrelationId().toString()); assertEquals(expectedTimeToLive, originalMessage.getAmqpAnnotatedMessage().getHeader().getTimeToLive()); From ae8f8f2d266adb8fdf59b68755ec82b4c7980172 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Thu, 12 Nov 2020 00:07:56 -0800 Subject: [PATCH 05/23] Adding java doc --- .../java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java | 1 + .../core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java | 0 2 files changed, 1 insertion(+) rename sdk/core/azure-core-amqp/src/{sampels => samples}/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java (100%) diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index 9356c719542e..01740a1f64dd 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -93,6 +93,7 @@ public Map getApplicationProperties() { * Client should test for {@link AmqpBodyType} before calling corresponding get method on {@link AmqpMessageBody} *

How to check for {@link AmqpBodyType}

* {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} + * * @return the {@link AmqpMessageBody} object. */ public AmqpMessageBody getBody() { diff --git a/sdk/core/azure-core-amqp/src/sampels/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java similarity index 100% rename from sdk/core/azure-core-amqp/src/sampels/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java rename to sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java From 2918312ae75b15aad3b3be6d78e8fe1320652b85 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Thu, 12 Nov 2020 08:49:08 -0800 Subject: [PATCH 06/23] fix javadoc --- .../main/java/com/azure/core/amqp/models/AmqpAddress.java | 2 +- .../com/azure/core/amqp/models/AmqpAnnotatedMessage.java | 3 ++- .../java/com/azure/core/amqp/models/AmqpMessageBody.java | 8 ++++---- .../java/com/azure/core/amqp/models/AmqpMessageId.java | 2 +- .../java/com/azure/core/amqp/models/AmqpAddressTest.java | 8 ++------ .../com/azure/core/amqp/models/AmqpMessageIdTest.java | 7 +------ 6 files changed, 11 insertions(+), 19 deletions(-) diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java index 3aec41f824b0..17848509a4a1 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java @@ -43,7 +43,7 @@ public boolean equals(Object other) { return true; } - if (!address.equals(((AmqpAddress) other).toString())) { + if (!address.equals(other.toString())) { return false; } diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index 01740a1f64dd..6283e60bd7a2 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -90,7 +90,8 @@ public Map getApplicationProperties() { /** * Gets the {@link AmqpMessageBody} of an amqp message. - * Client should test for {@link AmqpBodyType} before calling corresponding get method on {@link AmqpMessageBody} + * Client should test for {@link AmqpBodyType} before calling corresponding get method on + * {@link AmqpMessageBody} *

How to check for {@link AmqpBodyType}

* {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index 33a6de04bede..519653ca92ca 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -12,14 +12,14 @@ /** * This class encapsulates the body of a message. The {@link AmqpBodyType} map to AMQP specification message body types. * Current implementation only support {@link AmqpBodyType#DATA DATA}. Other types will be supported in future releases. - * Amqp message body types: + * Amqp message body types: * * Client should test for {@link AmqpBodyType} before calling corresponding get method.Get methods not corresponding - * to the type of the body throw exception. + * to the type of the body throw exception. *

How to check for {@link AmqpBodyType}

* {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @@ -62,8 +62,8 @@ public AmqpBodyType getBodyType() { * Gets an immutable list containing only first byte array set on this {@link AmqpMessageBody}. The proton-j * library used only support one byte array, so the returned list will have only one element. Look for future * releases where we will support multiple byte array. - * Client should test for {@link AmqpBodyType} before calling corresponding get method.Get methods not corresponding - * to the type of the body throw exception. + * Client should test for {@link AmqpBodyType} before calling corresponding get method.Get methods not + * corresponding to the type of the body throw exception. *

How to check for {@link AmqpBodyType}

* {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @return data set on {@link AmqpMessageBody}. diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java index 6bf0a6ddee04..ad05b4db29e2 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java @@ -45,7 +45,7 @@ public boolean equals(Object other) { return true; } - if (!messageId.equals(((AmqpMessageId) other).toString())) { + if (!messageId.equals(other.toString())) { return false; } diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAddressTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAddressTest.java index bc0512196ff2..05d8e50c2bf8 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAddressTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAddressTest.java @@ -6,21 +6,17 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import java.util.List; - /** * Test for {@link AmqpAddress}. */ public class AmqpAddressTest { + /** * Verifies {@link AmqpAddress} constructor for null values. */ @Test public void constructorNullValue() { - // Arrange - final List listBinaryData = null; - - // Act & Assert + // Arrange, Act & Assert Assertions.assertThrows(NullPointerException.class, () -> new AmqpAddress(null)); } diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java index 3f7dabb04cb7..bb4c7bbdf6d1 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageIdTest.java @@ -6,8 +6,6 @@ import org.junit.jupiter.api.Assertions; import org.junit.jupiter.api.Test; -import java.util.List; - /** * Test for {@link AmqpMessageId}. */ @@ -17,10 +15,7 @@ public class AmqpMessageIdTest { */ @Test public void constructorNullValue() { - // Arrange - final List listBinaryData = null; - - // Act & Assert + // Arrange, Act & Assert Assertions.assertThrows(NullPointerException.class, () -> new AmqpMessageId(null)); } From fbf1efeefbf286154b0dc30b6e36872885f21352 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Thu, 12 Nov 2020 08:56:29 -0800 Subject: [PATCH 07/23] Adding java doc --- .../main/java/com/azure/core/amqp/models/AmqpMessageBody.java | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index 519653ca92ca..239e96efc9de 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -19,7 +19,7 @@ *
  • VALUE
    • * * Client should test for {@link AmqpBodyType} before calling corresponding get method.Get methods not corresponding - * to the type of the body throw exception. + * to the type of the body throws exception.     *

    How to check for {@link AmqpBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @@ -63,7 +63,7 @@ public AmqpBodyType getBodyType() { * library used only support one byte array, so the returned list will have only one element. Look for future * releases where we will support multiple byte array. * Client should test for {@link AmqpBodyType} before calling corresponding get method.Get methods not - * corresponding to the type of the body throw exception. + * corresponding to the type of the body throws exception.     *

    How to check for {@link AmqpBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @return data set on {@link AmqpMessageBody}. From e4525a5bd24b660558575f6a40f1444a2f25bbc4 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Thu, 12 Nov 2020 09:22:01 -0800 Subject: [PATCH 08/23] Renames amqp body type --- .../src/main/resources/revapi/revapi.json | 6 ++++ .../azure/core/amqp/models/AmqpAddress.java | 5 ++- .../amqp/models/AmqpAnnotatedMessage.java | 12 +++---- .../core/amqp/models/AmqpMessageBody.java | 31 ++++++++++--------- ...BodyType.java => AmqpMessageBodyType.java} | 2 +- ...mqpAnnotatedMessageJavaDocCodeSamples.java | 2 +- .../amqp/models/AmqpAnnotatedMessageTest.java | 8 ++--- .../core/amqp/models/AmqpMessageBodyTest.java | 2 +- 8 files changed, 39 insertions(+), 29 deletions(-) rename sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/{AmqpBodyType.java => AmqpMessageBodyType.java} (94%) diff --git a/eng/code-quality-reports/src/main/resources/revapi/revapi.json b/eng/code-quality-reports/src/main/resources/revapi/revapi.json index efd1dd748636..a0a4a1f8fe31 100644 --- a/eng/code-quality-reports/src/main/resources/revapi/revapi.json +++ b/eng/code-quality-reports/src/main/resources/revapi/revapi.json @@ -292,6 +292,12 @@ "new": "class com.azure.core.amqp.models.AmqpMessageBody", "justification": "Made it final." }, + { + "code": "java.method.returnTypeChanged", + "old": "method com.azure.core.amqp.models.AmqpBodyType com.azure.core.amqp.models.AmqpMessageBody::getBodyType()", + "new": "method com.azure.core.amqp.models.AmqpMessageBodyType com.azure.core.amqp.models.AmqpMessageBody::getBodyType()", + "justification": "Renamed to matach AmqpMessage prefix." + }, { "code": "java.method.returnTypeChanged", "old": "method java.lang.String com.azure.core.amqp.models.AmqpMessageProperties::getCorrelationId()", diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java index 17848509a4a1..71595728f0be 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java @@ -6,7 +6,10 @@ import java.util.Objects; /** - * This represents amqp address information. + * This represents amqp address information. This will be used in populating information like 'To', 'ReplyTo' etc. + * + * @see + * Address type Format. */ public final class AmqpAddress { diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index 6283e60bd7a2..61c38bf9bf18 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -49,14 +49,14 @@ public AmqpAnnotatedMessage(AmqpMessageBody body) { * @param message used to create another instance of {@link AmqpAnnotatedMessage}. * * @throws NullPointerException if {@code message} or {@link AmqpAnnotatedMessage#getBody() body} is null. - * @throws UnsupportedOperationException if {@link AmqpBodyType} is {@link AmqpBodyType#SEQUENCE} or - * {@link AmqpBodyType#VALUE}. - * @throws IllegalStateException for invalid {@link AmqpBodyType}. + * @throws UnsupportedOperationException if {@link AmqpMessageBodyType} is {@link AmqpMessageBodyType#SEQUENCE} or + * {@link AmqpMessageBodyType#VALUE}. + * @throws IllegalStateException for invalid {@link AmqpMessageBodyType}. */ public AmqpAnnotatedMessage(AmqpAnnotatedMessage message) { Objects.requireNonNull(message, "'message' cannot be null."); - AmqpBodyType bodyType = message.getBody().getBodyType(); + AmqpMessageBodyType bodyType = message.getBody().getBodyType(); switch (bodyType) { case DATA: amqpMessageBody = AmqpMessageBody.fromData(message.getBody().getData().get(0)); @@ -90,9 +90,9 @@ public Map getApplicationProperties() { /** * Gets the {@link AmqpMessageBody} of an amqp message. - * Client should test for {@link AmqpBodyType} before calling corresponding get method on + * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method on * {@link AmqpMessageBody} - *

    How to check for {@link AmqpBodyType}

    + *

    How to check for {@link AmqpMessageBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * * @return the {@link AmqpMessageBody} object. diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index 239e96efc9de..5afbe636f009 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -10,24 +10,25 @@ import java.util.Objects; /** - * This class encapsulates the body of a message. The {@link AmqpBodyType} map to AMQP specification message body types. - * Current implementation only support {@link AmqpBodyType#DATA DATA}. Other types will be supported in future releases. + * This class encapsulates the body of a message. The {@link AmqpMessageBodyType} map to AMQP specification message body + * types. Current implementation only support {@link AmqpMessageBodyType#DATA DATA}. Other types will be supported in + * future releases. * Amqp message body types: * - * Client should test for {@link AmqpBodyType} before calling corresponding get method.Get methods not corresponding - * to the type of the body throws exception. - *

    How to check for {@link AmqpBodyType}

    + * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method.Get methods not + * corresponding to the type of the body throws exception. + *

    How to check for {@link AmqpMessageBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * - * @see AmqpBodyType + * @see AmqpMessageBodyType */ public final class AmqpMessageBody { private static final ClientLogger LOGGER = new ClientLogger(AmqpMessageBody.class); - private AmqpBodyType bodyType; + private AmqpMessageBodyType bodyType; private byte[] data; @@ -43,18 +44,18 @@ public final class AmqpMessageBody { public static AmqpMessageBody fromData(byte[] data) { Objects.requireNonNull(data, "'data' cannot be null."); AmqpMessageBody body = new AmqpMessageBody(); - body.bodyType = AmqpBodyType.DATA; + body.bodyType = AmqpMessageBodyType.DATA; body.data = data; return body; } /** - * Gets the {@link AmqpBodyType} of the message. - *

    How to check for {@link AmqpBodyType}

    + * Gets the {@link AmqpMessageBodyType} of the message. + *

    How to check for {@link AmqpMessageBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @return AmqpBodyType type of the message. */ - public AmqpBodyType getBodyType() { + public AmqpMessageBodyType getBodyType() { return bodyType; } @@ -62,16 +63,16 @@ public AmqpBodyType getBodyType() { * Gets an immutable list containing only first byte array set on this {@link AmqpMessageBody}. The proton-j * library used only support one byte array, so the returned list will have only one element. Look for future * releases where we will support multiple byte array. - * Client should test for {@link AmqpBodyType} before calling corresponding get method.Get methods not + * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method.Get methods not * corresponding to the type of the body throws exception. - *

    How to check for {@link AmqpBodyType}

    + *

    How to check for {@link AmqpMessageBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @return data set on {@link AmqpMessageBody}. * - * @throws IllegalArgumentException If {@link AmqpBodyType} is not {@link AmqpBodyType#DATA DATA}. + * @throws IllegalArgumentException If {@link AmqpMessageBodyType} is not {@link AmqpMessageBodyType#DATA DATA}. */ public List getData() { - if (bodyType != AmqpBodyType.DATA) { + if (bodyType != AmqpMessageBodyType.DATA) { throw LOGGER.logExceptionAsError(new IllegalArgumentException(String.format("Can not return data for a " + "message which is of type %s.", getBodyType().toString()))); } diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpBodyType.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBodyType.java similarity index 94% rename from sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpBodyType.java rename to sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBodyType.java index 39b398729a0a..5f9493afc004 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpBodyType.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBodyType.java @@ -6,7 +6,7 @@ /** * All AmqpBodyType available for AMQP Message. */ -public enum AmqpBodyType { +public enum AmqpMessageBodyType { /** * Message content is byte array, equivalent to AMQP Data. */ diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java index 9af22d3ed4af..1f05d6be3f9b 100644 --- a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java +++ b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java @@ -13,7 +13,7 @@ public class AmqpAnnotatedMessageJavaDocCodeSamples { public void checkBodyType() { AmqpAnnotatedMessage amqpAnnotatedMessage = null; // BEGIN: com.azure.core.amqp.models.AmqpBodyType.checkBodyType - AmqpBodyType bodyType = amqpAnnotatedMessage.getBody().getBodyType(); + AmqpMessageBodyType bodyType = amqpAnnotatedMessage.getBody().getBodyType(); AmqpMessageBody messageBody = null; switch (bodyType) { case DATA: diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java index 4fe024527b2c..84c441acf2c4 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java @@ -111,7 +111,7 @@ public void constructorValidValues() { final AmqpAnnotatedMessage actual = new AmqpAnnotatedMessage(amqpMessageBody); // Assert - assertMessageCreation(AmqpBodyType.DATA, actual); + assertMessageCreation(AmqpMessageBodyType.DATA, actual); } /** @@ -127,7 +127,7 @@ public void constructorAmqpValidValues() { final AmqpAnnotatedMessage actual = new AmqpAnnotatedMessage(expected); // Assert - assertMessageCreation(AmqpBodyType.DATA, actual); + assertMessageCreation(AmqpMessageBodyType.DATA, actual); } /** @@ -142,7 +142,7 @@ public void constructorNullValidValues() { Assertions.assertThrows(NullPointerException.class, () -> new AmqpAnnotatedMessage(body)); } - private void assertMessageCreation(AmqpBodyType expectedType, AmqpAnnotatedMessage actual) { + private void assertMessageCreation(AmqpMessageBodyType expectedType, AmqpAnnotatedMessage actual) { assertEquals(expectedType, actual.getBody().getBodyType()); assertNotNull(actual.getProperties()); assertNotNull(actual.getHeader()); @@ -158,7 +158,7 @@ private void assertMessageCreation(AmqpBodyType expectedType, AmqpAnnotatedMessa } private void assertMessageBody(byte[] expectedbody, AmqpAnnotatedMessage actual) { - final AmqpBodyType actualType = actual.getBody().getBodyType(); + final AmqpMessageBodyType actualType = actual.getBody().getBodyType(); switch (actualType) { case DATA: byte[] actualData = actual.getBody().getData().get(0); diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java index 431f0113f24a..c206bb566cf6 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java @@ -26,7 +26,7 @@ public void constructorValidValues() { final AmqpMessageBody actual = AmqpMessageBody.fromData(expectedData); // Assert - assertEquals(AmqpBodyType.DATA, actual.getBodyType()); + assertEquals(AmqpMessageBodyType.DATA, actual.getBodyType()); // Validate Message Body assertArrayEquals(expectedData, actual.getData().get(0)); From ab9a71c33bf547b7ee067ec9a15b7ab618645b49 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Thu, 12 Nov 2020 11:51:27 -0800 Subject: [PATCH 09/23] type rename-> service bus change --- .../com/azure/messaging/servicebus/ServiceBusMessage.java | 4 ++-- .../azure/messaging/servicebus/ServiceBusReceivedMessage.java | 4 ++-- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java index 69f81a818fa8..26132babeebb 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java @@ -6,8 +6,8 @@ import com.azure.core.amqp.AmqpMessageConstant; import com.azure.core.amqp.models.AmqpAddress; import com.azure.core.amqp.models.AmqpAnnotatedMessage; -import com.azure.core.amqp.models.AmqpBodyType; import com.azure.core.amqp.models.AmqpMessageBody; +import com.azure.core.amqp.models.AmqpMessageBodyType; import com.azure.core.amqp.models.AmqpMessageId; import com.azure.core.util.Context; import com.azure.core.util.logging.ClientLogger; @@ -162,7 +162,7 @@ public Map getApplicationProperties() { * @return A byte array representing the data. */ public BinaryData getBody() { - final AmqpBodyType type = amqpAnnotatedMessage.getBody().getBodyType(); + final AmqpMessageBodyType type = amqpAnnotatedMessage.getBody().getBodyType(); switch (type) { case DATA: Optional byteArrayData = amqpAnnotatedMessage.getBody().getData().stream().findFirst(); diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java index 9b983fceb045..3065e5fb8072 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java @@ -6,8 +6,8 @@ import com.azure.core.amqp.AmqpMessageConstant; import com.azure.core.amqp.models.AmqpAddress; import com.azure.core.amqp.models.AmqpAnnotatedMessage; -import com.azure.core.amqp.models.AmqpBodyType; import com.azure.core.amqp.models.AmqpMessageBody; +import com.azure.core.amqp.models.AmqpMessageBodyType; import com.azure.core.amqp.models.AmqpMessageId; import com.azure.core.experimental.util.BinaryData; import com.azure.core.util.logging.ClientLogger; @@ -73,7 +73,7 @@ public AmqpAnnotatedMessage getAmqpAnnotatedMessage() { * @return A byte array representing the data. */ public BinaryData getBody() { - final AmqpBodyType bodyType = amqpAnnotatedMessage.getBody().getBodyType(); + final AmqpMessageBodyType bodyType = amqpAnnotatedMessage.getBody().getBodyType(); switch (bodyType) { case DATA: return BinaryData.fromBytes(amqpAnnotatedMessage.getBody().getData().stream().findFirst().get()); From efdc61115a958a7e2f1ad4f9b78d86ddff526e2d Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Thu, 12 Nov 2020 12:03:10 -0800 Subject: [PATCH 10/23] Rev API changes for body type rename --- .../src/main/resources/revapi/revapi.json | 5 +++++ 1 file changed, 5 insertions(+) diff --git a/eng/code-quality-reports/src/main/resources/revapi/revapi.json b/eng/code-quality-reports/src/main/resources/revapi/revapi.json index a0a4a1f8fe31..4facc1a87e25 100644 --- a/eng/code-quality-reports/src/main/resources/revapi/revapi.json +++ b/eng/code-quality-reports/src/main/resources/revapi/revapi.json @@ -292,6 +292,11 @@ "new": "class com.azure.core.amqp.models.AmqpMessageBody", "justification": "Made it final." }, + { + "code": "java.class.removed", + "old": "enum com.azure.core.amqp.models.AmqpBodyType", + "justification": "Because It is renamed to AmqpMessageBodyType" + }, { "code": "java.method.returnTypeChanged", "old": "method com.azure.core.amqp.models.AmqpBodyType com.azure.core.amqp.models.AmqpMessageBody::getBodyType()", From bb72b2129f5d11de43a0c6055bc22d6f2472a01b Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Thu, 12 Nov 2020 14:51:52 -0800 Subject: [PATCH 11/23] Added API based on review comments --- .../amqp/models/AmqpAnnotatedMessage.java | 2 +- .../core/amqp/models/AmqpMessageBody.java | 39 ++++++++++++++++--- ...mqpAnnotatedMessageJavaDocCodeSamples.java | 2 +- .../amqp/models/AmqpAnnotatedMessageTest.java | 2 +- .../core/amqp/models/AmqpMessageBodyTest.java | 3 +- .../servicebus/ServiceBusReceivedMessage.java | 3 +- 6 files changed, 40 insertions(+), 11 deletions(-) diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index 61c38bf9bf18..f7773205dbc3 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -59,7 +59,7 @@ public AmqpAnnotatedMessage(AmqpAnnotatedMessage message) { AmqpMessageBodyType bodyType = message.getBody().getBodyType(); switch (bodyType) { case DATA: - amqpMessageBody = AmqpMessageBody.fromData(message.getBody().getData().get(0)); + amqpMessageBody = AmqpMessageBody.fromData(message.getBody().getFirstData()); break; case SEQUENCE: case VALUE: diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index 5afbe636f009..da64f3307923 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -3,10 +3,13 @@ package com.azure.core.amqp.models; +import org.apache.qpid.proton.amqp.messaging.Data; + +import com.azure.core.util.IterableStream; import com.azure.core.util.logging.ClientLogger; +import java.util.Arrays; import java.util.Collections; -import java.util.List; import java.util.Objects; /** @@ -60,9 +63,30 @@ public AmqpMessageBodyType getBodyType() { } /** - * Gets an immutable list containing only first byte array set on this {@link AmqpMessageBody}. The proton-j - * library used only support one byte array, so the returned list will have only one element. Look for future - * releases where we will support multiple byte array. + * Gets an immutable list containing only first byte array set on this {@link AmqpMessageBody}. This library only + * support one byte array, so the returned list will have only one element. Look for future releases where we will + * support multiple byte array. + * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method.Get methods not + * corresponding to the type of the body throws exception. + *

    How to check for {@link AmqpMessageBodyType}

    + * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} + * @return data set on {@link AmqpMessageBody}. + * + * @throws IllegalArgumentException If {@link AmqpMessageBodyType} is not {@link AmqpMessageBodyType#DATA DATA}. + */ + public IterableStream getData() { + if (bodyType != AmqpMessageBodyType.DATA) { + throw LOGGER.logExceptionAsError(new IllegalArgumentException(String.format("Can not return data for a " + + "message which is of type %s.", getBodyType().toString()))); + } + + return new IterableStream<>(Collections.singletonList(data)); + } + + /** + * Gets first byte array set on this {@link AmqpMessageBody}. This library only support one byte array on Amqp + * Message. Look for future releases where we will support multiple byte array and you can use + * {@link AmqpMessageBody#getData()} API. * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method.Get methods not * corresponding to the type of the body throws exception. *

    How to check for {@link AmqpMessageBodyType}

    @@ -70,12 +94,15 @@ public AmqpMessageBodyType getBodyType() { * @return data set on {@link AmqpMessageBody}. * * @throws IllegalArgumentException If {@link AmqpMessageBodyType} is not {@link AmqpMessageBodyType#DATA DATA}. + * @see + * Amqp Message Format. */ - public List getData() { + public byte[] getFirstData() { if (bodyType != AmqpMessageBodyType.DATA) { throw LOGGER.logExceptionAsError(new IllegalArgumentException(String.format("Can not return data for a " + "message which is of type %s.", getBodyType().toString()))); } - return Collections.singletonList(data); + + return Arrays.copyOf(data, data.length); } } diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java index 1f05d6be3f9b..8cca8bdd6659 100644 --- a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java +++ b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java @@ -17,7 +17,7 @@ public void checkBodyType() { AmqpMessageBody messageBody = null; switch (bodyType) { case DATA: - messageBody = AmqpMessageBody.fromData(amqpAnnotatedMessage.getBody().getData().get(0)); + messageBody = AmqpMessageBody.fromData(amqpAnnotatedMessage.getBody().getFirstData()); break; case SEQUENCE: case VALUE: diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java index 84c441acf2c4..df5e3f7f2d20 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java @@ -161,7 +161,7 @@ private void assertMessageBody(byte[] expectedbody, AmqpAnnotatedMessage actual) final AmqpMessageBodyType actualType = actual.getBody().getBodyType(); switch (actualType) { case DATA: - byte[] actualData = actual.getBody().getData().get(0); + byte[] actualData = actual.getBody().getData().stream().findFirst().get(); assertArrayEquals(expectedbody, actualData); break; case VALUE: diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java index c206bb566cf6..2f8a3d303c7b 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpMessageBodyTest.java @@ -29,7 +29,8 @@ public void constructorValidValues() { assertEquals(AmqpMessageBodyType.DATA, actual.getBodyType()); // Validate Message Body - assertArrayEquals(expectedData, actual.getData().get(0)); + assertArrayEquals(expectedData, actual.getFirstData()); + assertArrayEquals(expectedData, actual.getData().stream().findFirst().get()); } /** diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java index 3065e5fb8072..3ce23ca4ef2f 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusReceivedMessage.java @@ -76,7 +76,8 @@ public BinaryData getBody() { final AmqpMessageBodyType bodyType = amqpAnnotatedMessage.getBody().getBodyType(); switch (bodyType) { case DATA: - return BinaryData.fromBytes(amqpAnnotatedMessage.getBody().getData().stream().findFirst().get()); + return BinaryData.fromBytes(amqpAnnotatedMessage.getBody() + .getData().stream().findFirst().get()); case SEQUENCE: case VALUE: throw logger.logExceptionAsError(new UnsupportedOperationException("Body type not supported yet " From 9eb5273d7e3e30623d97f771e34a3e2ddbff2286 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Thu, 12 Nov 2020 15:18:21 -0800 Subject: [PATCH 12/23] Review commnets --- .../java/com/azure/core/amqp/models/AmqpMessageBody.java | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index da64f3307923..8bca0974d899 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -3,8 +3,6 @@ package com.azure.core.amqp.models; -import org.apache.qpid.proton.amqp.messaging.Data; - import com.azure.core.util.IterableStream; import com.azure.core.util.logging.ClientLogger; @@ -35,6 +33,10 @@ public final class AmqpMessageBody { private byte[] data; + AmqpMessageBody() { + // package constructor so no one can create instance of this except classes im this package. + } + /** * Creates instance of {@link AmqpMessageBody} with given byte array. * From 5513888eff77aa242c898e40f8e7d4c5fe71ae38 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Fri, 13 Nov 2020 09:05:23 -0800 Subject: [PATCH 13/23] Review Comments --- .../checkstyle/checkstyle-suppressions.xml | 1 - eng/versioning/version_client.txt | 1 + .../azure/core/amqp/models/AmqpAddress.java | 10 ++++----- .../core/amqp/models/AmqpMessageBody.java | 18 +++++++++------ .../azure/core/amqp/models/AmqpMessageId.java | 22 ++++++++++++------- .../models/AmqpAddressJavaDocCodeSamples.java | 21 ++++++++++++++++++ .../AmqpMessageIdJavaDocCodeSamples.java | 21 ++++++++++++++++++ .../azure-messaging-servicebus/pom.xml | 2 +- 8 files changed, 74 insertions(+), 22 deletions(-) create mode 100644 sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAddressJavaDocCodeSamples.java create mode 100644 sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpMessageIdJavaDocCodeSamples.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 c8816fe48fa6..42252c13d1b0 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 @@ -269,7 +269,6 @@ - diff --git a/eng/versioning/version_client.txt b/eng/versioning/version_client.txt index 0504b20c386e..397f76f7045b 100644 --- a/eng/versioning/version_client.txt +++ b/eng/versioning/version_client.txt @@ -177,6 +177,7 @@ com.microsoft:microsoft-opentelemetry-exporter-azuremonitor;1.0.0-beta.1;1.0.0-b # unreleased_:;dependency-version # note: The unreleased dependencies will not be manipulated with the automatic PR creation code. unreleased_com.azure:azure-core-experimental;1.0.0-beta.9 +unreleased_com.azure:azure-core-amqp;1.7.0-beta.2 unreleased_com.azure:azure-messaging-servicebus;7.0.0-beta.7 unreleased_com.azure:azure-messaging-eventhubs;5.3.0 diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java index 71595728f0be..4b7f1ce022a0 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java @@ -8,6 +8,9 @@ /** * This represents amqp address information. This will be used in populating information like 'To', 'ReplyTo' etc. * + *

    Create and retrieve address

    + * {@codesnippet com.azure.core.amqp.models.AmqpAddress.createAndGet} + * * @see * Address type Format. */ @@ -19,6 +22,7 @@ public final class AmqpAddress { * Creates the {@link AmqpAddress} with given {@code address}. * * @param address to use. + * @throws NullPointerException if {@code address} is null. */ public AmqpAddress(String address) { this.address = Objects.requireNonNull(address, "'address' cannot be null."); @@ -46,11 +50,7 @@ public boolean equals(Object other) { return true; } - if (!address.equals(other.toString())) { - return false; - } - - return true; + return Objects.equals(address, other.toString()); } /** diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index 8bca0974d899..c04f1c21ba99 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -20,15 +20,17 @@ *
  • SEQUENCE
    • *
  • VALUE
    • * + * * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method.Get methods not * corresponding to the type of the body throws exception. + * *

    How to check for {@link AmqpMessageBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * * @see AmqpMessageBodyType */ public final class AmqpMessageBody { - private static final ClientLogger LOGGER = new ClientLogger(AmqpMessageBody.class); + private final ClientLogger logger = new ClientLogger(AmqpMessageBody.class); private AmqpMessageBodyType bodyType; private byte[] data; @@ -65,11 +67,13 @@ public AmqpMessageBodyType getBodyType() { } /** - * Gets an immutable list containing only first byte array set on this {@link AmqpMessageBody}. This library only - * support one byte array, so the returned list will have only one element. Look for future releases where we will - * support multiple byte array. + * Gets an {@link IterableStream} of byte array containing only first byte array set on this + * {@link AmqpMessageBody}. This library only support one byte array, so the returned list will have only one + * element. Look for future releases where we will support multiple byte array. + * * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method.Get methods not * corresponding to the type of the body throws exception. + * *

    How to check for {@link AmqpMessageBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @return data set on {@link AmqpMessageBody}. @@ -78,11 +82,11 @@ public AmqpMessageBodyType getBodyType() { */ public IterableStream getData() { if (bodyType != AmqpMessageBodyType.DATA) { - throw LOGGER.logExceptionAsError(new IllegalArgumentException(String.format("Can not return data for a " + throw logger.logExceptionAsError(new IllegalArgumentException(String.format("Cannot return data for a " + "message which is of type %s.", getBodyType().toString()))); } - return new IterableStream<>(Collections.singletonList(data)); + return new IterableStream<>(Collections.singletonList(Arrays.copyOf(data, data.length))); } /** @@ -101,7 +105,7 @@ public IterableStream getData() { */ public byte[] getFirstData() { if (bodyType != AmqpMessageBodyType.DATA) { - throw LOGGER.logExceptionAsError(new IllegalArgumentException(String.format("Can not return data for a " + throw logger.logExceptionAsError(new IllegalArgumentException(String.format("Cannot return data for a " + "message which is of type %s.", getBodyType().toString()))); } diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java index ad05b4db29e2..46380fca32a7 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java @@ -6,10 +6,19 @@ import java.util.Objects; /** - * This represents Message id in various forms. + * This represents Message id. Amqp specification support message id in various types. This class only implements + * {@link String} representation at present. * - * @see - * Amqp Message Id. + *

    Create and retrieve message id

    + * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} + * + * Amqp message id types: + * */ public final class AmqpMessageId { private final String messageId; @@ -18,6 +27,7 @@ public final class AmqpMessageId { * Creates the {@link AmqpMessageId} with given {@code messageId}. * * @param messageId representing id of the message. + * @throws NullPointerException if {@code messageId} is null. */ public AmqpMessageId(String messageId) { this.messageId = Objects.requireNonNull(messageId, "'messageId' cannot be null."); @@ -45,11 +55,7 @@ public boolean equals(Object other) { return true; } - if (!messageId.equals(other.toString())) { - return false; - } - - return true; + return messageId.equals(other.toString()); } /** diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAddressJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAddressJavaDocCodeSamples.java new file mode 100644 index 000000000000..10b33680e97a --- /dev/null +++ b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAddressJavaDocCodeSamples.java @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.amqp.models; + +/** + * Class contains sample code snippets that will be used in javadocs. + */ +public class AmqpAddressJavaDocCodeSamples { + /** + * Get message body from {@link AmqpAddress}. + */ + public void address() { + // BEGIN: com.azure.core.amqp.models.AmqpAddress.createAndGet + AmqpAddress amqpAddress = new AmqpAddress("my-address"); + // Retrieve Adderss + String address = amqpAddress.toString(); + System.out.println("Address " + address); + // END: com.azure.core.amqp.models.AmqpAddress.createAndGet + } +} diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpMessageIdJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpMessageIdJavaDocCodeSamples.java new file mode 100644 index 000000000000..f78d8661c737 --- /dev/null +++ b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpMessageIdJavaDocCodeSamples.java @@ -0,0 +1,21 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + +package com.azure.core.amqp.models; + +/** + * Class contains sample code snippets that will be used in javadocs. + */ +public class AmqpMessageIdJavaDocCodeSamples { + /** + * Get message body from {@link AmqpMessageId}. + */ + public void messageId() { + // BEGIN: com.azure.core.amqp.models.AmqpMessageId.createAndGet + AmqpMessageId messageId = new AmqpMessageId("my-message-id"); + // Retrieve Message id + String id = messageId.toString(); + System.out.println("Message Id " + id); + // END: com.azure.core.amqp.models.AmqpMessageId.createAndGet + } +} diff --git a/sdk/servicebus/azure-messaging-servicebus/pom.xml b/sdk/servicebus/azure-messaging-servicebus/pom.xml index 42626aa360d3..79352798d809 100644 --- a/sdk/servicebus/azure-messaging-servicebus/pom.xml +++ b/sdk/servicebus/azure-messaging-servicebus/pom.xml @@ -52,7 +52,7 @@ com.azure azure-core-amqp - 1.7.0-beta.2 + 1.7.0-beta.2 com.azure From 0886d24a21dc92ebb8db6f2ebd5eacfc69facbda Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 17 Nov 2020 00:27:54 -0800 Subject: [PATCH 14/23] Review Comments --- .../src/main/resources/revapi/revapi.json | 2 +- .../azure/core/amqp/models/AmqpAddress.java | 9 ++-- .../amqp/models/AmqpAnnotatedMessage.java | 25 ++++++--- .../core/amqp/models/AmqpMessageBody.java | 53 ++++++++++--------- .../core/amqp/models/AmqpMessageBodyType.java | 12 ++++- .../core/amqp/models/AmqpMessageHeader.java | 2 +- .../azure/core/amqp/models/AmqpMessageId.java | 7 +-- ...mqpAnnotatedMessageJavaDocCodeSamples.java | 21 ++++++++ .../servicebus/ServiceBusMessage.java | 12 +---- 9 files changed, 90 insertions(+), 53 deletions(-) diff --git a/eng/code-quality-reports/src/main/resources/revapi/revapi.json b/eng/code-quality-reports/src/main/resources/revapi/revapi.json index 4facc1a87e25..c0d9f16c1260 100644 --- a/eng/code-quality-reports/src/main/resources/revapi/revapi.json +++ b/eng/code-quality-reports/src/main/resources/revapi/revapi.json @@ -301,7 +301,7 @@ "code": "java.method.returnTypeChanged", "old": "method com.azure.core.amqp.models.AmqpBodyType com.azure.core.amqp.models.AmqpMessageBody::getBodyType()", "new": "method com.azure.core.amqp.models.AmqpMessageBodyType com.azure.core.amqp.models.AmqpMessageBody::getBodyType()", - "justification": "Renamed to matach AmqpMessage prefix." + "justification": "Renamed to match AmqpMessage prefix." }, { "code": "java.method.returnTypeChanged", diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java index 4b7f1ce022a0..4d2e33a2f5ef 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAddress.java @@ -21,13 +21,16 @@ public final class AmqpAddress { /** * Creates the {@link AmqpAddress} with given {@code address}. * - * @param address to use. + * @param address The address to set for this instance. * @throws NullPointerException if {@code address} is null. */ public AmqpAddress(String address) { this.address = Objects.requireNonNull(address, "'address' cannot be null."); } + /** + * {@inheritDoc} + */ @Override public int hashCode() { return address.hashCode(); @@ -54,9 +57,7 @@ public boolean equals(Object other) { } /** - * Gets string representation of the address. - * - * @return string representation of the address. + * {@inheritDoc} */ @Override public String toString() { diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index f7773205dbc3..095d6eaf4cb2 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -5,15 +5,18 @@ import com.azure.core.util.logging.ClientLogger; +import java.util.Arrays; import java.util.HashMap; +import java.util.Locale; import java.util.Map; import java.util.Objects; /** * The representation of message as defined by AMQP protocol. * - * @see - * Amqp Message Format. + * @see + * Amqp Message Format + * @see AmqpMessageBody */ public final class AmqpAnnotatedMessage { private final ClientLogger logger = new ClientLogger(AmqpAnnotatedMessage.class); @@ -45,12 +48,19 @@ public AmqpAnnotatedMessage(AmqpMessageBody body) { /** * Creates instance of {@link AmqpAnnotatedMessage} with given {@link AmqpAnnotatedMessage} instance. + * The Current implementation only support {@link AmqpMessageBodyType#DATA DATA} AMQP data type. Track this + * issue to find out support + * for other AMQP types. * + *

    How to check for {@link AmqpMessageBodyType} before calling this constructor

    + * {@codesnippet com.azure.core.amqp.models.AmqpAnnotatedMessage.copyAmqpAnnotatedMessage} + * @param message used to create another instance of {@link AmqpAnnotatedMessage}. * * @throws NullPointerException if {@code message} or {@link AmqpAnnotatedMessage#getBody() body} is null. * @throws UnsupportedOperationException if {@link AmqpMessageBodyType} is {@link AmqpMessageBodyType#SEQUENCE} or - * {@link AmqpMessageBodyType#VALUE}. + * {@link AmqpMessageBodyType#VALUE}. See code sample above explaining how to check for {@link AmqpMessageBodyType} + * before calling this constructor. * @throws IllegalStateException for invalid {@link AmqpMessageBodyType}. */ public AmqpAnnotatedMessage(AmqpAnnotatedMessage message) { @@ -59,12 +69,15 @@ public AmqpAnnotatedMessage(AmqpAnnotatedMessage message) { AmqpMessageBodyType bodyType = message.getBody().getBodyType(); switch (bodyType) { case DATA: - amqpMessageBody = AmqpMessageBody.fromData(message.getBody().getFirstData()); + final byte[] data = message.getBody().getFirstData(); + amqpMessageBody = AmqpMessageBody.fromData(Arrays.copyOf(data, data.length)); break; case SEQUENCE: case VALUE: - throw logger.logExceptionAsError(new UnsupportedOperationException("Body type not supported yet " - + bodyType.toString())); + throw logger.logExceptionAsError(new UnsupportedOperationException( + String.format(Locale.US, "This constructor only support body type [%s] at present. Track " + + "this issue, https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type " + + "support in future.", AmqpMessageBodyType.DATA.toString()))); default: throw logger.logExceptionAsError(new IllegalStateException("Body type not valid " + bodyType.toString())); diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index 930b3d10369c..beefd2343848 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -6,24 +6,19 @@ import com.azure.core.util.IterableStream; import com.azure.core.util.logging.ClientLogger; -import java.util.Arrays; import java.util.Collections; import java.util.List; +import java.util.Locale; import java.util.Objects; /** - * This class encapsulates the body of a message. The {@link AmqpMessageBodyType} map to AMQP specification message body - * types. Current implementation only support {@link AmqpMessageBodyType#DATA DATA}. Other types will be supported in - * future releases. - * Amqp message body types: - * + * This class encapsulates the body of a message. The {@link AmqpMessageBodyType} map to an AMQP specification message + * body types. Current implementation only support {@link AmqpMessageBodyType#DATA DATA} AMQP data type. Track this + * issue to find out support for + * other AMQP types. * - * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method.Get methods not - * corresponding to the type of the body throws exception. + *

    Client should test for {@link AmqpMessageBodyType} before calling corresponding get method. Get methods not + * corresponding to the type of the body throws exception.

    * *

    How to check for {@link AmqpMessageBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} @@ -36,7 +31,7 @@ public final class AmqpMessageBody { private List data; - AmqpMessageBody() { + private AmqpMessageBody() { // package constructor so no one can create instance of this except classes im this package. } @@ -53,7 +48,7 @@ public static AmqpMessageBody fromData(byte[] data) { Objects.requireNonNull(data, "'data' cannot be null."); AmqpMessageBody body = new AmqpMessageBody(); body.bodyType = AmqpMessageBodyType.DATA; - body.data = Collections.singletonList(Arrays.copyOf(data, data.length)); + body.data = Collections.singletonList(data); return body; } @@ -69,11 +64,10 @@ public AmqpMessageBodyType getBodyType() { /** * Gets an {@link IterableStream} of byte array containing only first byte array set on this - * {@link AmqpMessageBody}. This library only support one byte array, so the returned list will have only one - * element. Look for future releases where we will support multiple byte array. - * - * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method.Get methods not - * corresponding to the type of the body throws exception. + * {@link AmqpMessageBody}. This library only support one byte array at present, so the returned list will have only + * one element. + *

    Client should test for {@link AmqpMessageBodyType} before calling corresponding get method. Get methods not + * corresponding to the type of the body throws exception.

    * *

    How to check for {@link AmqpMessageBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} @@ -83,8 +77,11 @@ public AmqpMessageBodyType getBodyType() { */ public IterableStream getData() { if (bodyType != AmqpMessageBodyType.DATA) { - throw logger.logExceptionAsError(new IllegalArgumentException(String.format("Cannot return data for a " - + "message which is of type %s.", getBodyType().toString()))); + + throw logger.logExceptionAsError(new IllegalArgumentException( + String.format(Locale.US, "This method can only be called for body type [%s]. Track this issue, " + + "https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type support in " + + "future.", AmqpMessageBodyType.DATA.toString()))); } return new IterableStream<>(data); @@ -94,20 +91,24 @@ public IterableStream getData() { * Gets first byte array set on this {@link AmqpMessageBody}. This library only support one byte array on Amqp * Message. Look for future releases where we will support multiple byte array and you can use * {@link AmqpMessageBody#getData()} API. - * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method.Get methods not - * corresponding to the type of the body throws exception. + *

    Client should test for {@link AmqpMessageBodyType} before calling corresponding get method. Get methods not + * corresponding to the type of the body throws exception.

    + * *

    How to check for {@link AmqpMessageBodyType}

    * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * @return data set on {@link AmqpMessageBody}. * * @throws IllegalArgumentException If {@link AmqpMessageBodyType} is not {@link AmqpMessageBodyType#DATA DATA}. - * @see + * @see * Amqp Message Format. */ public byte[] getFirstData() { if (bodyType != AmqpMessageBodyType.DATA) { - throw logger.logExceptionAsError(new IllegalArgumentException(String.format("Cannot return data for a " - + "message which is of type %s.", getBodyType().toString()))); + + throw logger.logExceptionAsError(new IllegalArgumentException( + String.format(Locale.US, "This method can only be called for body type [%s]. Track this issue, " + + "https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type support in " + + "future.", AmqpMessageBodyType.DATA.toString()))); } return data.get(0); diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBodyType.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBodyType.java index 5f9493afc004..55119fd7edb3 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBodyType.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBodyType.java @@ -4,7 +4,17 @@ package com.azure.core.amqp.models; /** - * All AmqpBodyType available for AMQP Message. + * Represents all valid {@link AmqpMessageBodyType} for an AMQP Message. Current SDK only support + * {@link AmqpMessageBodyType#DATA DATA} AMQP data type. Track this issue + * to find out support for other AMQP types. + * + *

    Types of Amqp message body

    + * + */ public enum AmqpMessageBodyType { /** diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageHeader.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageHeader.java index e15a482558f3..67e5efd722ac 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageHeader.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageHeader.java @@ -10,7 +10,7 @@ /** * The representation of message header as defined by AMQP protocol. - * @see + * @see * Amqp Message Format. */ @Fluent diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java index 46380fca32a7..aeeae2424e53 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java @@ -33,6 +33,9 @@ public AmqpMessageId(String messageId) { this.messageId = Objects.requireNonNull(messageId, "'messageId' cannot be null."); } + /** + * {@inheritDoc} + */ @Override public int hashCode() { return messageId.hashCode(); @@ -59,9 +62,7 @@ public boolean equals(Object other) { } /** - * Gets string representation of the message id. - * - * @return string representation of the message id. + * {@inheritDoc} */ @Override public String toString() { diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java index 8cca8bdd6659..532efb37c4a6 100644 --- a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java +++ b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java @@ -27,4 +27,25 @@ public void checkBodyType() { } // END: com.azure.core.amqp.models.AmqpBodyType.checkBodyType } + + /** + * Copy {@link AmqpAnnotatedMessage}. + */ + public void copyAmqpAnnotatedMessage() { + AmqpAnnotatedMessage sourceAnnotatedMessage = null; + // BEGIN: com.azure.core.amqp.models.AmqpAnnotatedMessage.copyAmqpAnnotatedMessage + AmqpMessageBodyType bodyType = sourceAnnotatedMessage.getBody().getBodyType(); + AmqpAnnotatedMessage copyAnnotatedMessage = null; + switch (bodyType) { + case DATA: + copyAnnotatedMessage = new AmqpAnnotatedMessage(sourceAnnotatedMessage); + break; + case SEQUENCE: + case VALUE: + throw new RuntimeException("Body type not supported yet."); + default: + throw new RuntimeException("Body type not valid."); + } + // END: com.azure.core.amqp.models.AmqpAnnotatedMessage.copyAmqpAnnotatedMessage + } } diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java index 26132babeebb..48900920db92 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java @@ -18,7 +18,6 @@ import java.time.ZoneOffset; import java.util.Map; import java.util.Objects; -import java.util.Optional; import static com.azure.core.amqp.AmqpMessageConstant.DEAD_LETTER_DESCRIPTION_ANNOTATION_NAME; import static com.azure.core.amqp.AmqpMessageConstant.DEAD_LETTER_REASON_ANNOTATION_NAME; @@ -165,16 +164,7 @@ public BinaryData getBody() { final AmqpMessageBodyType type = amqpAnnotatedMessage.getBody().getBodyType(); switch (type) { case DATA: - Optional byteArrayData = amqpAnnotatedMessage.getBody().getData().stream().findFirst(); - final byte[] bytes; - - if (byteArrayData.isPresent()) { - bytes = byteArrayData.get(); - } else { - logger.warning("Data not present."); - bytes = new byte[0]; - } - return BinaryData.fromBytes(bytes); + return BinaryData.fromBytes(amqpAnnotatedMessage.getBody().getFirstData()); case SEQUENCE: case VALUE: throw logger.logExceptionAsError(new UnsupportedOperationException("Not supported AmqpBodyType: " From c64ca0188de5e7580234d8dd9122c4be3933e706 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 17 Nov 2020 00:33:27 -0800 Subject: [PATCH 15/23] Review Comments --- .../java/com/azure/core/amqp/models/AmqpMessageId.java | 8 ++++---- .../com/azure/core/amqp/models/AmqpMessageProperties.java | 2 +- 2 files changed, 5 insertions(+), 5 deletions(-) diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java index aeeae2424e53..9f8f180a84cb 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java @@ -14,10 +14,10 @@ * * Amqp message id types: * */ public final class AmqpMessageId { diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java index 77c248e407e9..4b35dd0e9d2a 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java @@ -12,7 +12,7 @@ /** * The representation of message properties as defined by AMQP protocol. * - * @see + * @see * Amqp Message Format. */ @Fluent From 0292180d767ac84a00f010f705576b0abe179898 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 17 Nov 2020 09:42:52 -0800 Subject: [PATCH 16/23] Removed copy constructors --- .../amqp/models/AmqpAnnotatedMessage.java | 48 -------- .../core/amqp/models/AmqpMessageHeader.java | 14 --- .../amqp/models/AmqpMessageProperties.java | 22 ---- ...mqpAnnotatedMessageJavaDocCodeSamples.java | 21 ---- .../amqp/models/AmqpAnnotatedMessageTest.java | 94 --------------- .../servicebus/ServiceBusMessage.java | 107 ++++++++++++++++-- .../ServiceBusMessageJavaDocCodeSamples.java | 27 +++++ .../servicebus/ServiceBusMessageTest.java | 53 --------- 8 files changed, 125 insertions(+), 261 deletions(-) create mode 100644 sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index 095d6eaf4cb2..2dd5a520f423 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -5,9 +5,7 @@ import com.azure.core.util.logging.ClientLogger; -import java.util.Arrays; import java.util.HashMap; -import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -46,52 +44,6 @@ public AmqpAnnotatedMessage(AmqpMessageBody body) { properties = new AmqpMessageProperties(); } - /** - * Creates instance of {@link AmqpAnnotatedMessage} with given {@link AmqpAnnotatedMessage} instance. - * The Current implementation only support {@link AmqpMessageBodyType#DATA DATA} AMQP data type. Track this - * issue to find out support - * for other AMQP types. - * - *

    How to check for {@link AmqpMessageBodyType} before calling this constructor

    - * {@codesnippet com.azure.core.amqp.models.AmqpAnnotatedMessage.copyAmqpAnnotatedMessage} - - * @param message used to create another instance of {@link AmqpAnnotatedMessage}. - * - * @throws NullPointerException if {@code message} or {@link AmqpAnnotatedMessage#getBody() body} is null. - * @throws UnsupportedOperationException if {@link AmqpMessageBodyType} is {@link AmqpMessageBodyType#SEQUENCE} or - * {@link AmqpMessageBodyType#VALUE}. See code sample above explaining how to check for {@link AmqpMessageBodyType} - * before calling this constructor. - * @throws IllegalStateException for invalid {@link AmqpMessageBodyType}. - */ - public AmqpAnnotatedMessage(AmqpAnnotatedMessage message) { - Objects.requireNonNull(message, "'message' cannot be null."); - - AmqpMessageBodyType bodyType = message.getBody().getBodyType(); - switch (bodyType) { - case DATA: - final byte[] data = message.getBody().getFirstData(); - amqpMessageBody = AmqpMessageBody.fromData(Arrays.copyOf(data, data.length)); - break; - case SEQUENCE: - case VALUE: - throw logger.logExceptionAsError(new UnsupportedOperationException( - String.format(Locale.US, "This constructor only support body type [%s] at present. Track " - + "this issue, https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type " - + "support in future.", AmqpMessageBodyType.DATA.toString()))); - default: - throw logger.logExceptionAsError(new IllegalStateException("Body type not valid " - + bodyType.toString())); - } - - applicationProperties = new HashMap<>(message.getApplicationProperties()); - deliveryAnnotations = new HashMap<>(message.getDeliveryAnnotations()); - messageAnnotations = new HashMap<>(message.getMessageAnnotations()); - footer = new HashMap<>(message.getFooter()); - header = new AmqpMessageHeader(message.getHeader()); - properties = new AmqpMessageProperties(message.getProperties()); - } - - /** * Gets the {@link Map} of application properties. * diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageHeader.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageHeader.java index 67e5efd722ac..cc26df5f755f 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageHeader.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageHeader.java @@ -6,7 +6,6 @@ import com.azure.core.annotation.Fluent; import java.time.Duration; -import java.util.Objects; /** * The representation of message header as defined by AMQP protocol. @@ -26,19 +25,6 @@ public class AmqpMessageHeader { // This class does not have any public constructors, and is not able to be instantiated using 'new'. } - /** - * The constructor is used to clone the values. - */ - AmqpMessageHeader(AmqpMessageHeader header) { - super(); - Objects.requireNonNull(header, "'header' cannot be null."); - deliveryCount = header.getDeliveryCount(); - durable = header.isDurable(); - firstAcquirer = header.isFirstAcquirer(); - timeToLive = header.getTimeToLive(); - priority = header.getPriority(); - } - /** * Gets the delivery count from amqp message header. * diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java index 4b35dd0e9d2a..db716e433840 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageProperties.java @@ -7,7 +7,6 @@ import java.time.OffsetDateTime; import java.util.Arrays; -import java.util.Objects; /** * The representation of message properties as defined by AMQP protocol. @@ -36,27 +35,6 @@ public class AmqpMessageProperties { // This class does not have any public constructors, and is not able to be instantiated using 'new'. } - /** - * The constructor is used to clone the values. - */ - AmqpMessageProperties(AmqpMessageProperties properties) { - super(); - Objects.requireNonNull(properties, "'properties' cannot be null."); - absoluteExpiryTime = properties.getAbsoluteExpiryTime(); - contentEncoding = properties.getContentEncoding(); - contentType = properties.getContentType(); - correlationId = properties.getCorrelationId(); - creationTime = properties.getCreationTime(); - groupId = properties.getGroupId(); - groupSequence = properties.getGroupSequence(); - messageId = properties.getMessageId(); - replyToGroupId = properties.getReplyToGroupId(); - replyTo = properties.getReplyTo(); - to = properties.getTo(); - subject = properties.getSubject(); - userId = properties.getUserId(); - } - /** * Gets {@code absoluteExpiryTime} from amqp message properties. * diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java index 532efb37c4a6..8cca8bdd6659 100644 --- a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java +++ b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java @@ -27,25 +27,4 @@ public void checkBodyType() { } // END: com.azure.core.amqp.models.AmqpBodyType.checkBodyType } - - /** - * Copy {@link AmqpAnnotatedMessage}. - */ - public void copyAmqpAnnotatedMessage() { - AmqpAnnotatedMessage sourceAnnotatedMessage = null; - // BEGIN: com.azure.core.amqp.models.AmqpAnnotatedMessage.copyAmqpAnnotatedMessage - AmqpMessageBodyType bodyType = sourceAnnotatedMessage.getBody().getBodyType(); - AmqpAnnotatedMessage copyAnnotatedMessage = null; - switch (bodyType) { - case DATA: - copyAnnotatedMessage = new AmqpAnnotatedMessage(sourceAnnotatedMessage); - break; - case SEQUENCE: - case VALUE: - throw new RuntimeException("Body type not supported yet."); - default: - throw new RuntimeException("Body type not valid."); - } - // END: com.azure.core.amqp.models.AmqpAnnotatedMessage.copyAmqpAnnotatedMessage - } } diff --git a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java index df5e3f7f2d20..435386d93d0c 100644 --- a/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java +++ b/sdk/core/azure-core-amqp/src/test/java/com/azure/core/amqp/models/AmqpAnnotatedMessageTest.java @@ -8,13 +8,10 @@ import org.junit.jupiter.api.Test; import java.nio.charset.StandardCharsets; -import java.time.Duration; -import java.util.Map; import static org.junit.jupiter.api.Assertions.assertArrayEquals; import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; -import static org.junit.jupiter.api.Assertions.assertNotSame; /** * Test class for {@link AmqpAnnotatedMessage} @@ -24,81 +21,6 @@ public class AmqpAnnotatedMessageTest { private static final byte[] CONTENTS_BYTES = "Some-contents".getBytes(StandardCharsets.UTF_8); private final ClientLogger logger = new ClientLogger(AmqpAnnotatedMessageTest.class); - /** - * Verifies we correctly set values via copy constructor for {@link AmqpAnnotatedMessage} and create new - * instances of the properties. - */ - @Test - public void copyConstructorTest() { - // Arrange - final int expectedBinaryDataSize = 1; - - final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(CONTENTS_BYTES); - final AmqpAnnotatedMessage expected = new AmqpAnnotatedMessage(amqpMessageBody); - final Map expectedMessageAnnotations = expected.getMessageAnnotations(); - expectedMessageAnnotations.put("ma-1", "ma-value1"); - - final Map expectedDeliveryAnnotations = expected.getDeliveryAnnotations(); - expectedDeliveryAnnotations.put("da-1", "da-value1"); - - final Map expectedApplicationProperties = expected.getApplicationProperties(); - expectedApplicationProperties.put("ap-1", "ap-value1"); - - final Map expectedFooter = expected.getFooter(); - expectedFooter.put("foo-1", "foo-value1"); - - final AmqpMessageProperties expectedMessageProperties = expected.getProperties(); - expectedMessageProperties.setGroupSequence(2L); - expectedMessageProperties.setContentEncoding("content-enc"); - expectedMessageProperties.setReplyToGroupId("a"); - expectedMessageProperties.setReplyTo(new AmqpAddress("b")); - expectedMessageProperties.setCorrelationId(new AmqpMessageId("c")); - expectedMessageProperties.setSubject("d"); - expectedMessageProperties.setMessageId(new AmqpMessageId("id")); - - final AmqpMessageHeader expectedMessageHeader = expected.getHeader(); - expectedMessageHeader.setDeliveryCount(5L); - expectedMessageHeader.setTimeToLive(Duration.ofSeconds(20)); - expectedMessageHeader.setPriority(Short.valueOf("4")); - - final AmqpAnnotatedMessage actual = new AmqpAnnotatedMessage(expected); - - // Act - // Now update the values after we have created AmqpAnnotatedMessage using copy constructor. - expectedDeliveryAnnotations.remove("da-1"); - expectedApplicationProperties.put("ap-2", "ap-value2"); - expectedFooter.remove("foo-1"); - expected.getHeader().setDeliveryCount(Long.valueOf(100)); - - // Assert - // Ensure the memory references are not same. - assertNotSame(expected.getProperties(), actual.getProperties()); - assertNotSame(expected.getApplicationProperties(), actual.getApplicationProperties()); - assertNotSame(expected.getDeliveryAnnotations(), actual.getDeliveryAnnotations()); - assertNotSame(expected.getFooter(), actual.getFooter()); - assertNotSame(expected.getHeader(), actual.getHeader()); - assertNotSame(expected.getMessageAnnotations(), actual.getMessageAnnotations()); - assertNotSame(expected.getProperties().getUserId(), actual.getProperties().getUserId()); - assertNotSame(expected.getHeader().getDeliveryCount(), actual.getHeader().getDeliveryCount()); - - assertEquals(1, actual.getDeliveryAnnotations().size()); - assertEquals(1, actual.getApplicationProperties().size()); - assertEquals(1, actual.getFooter().size()); - - assertEquals(expectedMessageProperties.getGroupSequence(), actual.getProperties().getGroupSequence()); - assertEquals(expectedMessageProperties.getContentEncoding(), actual.getProperties().getContentEncoding()); - assertEquals(expectedMessageProperties.getReplyToGroupId(), actual.getProperties().getReplyToGroupId()); - assertEquals(expectedMessageProperties.getReplyTo(), actual.getProperties().getReplyTo()); - assertEquals(expectedMessageProperties.getCorrelationId(), actual.getProperties().getCorrelationId()); - assertEquals(expectedMessageProperties.getSubject(), actual.getProperties().getSubject()); - assertEquals(expectedMessageProperties.getMessageId(), actual.getProperties().getMessageId()); - - assertEquals(expectedMessageHeader.getTimeToLive(), actual.getHeader().getTimeToLive()); - assertEquals(expectedMessageHeader.getPriority(), actual.getHeader().getPriority()); - - assertMessageBody(CONTENTS_BYTES, actual); - } - /** * Verifies we correctly set values via constructor for {@link AmqpAnnotatedMessage}. */ @@ -114,22 +36,6 @@ public void constructorValidValues() { assertMessageCreation(AmqpMessageBodyType.DATA, actual); } - /** - * Verifies we correctly set values via constructor for {@link AmqpAnnotatedMessage}. - */ - @Test - public void constructorAmqpValidValues() { - // Arrange - final AmqpMessageBody amqpMessageBody = AmqpMessageBody.fromData(CONTENTS_BYTES); - final AmqpAnnotatedMessage expected = new AmqpAnnotatedMessage(amqpMessageBody); - - // Act - final AmqpAnnotatedMessage actual = new AmqpAnnotatedMessage(expected); - - // Assert - assertMessageCreation(AmqpMessageBodyType.DATA, actual); - } - /** * Verifies {@link AmqpAnnotatedMessage} constructor for null values. */ diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java index 48900920db92..239e35afe22b 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java @@ -8,7 +8,9 @@ import com.azure.core.amqp.models.AmqpAnnotatedMessage; import com.azure.core.amqp.models.AmqpMessageBody; import com.azure.core.amqp.models.AmqpMessageBodyType; +import com.azure.core.amqp.models.AmqpMessageHeader; import com.azure.core.amqp.models.AmqpMessageId; +import com.azure.core.amqp.models.AmqpMessageProperties; import com.azure.core.util.Context; import com.azure.core.util.logging.ClientLogger; import com.azure.core.experimental.util.BinaryData; @@ -16,6 +18,8 @@ import java.time.Duration; import java.time.OffsetDateTime; import java.time.ZoneOffset; +import java.util.Arrays; +import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -107,23 +111,108 @@ public ServiceBusMessage(BinaryData body) { * @param receivedMessage The received message to create new message from. * * @throws NullPointerException if {@code receivedMessage} is {@code null}. + * @throws UnsupportedOperationException if {@link AmqpMessageBodyType} is {@link AmqpMessageBodyType#SEQUENCE} or + * {@link AmqpMessageBodyType#VALUE}. See code sample above explaining how to check for {@link AmqpMessageBodyType} + * before calling this constructor. + *

    How to check for {@link AmqpMessageBodyType} before calling this constructor

    + * {@codesnippet com.azure.messaging.servicebus.ServiceBusMessage.copyServiceBusMessage} + * @throws IllegalStateException for invalid {@link AmqpMessageBodyType}. + * @see AmqpAnnotatedMessage */ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { Objects.requireNonNull(receivedMessage, "'receivedMessage' cannot be null."); - this.amqpAnnotatedMessage = new AmqpAnnotatedMessage(receivedMessage.getAmqpAnnotatedMessage()); - this.context = Context.NONE; + final AmqpMessageBodyType bodyType = receivedMessage.getAmqpAnnotatedMessage().getBody().getBodyType(); + AmqpMessageBody amqpMessageBody; + switch (bodyType) { + case DATA: + final byte[] data = receivedMessage.getAmqpAnnotatedMessage().getBody().getFirstData(); + amqpMessageBody = AmqpMessageBody.fromData(Arrays.copyOf(data, data.length)); + break; + case SEQUENCE: + case VALUE: + throw logger.logExceptionAsError(new UnsupportedOperationException( + String.format(Locale.US, "This constructor only support body type [%s] at present. Track " + + "this issue, https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type " + + "support in future.", AmqpMessageBodyType.DATA.toString()))); + default: + throw logger.logExceptionAsError(new IllegalStateException("Body type not valid " + + bodyType.toString())); + } + this.amqpAnnotatedMessage = new AmqpAnnotatedMessage(amqpMessageBody); + + // set properties + final AmqpMessageProperties receivedProperties = receivedMessage.getAmqpAnnotatedMessage().getProperties(); + final AmqpMessageProperties newProperties = amqpAnnotatedMessage.getProperties(); + newProperties.setMessageId(receivedProperties.getMessageId()); + newProperties.setUserId(receivedProperties.getUserId()); + newProperties.setTo(receivedProperties.getTo()); + newProperties.setSubject(receivedProperties.getSubject()); + newProperties.setReplyTo(receivedProperties.getReplyTo()); + newProperties.setCorrelationId(receivedProperties.getCorrelationId()); + newProperties.setContentType(receivedProperties.getContentType()); + newProperties.setContentEncoding(receivedProperties.getContentEncoding()); + newProperties.setAbsoluteExpiryTime(receivedProperties.getAbsoluteExpiryTime()); + newProperties.setCreationTime(receivedProperties.getCreationTime()); + newProperties.setGroupId(receivedProperties.getGroupId()); + newProperties.setGroupSequence(receivedProperties.getGroupSequence()); + newProperties.setReplyToGroupId(receivedProperties.getReplyToGroupId()); + + // copy header except for delivery count which should be set to null + final AmqpMessageHeader receivedHeader = receivedMessage.getAmqpAnnotatedMessage().getHeader(); + final AmqpMessageHeader newHeader = amqpAnnotatedMessage.getHeader(); + newHeader.setPriority(receivedHeader.getPriority()); + newHeader.setTimeToLive(receivedHeader.getTimeToLive()); + newHeader.setDurable(receivedHeader.isDurable()); + newHeader.setFirstAcquirer(receivedHeader.isFirstAcquirer()); + + // copy message annotations except for broker set ones + final Map receivedAnnotations = receivedMessage.getAmqpAnnotatedMessage().getMessageAnnotations(); + final Map newAnnotations = amqpAnnotatedMessage.getMessageAnnotations(); + + for (Map.Entry entry: receivedAnnotations.entrySet()) { + if (AmqpMessageConstant.fromString(entry.getKey()) == LOCKED_UNTIL_KEY_ANNOTATION_NAME + || AmqpMessageConstant.fromString(entry.getKey()) == SEQUENCE_NUMBER_ANNOTATION_NAME + || AmqpMessageConstant.fromString(entry.getKey()) == DEAD_LETTER_SOURCE_KEY_ANNOTATION_NAME + || AmqpMessageConstant.fromString(entry.getKey()) == ENQUEUED_SEQUENCE_NUMBER_ANNOTATION_NAME + || AmqpMessageConstant.fromString(entry.getKey()) == ENQUEUED_TIME_UTC_ANNOTATION_NAME) { + + continue; + } + newAnnotations.put(entry.getKey(), entry.getValue()); + } - // clean up data which user is not allowed to set. - amqpAnnotatedMessage.getHeader().setDeliveryCount(null); + // copy delivery annotations + final Map receivedDelivery = receivedMessage.getAmqpAnnotatedMessage().getDeliveryAnnotations(); + final Map newDelivery = amqpAnnotatedMessage.getMessageAnnotations(); - removeValues(amqpAnnotatedMessage.getMessageAnnotations(), LOCKED_UNTIL_KEY_ANNOTATION_NAME, - SEQUENCE_NUMBER_ANNOTATION_NAME, DEAD_LETTER_SOURCE_KEY_ANNOTATION_NAME, - ENQUEUED_SEQUENCE_NUMBER_ANNOTATION_NAME, ENQUEUED_TIME_UTC_ANNOTATION_NAME); + for (Map.Entry entry: receivedDelivery.entrySet()) { + newDelivery.put(entry.getKey(), entry.getValue()); + } - removeValues(amqpAnnotatedMessage.getApplicationProperties(), DEAD_LETTER_DESCRIPTION_ANNOTATION_NAME, - DEAD_LETTER_REASON_ANNOTATION_NAME); + // copy Footer + final Map receivedFooter = receivedMessage.getAmqpAnnotatedMessage().getFooter(); + final Map newFooter = amqpAnnotatedMessage.getMessageAnnotations(); + for (Map.Entry entry: receivedFooter.entrySet()) { + newFooter.put(entry.getKey(), entry.getValue()); + } + + // copy application properties except for broker set ones + final Map receivedApplicationProperties = receivedMessage.getAmqpAnnotatedMessage() + .getApplicationProperties(); + final Map newApplicationProperties = amqpAnnotatedMessage.getApplicationProperties(); + + for (Map.Entry entry: receivedApplicationProperties.entrySet()) { + if (AmqpMessageConstant.fromString(entry.getKey()) == DEAD_LETTER_DESCRIPTION_ANNOTATION_NAME + || AmqpMessageConstant.fromString(entry.getKey()) == DEAD_LETTER_REASON_ANNOTATION_NAME) { + + continue; + } + newApplicationProperties.put(entry.getKey(), entry.getValue()); + } + + this.context = Context.NONE; } /** diff --git a/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java b/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java new file mode 100644 index 000000000000..2cf1b104b077 --- /dev/null +++ b/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java @@ -0,0 +1,27 @@ +package com.azure.messaging.servicebus; + +import com.azure.core.amqp.models.AmqpAnnotatedMessage; +import com.azure.core.amqp.models.AmqpMessageBodyType; + +public class ServiceBusMessageJavaDocCodeSamples { + /** + * Copy {@link AmqpAnnotatedMessage}. + */ + public void copyServiceBusReceivedMessage() { + ServiceBusReceivedMessage sourceServiceBusReceivedMessage = null; + // BEGIN: com.azure.messaging.servicebus.ServiceBusMessage.copyServiceBusMessage + AmqpMessageBodyType bodyType = sourceServiceBusReceivedMessage.getAmqpAnnotatedMessage().getBody().getBodyType(); + ServiceBusMessage copyMessage = null; + switch (bodyType) { + case DATA: + copyMessage = new ServiceBusMessage(sourceServiceBusReceivedMessage); + break; + case SEQUENCE: + case VALUE: + throw new RuntimeException("Body type not supported yet."); + default: + throw new RuntimeException("Body type not valid."); + } + // END: com.azure.messaging.servicebus.ServiceBusMessage.copyServiceBusMessage + } +} diff --git a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java index 5920a34c9797..223fc61a5c42 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java @@ -103,59 +103,6 @@ public void copyConstructorTest() { assertNull(actual.getAmqpAnnotatedMessage().getHeader().getDeliveryCount()); } - - /** - * Verifies we correctly set values via copy constructor for {@link ServiceBusMessage}. - * 1. Ensure modifying original `ServiceBusReceivedMessage` object does not change values of new ServiceBusMessage - * object changes its values. - */ - @Test - public void copyConstructorModifyAfterCopyTest() { - // Arrange - final String expectedSubject = "old-subject"; - final String expectedTo = "old-to"; - final String expectedReplyTo = "old-reply-to"; - final String expectedReplyToSessionId = "old-reply-to-session-id"; - final String expectedCorrelationId = "old-d-id"; - final String expectedDeadLetterSource = "old-d-l-source"; - final Duration expectedTimeToLive = Duration.ofSeconds(20); - final String expectedPartitionKey = "old-p-key"; - - final ServiceBusReceivedMessage originalMessage = new ServiceBusReceivedMessage(PAYLOAD_BINARY); - originalMessage.setSubject(expectedSubject); - originalMessage.setTo(expectedTo); - originalMessage.setReplyTo(expectedReplyTo); - originalMessage.setReplyToSessionId(expectedReplyToSessionId); - originalMessage.setCorrelationId(expectedCorrelationId); - originalMessage.setDeadLetterSource(expectedDeadLetterSource); - originalMessage.setTimeToLive(expectedTimeToLive); - originalMessage.setPartitionKey(expectedPartitionKey); - - final ServiceBusMessage copiedMessage = new ServiceBusMessage(originalMessage); - - // Act - // Modify the values after invoking copy constructor - copiedMessage.setSubject("new-subject"); - copiedMessage.setTo("new-to"); - copiedMessage.setReplyTo("new-reply-to"); - copiedMessage.setReplyToSessionId("new-session-id"); - copiedMessage.setCorrelationId("new-c-id"); - copiedMessage.setTimeToLive(Duration.ofSeconds(40)); - copiedMessage.setPartitionKey("new-p-key"); - - // Assert - // Validate updated values - assertEquals(expectedSubject, originalMessage.getAmqpAnnotatedMessage().getProperties().getSubject()); - assertEquals(expectedTo, originalMessage.getAmqpAnnotatedMessage().getProperties().getTo().toString()); - assertEquals(expectedReplyTo, originalMessage.getAmqpAnnotatedMessage().getProperties().getReplyTo().toString()); - assertEquals(expectedReplyToSessionId, originalMessage.getAmqpAnnotatedMessage().getProperties().getReplyToGroupId()); - assertEquals(expectedCorrelationId, originalMessage.getAmqpAnnotatedMessage().getProperties().getCorrelationId().toString()); - - assertEquals(expectedTimeToLive, originalMessage.getAmqpAnnotatedMessage().getHeader().getTimeToLive()); - - assertEquals(expectedPartitionKey, originalMessage.getAmqpAnnotatedMessage().getMessageAnnotations().get(PARTITION_KEY_ANNOTATION_NAME.getValue())); - } - /** * Verify UTF_8 encoded body is created. */ From 235a6a4abcc1f9ccf0602ce20a1a8bc868b2718c Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 17 Nov 2020 09:59:19 -0800 Subject: [PATCH 17/23] unit test change --- .../servicebus/ServiceBusMessage.java | 12 +--- .../ServiceBusMessageJavaDocCodeSamples.java | 6 ++ .../servicebus/ServiceBusMessageTest.java | 64 +++++++++++++++++++ 3 files changed, 72 insertions(+), 10 deletions(-) diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java index 239e35afe22b..1ca0eb724358 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java @@ -167,7 +167,8 @@ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { newHeader.setFirstAcquirer(receivedHeader.isFirstAcquirer()); // copy message annotations except for broker set ones - final Map receivedAnnotations = receivedMessage.getAmqpAnnotatedMessage().getMessageAnnotations(); + final Map receivedAnnotations = receivedMessage.getAmqpAnnotatedMessage() + .getMessageAnnotations(); final Map newAnnotations = amqpAnnotatedMessage.getMessageAnnotations(); for (Map.Entry entry: receivedAnnotations.entrySet()) { @@ -654,15 +655,6 @@ public ServiceBusMessage addContext(String key, Object value) { return this; } - /* - * Gets value from given map. - */ - private void removeValues(Map dataMap, AmqpMessageConstant... keys) { - for (AmqpMessageConstant key : keys) { - dataMap.remove(key.getValue()); - } - } - /** * Checks the length of ID fields. * diff --git a/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java b/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java index 2cf1b104b077..6439b03f32d2 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java @@ -1,8 +1,14 @@ +// Copyright (c) Microsoft Corporation. All rights reserved. +// Licensed under the MIT License. + package com.azure.messaging.servicebus; import com.azure.core.amqp.models.AmqpAnnotatedMessage; import com.azure.core.amqp.models.AmqpMessageBodyType; +/** + * java doc sample for {@link ServiceBusMessage}. + */ public class ServiceBusMessageJavaDocCodeSamples { /** * Copy {@link AmqpAnnotatedMessage}. diff --git a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java index 223fc61a5c42..da85fbe069df 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java @@ -24,6 +24,7 @@ import org.junit.jupiter.api.Test; import java.time.Duration; +import java.util.Map; /** * Test for {@link ServiceBusMessage}. @@ -103,6 +104,69 @@ public void copyConstructorTest() { assertNull(actual.getAmqpAnnotatedMessage().getHeader().getDeliveryCount()); } + + /** + * Verifies we correctly set values via copy constructor for {@link ServiceBusMessage}. + * 1. Ensure modifying original `ServiceBusReceivedMessage` object does not change values of new ServiceBusMessage + * object changes its values. + */ + @Test + public void copyConstructorModifyAfterCopyTest() { + // Arrange + final String expectedSubject = "old-subject"; + final String expectedTo = "old-to"; + final String expectedReplyTo = "old-reply-to"; + final String expectedReplyToSessionId = "old-reply-to-session-id"; + final String expectedCorrelationId = "old-d-id"; + final String expectedDeadLetterSource = "old-d-l-source"; + final Duration expectedTimeToLive = Duration.ofSeconds(20); + final String expectedPartitionKey = "old-p-key"; + final short expectedPriority = 10; + + final ServiceBusReceivedMessage originalMessage = new ServiceBusReceivedMessage(PAYLOAD_BINARY); + originalMessage.setSubject(expectedSubject); + originalMessage.setTo(expectedTo); + originalMessage.setReplyTo(expectedReplyTo); + originalMessage.setReplyToSessionId(expectedReplyToSessionId); + originalMessage.setCorrelationId(expectedCorrelationId); + originalMessage.setDeadLetterSource(expectedDeadLetterSource); + originalMessage.setTimeToLive(expectedTimeToLive); + originalMessage.setPartitionKey(expectedPartitionKey); + originalMessage.getAmqpAnnotatedMessage().getHeader().setPriority(expectedPriority); + + final Map expectedFooter = originalMessage.getAmqpAnnotatedMessage().getFooter(); + expectedFooter.put("foo-1", "foo-value1"); + + final ServiceBusMessage copiedMessage = new ServiceBusMessage(originalMessage); + + // Act + // Modify the values after invoking copy constructor + copiedMessage.setSubject("new-subject"); + copiedMessage.setTo("new-to"); + copiedMessage.setReplyTo("new-reply-to"); + copiedMessage.setReplyToSessionId("new-session-id"); + copiedMessage.setCorrelationId("new-c-id"); + copiedMessage.setTimeToLive(Duration.ofSeconds(40)); + copiedMessage.setPartitionKey("new-p-key"); + + // change original message + originalMessage.getAmqpAnnotatedMessage().getHeader().setPriority((short) (expectedPriority+ 1)); + + // Assert + // Validate updated values + assertEquals(expectedSubject, originalMessage.getAmqpAnnotatedMessage().getProperties().getSubject()); + assertEquals(expectedTo, originalMessage.getAmqpAnnotatedMessage().getProperties().getTo().toString()); + assertEquals(expectedReplyTo, originalMessage.getAmqpAnnotatedMessage().getProperties().getReplyTo().toString()); + assertEquals(expectedReplyToSessionId, originalMessage.getAmqpAnnotatedMessage().getProperties().getReplyToGroupId()); + assertEquals(expectedCorrelationId, originalMessage.getAmqpAnnotatedMessage().getProperties().getCorrelationId().toString()); + + assertEquals(expectedTimeToLive, originalMessage.getAmqpAnnotatedMessage().getHeader().getTimeToLive()); + + assertEquals(expectedPartitionKey, originalMessage.getAmqpAnnotatedMessage().getMessageAnnotations().get(PARTITION_KEY_ANNOTATION_NAME.getValue())); + + assertEquals(expectedPriority, originalMessage.getAmqpAnnotatedMessage().getHeader().getPriority()); + } + /** * Verify UTF_8 encoded body is created. */ From 3140431d4111d34e3ecfb565244d7d6e18246ce2 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 17 Nov 2020 13:02:40 -0800 Subject: [PATCH 18/23] Incorporate review comments --- .../servicebus/ServiceBusMessage.java | 16 +++---- .../servicebus/ServiceBusMessageTest.java | 42 ++++++++++++++----- 2 files changed, 39 insertions(+), 19 deletions(-) diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java index 1ca0eb724358..71dfdf319a2d 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java @@ -132,9 +132,9 @@ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { case SEQUENCE: case VALUE: throw logger.logExceptionAsError(new UnsupportedOperationException( - String.format(Locale.US, "This constructor only support body type [%s] at present. Track " - + "this issue, https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type " - + "support in future.", AmqpMessageBodyType.DATA.toString()))); + "This constructor only supports the AMQP Data body type at present. Track this issue, " + + "https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type support in " + + "future.")); default: throw logger.logExceptionAsError(new IllegalStateException("Body type not valid " + bodyType.toString())); @@ -160,7 +160,7 @@ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { // copy header except for delivery count which should be set to null final AmqpMessageHeader receivedHeader = receivedMessage.getAmqpAnnotatedMessage().getHeader(); - final AmqpMessageHeader newHeader = amqpAnnotatedMessage.getHeader(); + final AmqpMessageHeader newHeader = this.amqpAnnotatedMessage.getHeader(); newHeader.setPriority(receivedHeader.getPriority()); newHeader.setTimeToLive(receivedHeader.getTimeToLive()); newHeader.setDurable(receivedHeader.isDurable()); @@ -169,7 +169,7 @@ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { // copy message annotations except for broker set ones final Map receivedAnnotations = receivedMessage.getAmqpAnnotatedMessage() .getMessageAnnotations(); - final Map newAnnotations = amqpAnnotatedMessage.getMessageAnnotations(); + final Map newAnnotations = this.amqpAnnotatedMessage.getMessageAnnotations(); for (Map.Entry entry: receivedAnnotations.entrySet()) { if (AmqpMessageConstant.fromString(entry.getKey()) == LOCKED_UNTIL_KEY_ANNOTATION_NAME @@ -185,7 +185,7 @@ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { // copy delivery annotations final Map receivedDelivery = receivedMessage.getAmqpAnnotatedMessage().getDeliveryAnnotations(); - final Map newDelivery = amqpAnnotatedMessage.getMessageAnnotations(); + final Map newDelivery = this.amqpAnnotatedMessage.getDeliveryAnnotations(); for (Map.Entry entry: receivedDelivery.entrySet()) { newDelivery.put(entry.getKey(), entry.getValue()); @@ -193,7 +193,7 @@ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { // copy Footer final Map receivedFooter = receivedMessage.getAmqpAnnotatedMessage().getFooter(); - final Map newFooter = amqpAnnotatedMessage.getMessageAnnotations(); + final Map newFooter = this.amqpAnnotatedMessage.getFooter(); for (Map.Entry entry: receivedFooter.entrySet()) { newFooter.put(entry.getKey(), entry.getValue()); @@ -202,7 +202,7 @@ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { // copy application properties except for broker set ones final Map receivedApplicationProperties = receivedMessage.getAmqpAnnotatedMessage() .getApplicationProperties(); - final Map newApplicationProperties = amqpAnnotatedMessage.getApplicationProperties(); + final Map newApplicationProperties = this.amqpAnnotatedMessage.getApplicationProperties(); for (Map.Entry entry: receivedApplicationProperties.entrySet()) { if (AmqpMessageConstant.fromString(entry.getKey()) == DEAD_LETTER_DESCRIPTION_ANNOTATION_NAME diff --git a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java index da85fbe069df..013888fd5889 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/test/java/com/azure/messaging/servicebus/ServiceBusMessageTest.java @@ -52,6 +52,12 @@ public void copyConstructorTest() { final Duration expectedTimeToLive = Duration.ofSeconds(20); final String expectedPartitionKey = "old-p-key"; + final short expectedPriority = 10; + final String expectedFooterValue = "foo-value1"; + final String expectedDeliveryAnnotationsValue = "da-value1"; + final String expectedApplicationValue = "ap-value1"; + + final ServiceBusReceivedMessage expected = new ServiceBusReceivedMessage(PAYLOAD_BINARY); expected.getAmqpAnnotatedMessage().getMessageAnnotations().put(SEQUENCE_NUMBER_ANNOTATION_NAME.getValue(), "10"); expected.getAmqpAnnotatedMessage().getMessageAnnotations().put(DEAD_LETTER_SOURCE_KEY_ANNOTATION_NAME.getValue(), "abc"); @@ -68,6 +74,17 @@ public void copyConstructorTest() { expected.setTimeToLive(expectedTimeToLive); expected.setPartitionKey(expectedPartitionKey); + expected.getAmqpAnnotatedMessage().getHeader().setPriority(expectedPriority); + + final Map expectedFooter = expected.getAmqpAnnotatedMessage().getFooter(); + expectedFooter.put("foo-1", expectedFooterValue); + + final Map expectedDeliveryAnnotations = expected.getAmqpAnnotatedMessage().getDeliveryAnnotations(); + expectedDeliveryAnnotations.put("da-1", expectedDeliveryAnnotationsValue); + + final Map expectedApplicationProperties = expected.getApplicationProperties(); + expectedApplicationProperties.put("ap-1", expectedApplicationValue); + final ServiceBusMessage actual = new ServiceBusMessage(expected); // Act @@ -80,6 +97,13 @@ public void copyConstructorTest() { expected.setTimeToLive(Duration.ofSeconds(40)); expected.setPartitionKey("new-p-key"); + // Change original values + expected.getAmqpAnnotatedMessage().getHeader().setPriority((short) (expectedPriority + 1)); + expectedFooter.put("foo-1", expectedFooterValue + "-changed"); + expected.getAmqpAnnotatedMessage().getDeliveryAnnotations().put("da-1", expectedDeliveryAnnotationsValue + "-changed"); + expected.getAmqpAnnotatedMessage().getApplicationProperties().put("ap-1", expectedApplicationValue + "-changed"); + + // Assert assertNotSame(expected.getAmqpAnnotatedMessage(), actual.getAmqpAnnotatedMessage()); @@ -102,8 +126,14 @@ public void copyConstructorTest() { assertNull(actual.getAmqpAnnotatedMessage().getApplicationProperties().get(DEAD_LETTER_DESCRIPTION_ANNOTATION_NAME.getValue())); assertNull(actual.getAmqpAnnotatedMessage().getApplicationProperties().get(DEAD_LETTER_REASON_ANNOTATION_NAME.getValue())); assertNull(actual.getAmqpAnnotatedMessage().getHeader().getDeliveryCount()); - } + // Testing , updating original message did not change copied message values.. + assertEquals(expectedPriority, actual.getAmqpAnnotatedMessage().getHeader().getPriority()); + assertEquals(expectedFooterValue, actual.getAmqpAnnotatedMessage().getFooter().get("foo-1").toString()); + assertEquals(expectedDeliveryAnnotationsValue, actual.getAmqpAnnotatedMessage().getDeliveryAnnotations().get("da-1").toString()); + assertEquals(expectedApplicationValue, actual.getAmqpAnnotatedMessage().getApplicationProperties().get("ap-1").toString()); + + } /** * Verifies we correctly set values via copy constructor for {@link ServiceBusMessage}. @@ -121,7 +151,6 @@ public void copyConstructorModifyAfterCopyTest() { final String expectedDeadLetterSource = "old-d-l-source"; final Duration expectedTimeToLive = Duration.ofSeconds(20); final String expectedPartitionKey = "old-p-key"; - final short expectedPriority = 10; final ServiceBusReceivedMessage originalMessage = new ServiceBusReceivedMessage(PAYLOAD_BINARY); originalMessage.setSubject(expectedSubject); @@ -132,10 +161,6 @@ public void copyConstructorModifyAfterCopyTest() { originalMessage.setDeadLetterSource(expectedDeadLetterSource); originalMessage.setTimeToLive(expectedTimeToLive); originalMessage.setPartitionKey(expectedPartitionKey); - originalMessage.getAmqpAnnotatedMessage().getHeader().setPriority(expectedPriority); - - final Map expectedFooter = originalMessage.getAmqpAnnotatedMessage().getFooter(); - expectedFooter.put("foo-1", "foo-value1"); final ServiceBusMessage copiedMessage = new ServiceBusMessage(originalMessage); @@ -149,9 +174,6 @@ public void copyConstructorModifyAfterCopyTest() { copiedMessage.setTimeToLive(Duration.ofSeconds(40)); copiedMessage.setPartitionKey("new-p-key"); - // change original message - originalMessage.getAmqpAnnotatedMessage().getHeader().setPriority((short) (expectedPriority+ 1)); - // Assert // Validate updated values assertEquals(expectedSubject, originalMessage.getAmqpAnnotatedMessage().getProperties().getSubject()); @@ -163,8 +185,6 @@ public void copyConstructorModifyAfterCopyTest() { assertEquals(expectedTimeToLive, originalMessage.getAmqpAnnotatedMessage().getHeader().getTimeToLive()); assertEquals(expectedPartitionKey, originalMessage.getAmqpAnnotatedMessage().getMessageAnnotations().get(PARTITION_KEY_ANNOTATION_NAME.getValue())); - - assertEquals(expectedPriority, originalMessage.getAmqpAnnotatedMessage().getHeader().getPriority()); } /** From 2d71f31d418865aa65ca65803b80d755236ffe14 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 17 Nov 2020 13:21:16 -0800 Subject: [PATCH 19/23] Incorporate review comments --- .../com/azure/core/amqp/models/AmqpMessageBody.java | 11 +++++------ 1 file changed, 5 insertions(+), 6 deletions(-) diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index beefd2343848..4ebc2815efaa 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -8,7 +8,6 @@ import java.util.Collections; import java.util.List; -import java.util.Locale; import java.util.Objects; /** @@ -79,9 +78,9 @@ public IterableStream getData() { if (bodyType != AmqpMessageBodyType.DATA) { throw logger.logExceptionAsError(new IllegalArgumentException( - String.format(Locale.US, "This method can only be called for body type [%s]. Track this issue, " - + "https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type support in " - + "future.", AmqpMessageBodyType.DATA.toString()))); + "This method can only be called for AMQP Data body type at present. Track this issue, " + + "https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type support in " + + "future.")); } return new IterableStream<>(data); @@ -106,9 +105,9 @@ public byte[] getFirstData() { if (bodyType != AmqpMessageBodyType.DATA) { throw logger.logExceptionAsError(new IllegalArgumentException( - String.format(Locale.US, "This method can only be called for body type [%s]. Track this issue, " + "This method can only be called for AMQP Data body type at present. Track this issue, " + "https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type support in " - + "future.", AmqpMessageBodyType.DATA.toString()))); + + "future.")); } return data.get(0); From 79c54385a95525092a645413af64c215995ce178 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 17 Nov 2020 13:28:27 -0800 Subject: [PATCH 20/23] Removed copy constructors --- .../com/azure/messaging/servicebus/ServiceBusMessage.java | 6 ++---- 1 file changed, 2 insertions(+), 4 deletions(-) diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java index 71dfdf319a2d..587a19ec108d 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java @@ -18,8 +18,6 @@ import java.time.Duration; import java.time.OffsetDateTime; import java.time.ZoneOffset; -import java.util.Arrays; -import java.util.Locale; import java.util.Map; import java.util.Objects; @@ -126,8 +124,8 @@ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { AmqpMessageBody amqpMessageBody; switch (bodyType) { case DATA: - final byte[] data = receivedMessage.getAmqpAnnotatedMessage().getBody().getFirstData(); - amqpMessageBody = AmqpMessageBody.fromData(Arrays.copyOf(data, data.length)); + amqpMessageBody = AmqpMessageBody.fromData(receivedMessage.getAmqpAnnotatedMessage().getBody() + .getFirstData()); break; case SEQUENCE: case VALUE: From 627f57f54c88445bc2b6e52a1f0622df252b8afd Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 17 Nov 2020 15:53:40 -0800 Subject: [PATCH 21/23] Review comments , code snippets java doc etc --- .../resources/spotbugs/spotbugs-exclude.xml | 10 +++++++++- .../core/amqp/models/AmqpAnnotatedMessage.java | 7 +------ .../core/amqp/models/AmqpMessageBody.java | 18 ++++++++++-------- .../azure/core/amqp/models/AmqpMessageId.java | 2 +- ...AmqpAnnotatedMessageJavaDocCodeSamples.java | 10 ++++++++-- 5 files changed, 29 insertions(+), 18 deletions(-) diff --git a/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml b/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml index eb0ead6266a7..5f2bbda2112b 100755 --- a/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml +++ b/eng/code-quality-reports/src/main/resources/spotbugs/spotbugs-exclude.xml @@ -2297,7 +2297,7 @@ - + @@ -2430,4 +2430,12 @@ + + + + + + diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index 2dd5a520f423..f7d18b7eafe6 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -17,7 +17,6 @@ * @see AmqpMessageBody */ public final class AmqpAnnotatedMessage { - private final ClientLogger logger = new ClientLogger(AmqpAnnotatedMessage.class); private final AmqpMessageBody amqpMessageBody; private final Map applicationProperties; private final Map deliveryAnnotations; @@ -35,7 +34,6 @@ public final class AmqpAnnotatedMessage { */ public AmqpAnnotatedMessage(AmqpMessageBody body) { amqpMessageBody = Objects.requireNonNull(body, "'body' cannot be null."); - applicationProperties = new HashMap<>(); deliveryAnnotations = new HashMap<>(); messageAnnotations = new HashMap<>(); @@ -55,12 +53,9 @@ public Map getApplicationProperties() { /** * Gets the {@link AmqpMessageBody} of an amqp message. - * Client should test for {@link AmqpMessageBodyType} before calling corresponding get method on - * {@link AmqpMessageBody} - *

    How to check for {@link AmqpMessageBodyType}

    - * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} * * @return the {@link AmqpMessageBody} object. + * @see AmqpMessageBody */ public AmqpMessageBody getBody() { return amqpMessageBody; diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index 4ebc2815efaa..6f0b8fa70aad 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -28,7 +28,10 @@ public final class AmqpMessageBody { private final ClientLogger logger = new ClientLogger(AmqpMessageBody.class); private AmqpMessageBodyType bodyType; - private List data; + // We expect user to call `getFirstData()` more because we support one byte[] as present. + // This the priority here to store payload as `byte[] data` and + private byte[] data; + private List dataList; private AmqpMessageBody() { // package constructor so no one can create instance of this except classes im this package. @@ -47,7 +50,7 @@ public static AmqpMessageBody fromData(byte[] data) { Objects.requireNonNull(data, "'data' cannot be null."); AmqpMessageBody body = new AmqpMessageBody(); body.bodyType = AmqpMessageBodyType.DATA; - body.data = Collections.singletonList(data); + body.data = data; return body; } @@ -76,14 +79,15 @@ public AmqpMessageBodyType getBodyType() { */ public IterableStream getData() { if (bodyType != AmqpMessageBodyType.DATA) { - throw logger.logExceptionAsError(new IllegalArgumentException( "This method can only be called for AMQP Data body type at present. Track this issue, " + "https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type support in " + "future.")); } - - return new IterableStream<>(data); + if (dataList == null) { + dataList = Collections.singletonList(data); + } + return new IterableStream<>(dataList); } /** @@ -103,13 +107,11 @@ public IterableStream getData() { */ public byte[] getFirstData() { if (bodyType != AmqpMessageBodyType.DATA) { - throw logger.logExceptionAsError(new IllegalArgumentException( "This method can only be called for AMQP Data body type at present. Track this issue, " + "https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type support in " + "future.")); } - - return data.get(0); + return data; } } diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java index 9f8f180a84cb..4e525804499d 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java @@ -10,7 +10,7 @@ * {@link String} representation at present. * *

    Create and retrieve message id

    - * {@codesnippet com.azure.core.amqp.models.AmqpBodyType.checkBodyType} + * {@codesnippet com.azure.core.amqp.models.AmqpMessageId.createAndGet} * * Amqp message id types: *
      diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java index 8cca8bdd6659..3e561edd802c 100644 --- a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java +++ b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java @@ -13,11 +13,16 @@ public class AmqpAnnotatedMessageJavaDocCodeSamples { public void checkBodyType() { AmqpAnnotatedMessage amqpAnnotatedMessage = null; // BEGIN: com.azure.core.amqp.models.AmqpBodyType.checkBodyType + // If client do not check `AmqpMessageBody.getBodyType()` and payload is not of type `AmqpMessageBodyType.DATA`, + // calling `getFirstData()` or `getData()` on `AmqpMessageBody` will throw Runtime exception. + // https://github.com/Azure/azure-sdk-for-java/issues/17614 : This issue tracks additional AMQP body type + // support in future. + + byte[] payload = null; AmqpMessageBodyType bodyType = amqpAnnotatedMessage.getBody().getBodyType(); - AmqpMessageBody messageBody = null; switch (bodyType) { case DATA: - messageBody = AmqpMessageBody.fromData(amqpAnnotatedMessage.getBody().getFirstData()); + payload = amqpAnnotatedMessage.getBody().getFirstData(); break; case SEQUENCE: case VALUE: @@ -25,6 +30,7 @@ public void checkBodyType() { default: throw new RuntimeException("Body type not valid."); } + System.out.println(new String(payload)); // END: com.azure.core.amqp.models.AmqpBodyType.checkBodyType } } From 6c9667aa9134963ad6f27882617e18331bfd9a34 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Tue, 17 Nov 2020 20:05:24 -0800 Subject: [PATCH 22/23] fix copy constructor API change --- .../src/main/resources/revapi/revapi.json | 5 +++++ .../com/azure/core/amqp/models/AmqpAnnotatedMessage.java | 2 -- 2 files changed, 5 insertions(+), 2 deletions(-) diff --git a/eng/code-quality-reports/src/main/resources/revapi/revapi.json b/eng/code-quality-reports/src/main/resources/revapi/revapi.json index c0d9f16c1260..fa2c1d3d7739 100644 --- a/eng/code-quality-reports/src/main/resources/revapi/revapi.json +++ b/eng/code-quality-reports/src/main/resources/revapi/revapi.json @@ -350,6 +350,11 @@ "old": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setTo(===java.lang.String===)", "new": "parameter com.azure.core.amqp.models.AmqpMessageProperties com.azure.core.amqp.models.AmqpMessageProperties::setTo(===com.azure.core.amqp.models.AmqpAddress===)", "justification": "Introduced new type AmqpAddress." + }, + { + "code": "java.method.removed", + "old": "method void com.azure.core.amqp.models.AmqpAnnotatedMessage::(com.azure.core.amqp.models.AmqpAnnotatedMessage)", + "justification": "Removed copy constructor, It is not required for Service bus message." } ] } diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java index f7d18b7eafe6..6e2d2a8b394e 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpAnnotatedMessage.java @@ -3,8 +3,6 @@ package com.azure.core.amqp.models; -import com.azure.core.util.logging.ClientLogger; - import java.util.HashMap; import java.util.Map; import java.util.Objects; From c72a301e2ceda03fc8262b3d4e4de97f72e57818 Mon Sep 17 00:00:00 2001 From: Hemant Tanwar Date: Thu, 19 Nov 2020 09:23:16 -0800 Subject: [PATCH 23/23] Resolved more review comments --- .../core/amqp/models/AmqpMessageBody.java | 5 ++- .../azure/core/amqp/models/AmqpMessageId.java | 9 ++--- .../models/AmqpAddressJavaDocCodeSamples.java | 21 ------------ ...mqpAnnotatedMessageJavaDocCodeSamples.java | 22 +++++++++++++ .../AmqpMessageIdJavaDocCodeSamples.java | 21 ------------ .../servicebus/ServiceBusMessage.java | 7 ++-- .../ServiceBusMessageJavaDocCodeSamples.java | 33 ------------------- 7 files changed, 28 insertions(+), 90 deletions(-) delete mode 100644 sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAddressJavaDocCodeSamples.java delete mode 100644 sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpMessageIdJavaDocCodeSamples.java delete mode 100644 sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java index 6f0b8fa70aad..583755fa9396 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageBody.java @@ -34,7 +34,7 @@ public final class AmqpMessageBody { private List dataList; private AmqpMessageBody() { - // package constructor so no one can create instance of this except classes im this package. + // private constructor so no one outside can create instance of this except classes im this package. } /** @@ -92,8 +92,7 @@ public IterableStream getData() { /** * Gets first byte array set on this {@link AmqpMessageBody}. This library only support one byte array on Amqp - * Message. Look for future releases where we will support multiple byte array and you can use - * {@link AmqpMessageBody#getData()} API. + * Message at present. *

      Client should test for {@link AmqpMessageBodyType} before calling corresponding get method. Get methods not * corresponding to the type of the body throws exception.

      * diff --git a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java index 4e525804499d..714571632848 100644 --- a/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java +++ b/sdk/core/azure-core-amqp/src/main/java/com/azure/core/amqp/models/AmqpMessageId.java @@ -12,13 +12,8 @@ *

      Create and retrieve message id

      * {@codesnippet com.azure.core.amqp.models.AmqpMessageId.createAndGet} * - * Amqp message id types: - * + * @see + * String */ public final class AmqpMessageId { private final String messageId; diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAddressJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAddressJavaDocCodeSamples.java deleted file mode 100644 index 10b33680e97a..000000000000 --- a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAddressJavaDocCodeSamples.java +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.azure.core.amqp.models; - -/** - * Class contains sample code snippets that will be used in javadocs. - */ -public class AmqpAddressJavaDocCodeSamples { - /** - * Get message body from {@link AmqpAddress}. - */ - public void address() { - // BEGIN: com.azure.core.amqp.models.AmqpAddress.createAndGet - AmqpAddress amqpAddress = new AmqpAddress("my-address"); - // Retrieve Adderss - String address = amqpAddress.toString(); - System.out.println("Address " + address); - // END: com.azure.core.amqp.models.AmqpAddress.createAndGet - } -} diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java index 3e561edd802c..a37e5b9a10c6 100644 --- a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java +++ b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpAnnotatedMessageJavaDocCodeSamples.java @@ -33,4 +33,26 @@ public void checkBodyType() { System.out.println(new String(payload)); // END: com.azure.core.amqp.models.AmqpBodyType.checkBodyType } + + public void address() { + // BEGIN: com.azure.core.amqp.models.AmqpAddress.createAndGet + AmqpAddress amqpAddress = new AmqpAddress("my-address"); + // Retrieve Adderss + String address = amqpAddress.toString(); + System.out.println("Address " + address); + // END: com.azure.core.amqp.models.AmqpAddress.createAndGet + } + + /** + * Get message body from {@link AmqpMessageId}. + */ + public void messageId() { + // BEGIN: com.azure.core.amqp.models.AmqpMessageId.createAndGet + AmqpMessageId messageId = new AmqpMessageId("my-message-id"); + // Retrieve Message id + String id = messageId.toString(); + System.out.println("Message Id " + id); + // END: com.azure.core.amqp.models.AmqpMessageId.createAndGet + } + } diff --git a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpMessageIdJavaDocCodeSamples.java b/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpMessageIdJavaDocCodeSamples.java deleted file mode 100644 index f78d8661c737..000000000000 --- a/sdk/core/azure-core-amqp/src/samples/java/com/azure/core/amqp/models/AmqpMessageIdJavaDocCodeSamples.java +++ /dev/null @@ -1,21 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.azure.core.amqp.models; - -/** - * Class contains sample code snippets that will be used in javadocs. - */ -public class AmqpMessageIdJavaDocCodeSamples { - /** - * Get message body from {@link AmqpMessageId}. - */ - public void messageId() { - // BEGIN: com.azure.core.amqp.models.AmqpMessageId.createAndGet - AmqpMessageId messageId = new AmqpMessageId("my-message-id"); - // Retrieve Message id - String id = messageId.toString(); - System.out.println("Message Id " + id); - // END: com.azure.core.amqp.models.AmqpMessageId.createAndGet - } -} diff --git a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java index 587a19ec108d..febcc79775a5 100644 --- a/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java +++ b/sdk/servicebus/azure-messaging-servicebus/src/main/java/com/azure/messaging/servicebus/ServiceBusMessage.java @@ -110,12 +110,8 @@ public ServiceBusMessage(BinaryData body) { * * @throws NullPointerException if {@code receivedMessage} is {@code null}. * @throws UnsupportedOperationException if {@link AmqpMessageBodyType} is {@link AmqpMessageBodyType#SEQUENCE} or - * {@link AmqpMessageBodyType#VALUE}. See code sample above explaining how to check for {@link AmqpMessageBodyType} - * before calling this constructor. - *

      How to check for {@link AmqpMessageBodyType} before calling this constructor

      - * {@codesnippet com.azure.messaging.servicebus.ServiceBusMessage.copyServiceBusMessage} + * {@link AmqpMessageBodyType#VALUE}. * @throws IllegalStateException for invalid {@link AmqpMessageBodyType}. - * @see AmqpAnnotatedMessage */ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { Objects.requireNonNull(receivedMessage, "'receivedMessage' cannot be null."); @@ -129,6 +125,7 @@ public ServiceBusMessage(ServiceBusReceivedMessage receivedMessage) { break; case SEQUENCE: case VALUE: + // This should not happen because we will not create `ServiceBusReceivedMessage` with these types. throw logger.logExceptionAsError(new UnsupportedOperationException( "This constructor only supports the AMQP Data body type at present. Track this issue, " + "https://github.com/Azure/azure-sdk-for-java/issues/17614 for other body type support in " diff --git a/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java b/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java deleted file mode 100644 index 6439b03f32d2..000000000000 --- a/sdk/servicebus/azure-messaging-servicebus/src/samples/java/com/azure/messaging/servicebus/ServiceBusMessageJavaDocCodeSamples.java +++ /dev/null @@ -1,33 +0,0 @@ -// Copyright (c) Microsoft Corporation. All rights reserved. -// Licensed under the MIT License. - -package com.azure.messaging.servicebus; - -import com.azure.core.amqp.models.AmqpAnnotatedMessage; -import com.azure.core.amqp.models.AmqpMessageBodyType; - -/** - * java doc sample for {@link ServiceBusMessage}. - */ -public class ServiceBusMessageJavaDocCodeSamples { - /** - * Copy {@link AmqpAnnotatedMessage}. - */ - public void copyServiceBusReceivedMessage() { - ServiceBusReceivedMessage sourceServiceBusReceivedMessage = null; - // BEGIN: com.azure.messaging.servicebus.ServiceBusMessage.copyServiceBusMessage - AmqpMessageBodyType bodyType = sourceServiceBusReceivedMessage.getAmqpAnnotatedMessage().getBody().getBodyType(); - ServiceBusMessage copyMessage = null; - switch (bodyType) { - case DATA: - copyMessage = new ServiceBusMessage(sourceServiceBusReceivedMessage); - break; - case SEQUENCE: - case VALUE: - throw new RuntimeException("Body type not supported yet."); - default: - throw new RuntimeException("Body type not valid."); - } - // END: com.azure.messaging.servicebus.ServiceBusMessage.copyServiceBusMessage - } -}