From c74f338ca28ccc423160d7a51fb1c34f43ad10f3 Mon Sep 17 00:00:00 2001 From: scottf Date: Wed, 13 Dec 2023 11:10:00 -0500 Subject: [PATCH 1/5] Message Headers Improvement --- .../java/io/nats/client/impl/Headers.java | 23 +++- .../java/io/nats/client/impl/NatsMessage.java | 15 +-- .../io/nats/client/impl/HeadersTests.java | 112 ++++++++++-------- .../io/nats/client/impl/NatsMessageTests.java | 2 - 4 files changed, 87 insertions(+), 65 deletions(-) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index 2c1f01436..70ee4f39b 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -40,17 +40,28 @@ public class Headers { private int dataLength; public Headers() { - valuesMap = new HashMap<>(); - lengthMap = new HashMap<>(); + this(null, false); } public Headers(Headers headers) { - this(); + this(headers, false); + } + + public Headers(Headers headers, boolean readOnly) { + Map> tempValuesMap = new HashMap<>(); + Map tempLengthMap = new HashMap<>(); if (headers != null) { - valuesMap.putAll(headers.valuesMap); - lengthMap.putAll(headers.lengthMap); + tempValuesMap.putAll(headers.valuesMap); + tempLengthMap.putAll(headers.lengthMap); dataLength = headers.dataLength; - serialized = null; + } + if (readOnly) { + valuesMap = Collections.unmodifiableMap(tempValuesMap); + lengthMap = Collections.unmodifiableMap(tempLengthMap); + } + else { + valuesMap = tempValuesMap; + lengthMap = tempLengthMap; } } diff --git a/src/main/java/io/nats/client/impl/NatsMessage.java b/src/main/java/io/nats/client/impl/NatsMessage.java index dfe9ae4aa..f4f4db203 100644 --- a/src/main/java/io/nats/client/impl/NatsMessage.java +++ b/src/main/java/io/nats/client/impl/NatsMessage.java @@ -90,7 +90,7 @@ public NatsMessage(String subject, String replyTo, Headers headers, byte[] data) this(data); this.subject = validateSubject(subject, true); this.replyTo = validateReplyTo(replyTo, false); - this.headers = headers; + this.headers = copyOf(headers); this.utf8mode = false; finishConstruct(); } @@ -99,11 +99,15 @@ public NatsMessage(Message message) { this(message.getData()); this.subject = message.getSubject(); this.replyTo = message.getReplyTo(); - this.headers = message.getHeaders(); + this.headers = copyOf(message.getHeaders()); this.utf8mode = message.isUtf8mode(); finishConstruct(); } + private static Headers copyOf(Headers headers) { + return headers == null ? null : new Headers(headers, true); + } + protected void finishConstruct() { int replyToLen = replyTo == null ? 0 : replyTo.length(); @@ -167,13 +171,6 @@ int getControlLineLength() { return controlLineLength; } - Headers getOrCreateHeaders() { - if (headers == null) { - headers = new Headers(); - } - return headers; - } - void setSubscription(NatsSubscription sub) { subscription = sub; } diff --git a/src/test/java/io/nats/client/impl/HeadersTests.java b/src/test/java/io/nats/client/impl/HeadersTests.java index 3afb985e8..d95a40a45 100644 --- a/src/test/java/io/nats/client/impl/HeadersTests.java +++ b/src/test/java/io/nats/client/impl/HeadersTests.java @@ -1,5 +1,6 @@ package io.nats.client.impl; +import io.nats.client.Message; import io.nats.client.support.IncomingHeadersProcessor; import io.nats.client.support.Status; import io.nats.client.support.Token; @@ -30,30 +31,29 @@ public class HeadersTests { @Test public void add_key_strings_works() { add( - headers -> headers.add(KEY1, VAL1), - headers -> headers.add(KEY1, VAL2), - headers -> headers.add(KEY2, VAL3), - headers -> { headers.add(KEY1_ALT, VAL4); headers.add(KEY1_ALT, VAL5); } - ); + headers -> headers.add(KEY1, VAL1), + headers -> headers.add(KEY1, VAL2), + headers -> headers.add(KEY2, VAL3), + headers -> { headers.add(KEY1_ALT, VAL4); headers.add(KEY1_ALT, VAL5); } + ); } @Test public void add_key_collection_works() { add( - headers -> headers.add(KEY1, Collections.singletonList(VAL1)), - headers -> headers.add(KEY1, Collections.singletonList(VAL2)), - headers -> headers.add(KEY2, Collections.singletonList(VAL3)), - headers -> headers.add(KEY1_ALT, Arrays.asList(VAL4, VAL5)) + headers -> headers.add(KEY1, Collections.singletonList(VAL1)), + headers -> headers.add(KEY1, Collections.singletonList(VAL2)), + headers -> headers.add(KEY2, Collections.singletonList(VAL3)), + headers -> headers.add(KEY1_ALT, Arrays.asList(VAL4, VAL5)) ); } private void add( - Consumer stepKey1Val1, - Consumer step2Key1Val2, - Consumer step3Key2Val3, - Consumer step4Key1AVal4Val5 - ) - { + Consumer stepKey1Val1, + Consumer step2Key1Val2, + Consumer step3Key2Val3, + Consumer step4Key1AVal4Val5 + ) { Headers headers = new Headers(); stepKey1Val1.accept(headers); @@ -92,31 +92,31 @@ private void add( @Test public void put_key_strings_works() { put( - headers -> headers.put(KEY1, VAL1), - headers -> headers.put(KEY1, VAL2), - headers -> headers.put(KEY2, VAL3), - headers -> headers.put(KEY1_ALT, VAL4), - headers -> headers.put(KEY1_OTHER, VAL5) + headers -> headers.put(KEY1, VAL1), + headers -> headers.put(KEY1, VAL2), + headers -> headers.put(KEY2, VAL3), + headers -> headers.put(KEY1_ALT, VAL4), + headers -> headers.put(KEY1_OTHER, VAL5) ); } @Test public void put_key_collection_works() { put( - headers -> headers.put(KEY1, Collections.singletonList(VAL1)), - headers -> headers.put(KEY1, Collections.singletonList(VAL2)), - headers -> headers.put(KEY2, Collections.singletonList(VAL3)), - headers -> headers.put(KEY1_ALT, Collections.singletonList(VAL4)), - headers -> headers.put(KEY1_OTHER, Collections.singletonList(VAL5)) + headers -> headers.put(KEY1, Collections.singletonList(VAL1)), + headers -> headers.put(KEY1, Collections.singletonList(VAL2)), + headers -> headers.put(KEY2, Collections.singletonList(VAL3)), + headers -> headers.put(KEY1_ALT, Collections.singletonList(VAL4)), + headers -> headers.put(KEY1_OTHER, Collections.singletonList(VAL5)) ); } private void put( - Consumer step1PutKey1Val1, - Consumer step2PutKey1Val2, - Consumer step3PutKey2Val3, - Consumer step4PutKey1AVal4, - Consumer step6PutKey1HVal5) + Consumer step1PutKey1Val1, + Consumer step2PutKey1Val2, + Consumer step3PutKey2Val3, + Consumer step4PutKey1AVal4, + Consumer step6PutKey1HVal5) { Headers headers = new Headers(); assertTrue(headers.isEmpty()); @@ -209,6 +209,22 @@ private void assertContainsKeys(Headers headers, int countUniqueKeys, List headers1.put(KEY1, VAL2)); + assertThrows(UnsupportedOperationException.class, () -> headers1.put(KEY2, VAL2)); + assertEquals(VAL1, headers1.getFirst(KEY1)); + + Message m = new NatsMessage("subject", null, notRO, null); + Headers headers2 = m.getHeaders(); + assertThrows(UnsupportedOperationException.class, () -> headers2.put(KEY1, VAL2)); + assertThrows(UnsupportedOperationException.class, () -> headers2.put(KEY2, VAL2)); + assertEquals(VAL1, headers2.getFirst(KEY1)); + } + @Test public void keyCannotBeNullOrEmpty() { Headers headers = new Headers(); @@ -339,20 +355,20 @@ public void valueCharactersMustBePrintableOrTab() { @Test public void remove_string_work() { remove( - headers -> headers.remove(KEY1), - headers -> headers.remove(KEY1_ALT), - headers -> headers.remove(KEY1_OTHER), - headers -> headers.remove(KEY2, KEY3) + headers -> headers.remove(KEY1), + headers -> headers.remove(KEY1_ALT), + headers -> headers.remove(KEY1_OTHER), + headers -> headers.remove(KEY2, KEY3) ); } @Test public void remove_collection_work() { remove( - headers -> headers.remove(Collections.singletonList(KEY1)), - headers -> headers.remove(Collections.singletonList(KEY1_ALT)), - headers -> headers.remove(Collections.singletonList(KEY1_OTHER)), - headers -> headers.remove(Arrays.asList(KEY2, KEY3)) + headers -> headers.remove(Collections.singletonList(KEY1)), + headers -> headers.remove(Collections.singletonList(KEY1_ALT)), + headers -> headers.remove(Collections.singletonList(KEY1_OTHER)), + headers -> headers.remove(Arrays.asList(KEY2, KEY3)) ); } @@ -373,10 +389,10 @@ public void testGetFirstGetLast() { } private void remove( - Consumer step1RemoveKey1, - Consumer step2RemoveKey1A, - Consumer step3RemoveKey1H, - Consumer step4RemoveKey2Key3) + Consumer step1RemoveKey1, + Consumer step2RemoveKey1A, + Consumer step3RemoveKey1H, + Consumer step4RemoveKey2Key3) { Headers headers = testHeaders(); @@ -711,19 +727,19 @@ public void constructorWithHeaders() { public void testToken() { byte[] serialized1 = "notspaceorcrlf".getBytes(StandardCharsets.US_ASCII); assertThrows(IllegalArgumentException.class, - () -> new Token(serialized1, serialized1.length, 0, TokenType.WORD)); + () -> new Token(serialized1, serialized1.length, 0, TokenType.WORD)); assertThrows(IllegalArgumentException.class, - () -> new Token(serialized1, serialized1.length, 0, TokenType.KEY)); + () -> new Token(serialized1, serialized1.length, 0, TokenType.KEY)); assertThrows(IllegalArgumentException.class, - () -> new Token(serialized1, serialized1.length, 0, TokenType.SPACE)); + () -> new Token(serialized1, serialized1.length, 0, TokenType.SPACE)); assertThrows(IllegalArgumentException.class, - () -> new Token(serialized1, serialized1.length, 0, TokenType.CRLF)); + () -> new Token(serialized1, serialized1.length, 0, TokenType.CRLF)); byte[] serialized2 = "\r".getBytes(StandardCharsets.US_ASCII); assertThrows(IllegalArgumentException.class, - () -> new Token(serialized2, serialized2.length, 0, TokenType.CRLF)); + () -> new Token(serialized2, serialized2.length, 0, TokenType.CRLF)); byte[] serialized3 = "\rnotlf".getBytes(StandardCharsets.US_ASCII); assertThrows(IllegalArgumentException.class, - () -> new Token(serialized3, serialized3.length, 0, TokenType.CRLF)); + () -> new Token(serialized3, serialized3.length, 0, TokenType.CRLF)); Token t = new Token("k1:v1\r\n\r\n".getBytes(StandardCharsets.US_ASCII), 9, 0, TokenType.KEY); t.mustBe(TokenType.KEY); assertThrows(IllegalArgumentException.class, () -> t.mustBe(TokenType.CRLF)); diff --git a/src/test/java/io/nats/client/impl/NatsMessageTests.java b/src/test/java/io/nats/client/impl/NatsMessageTests.java index c88e2012c..aeff74e94 100644 --- a/src/test/java/io/nats/client/impl/NatsMessageTests.java +++ b/src/test/java/io/nats/client/impl/NatsMessageTests.java @@ -240,7 +240,6 @@ public void miscCoverage() { assertNull(m.getConnection()); assertEquals(23, m.getControlLineLength()); assertNotNull(m.toDetailString()); // COVERAGE - assertNotNull(m.getOrCreateHeaders()); m.getHeaders().remove("key"); assertFalse(m.hasHeaders()); @@ -250,7 +249,6 @@ public void miscCoverage() { assertFalse(m.hasHeaders()); assertNull(m.getHeaders()); assertNotNull(m.toString()); // COVERAGE - assertNotNull(m.getOrCreateHeaders()); ProtocolMessage pm = new ProtocolMessage((byte[])null); assertNotNull(pm.protocolBab); From 6842b45bc7eb95a377fca190d25d40850ba918a2 Mon Sep 17 00:00:00 2001 From: scottf Date: Wed, 13 Dec 2023 11:52:37 -0500 Subject: [PATCH 2/5] fixed tests --- .../java/io/nats/client/api/MessageInfo.java | 14 ++--- .../java/io/nats/client/impl/Headers.java | 61 +++++++++++-------- .../client/impl/JetStreamManagementTests.java | 5 ++ .../io/nats/client/impl/NatsMessageTests.java | 4 -- 4 files changed, 47 insertions(+), 37 deletions(-) diff --git a/src/main/java/io/nats/client/api/MessageInfo.java b/src/main/java/io/nats/client/api/MessageInfo.java index 4d2c143ef..6a191aa4b 100644 --- a/src/main/java/io/nats/client/api/MessageInfo.java +++ b/src/main/java/io/nats/client/api/MessageInfo.java @@ -64,13 +64,13 @@ public MessageInfo(Message msg, String streamName, boolean direct) { this.direct = direct; if (direct) { - this.headers = msg.getHeaders(); - this.subject = headers.getLast(NATS_SUBJECT); + Headers msgHeaders = msg.getHeaders(); + this.subject = msgHeaders.getLast(NATS_SUBJECT); this.data = msg.getData(); - seq = Long.parseLong(headers.getFirst(NATS_SEQUENCE)); - time = DateTimeUtils.parseDateTime(headers.getFirst(NATS_TIMESTAMP)); - stream = headers.getFirst(NATS_STREAM); - String temp = headers.getFirst(NATS_LAST_SEQUENCE); + seq = Long.parseLong(msgHeaders.getFirst(NATS_SEQUENCE)); + time = DateTimeUtils.parseDateTime(msgHeaders.getFirst(NATS_TIMESTAMP)); + stream = msgHeaders.getFirst(NATS_STREAM); + String temp = msgHeaders.getFirst(NATS_LAST_SEQUENCE); if (temp == null) { lastSeq = -1; } @@ -78,7 +78,7 @@ public MessageInfo(Message msg, String streamName, boolean direct) { lastSeq = JsonUtils.safeParseLong(temp, -1); } // these are control headers, not real headers so don't give them to the user. - headers.remove(NATS_SUBJECT, NATS_SEQUENCE, NATS_TIMESTAMP, NATS_STREAM, NATS_LAST_SEQUENCE); + headers = new Headers(msgHeaders, true, NATS_SUBJECT, NATS_SEQUENCE, NATS_TIMESTAMP, NATS_STREAM, NATS_LAST_SEQUENCE); } else if (hasError()) { subject = null; diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index 70ee4f39b..47de40438 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -25,7 +25,7 @@ * An object that represents a map of keys to a list of values. It does not accept * null or invalid keys. It ignores null values, accepts empty string as a value * and rejects invalid values. - * + * !!! * THIS CLASS IS NOT THREAD SAFE */ public class Headers { @@ -40,20 +40,29 @@ public class Headers { private int dataLength; public Headers() { - this(null, false); + this(null, false, (String[])null); } public Headers(Headers headers) { - this(headers, false); + this(headers, false, (String[])null); } - public Headers(Headers headers, boolean readOnly) { + public Headers(Headers headers, boolean readOnly, String... keysToRemoveWhenCopying) { Map> tempValuesMap = new HashMap<>(); Map tempLengthMap = new HashMap<>(); if (headers != null) { tempValuesMap.putAll(headers.valuesMap); tempLengthMap.putAll(headers.lengthMap); dataLength = headers.dataLength; + if (keysToRemoveWhenCopying != null) { + for (String key : keysToRemoveWhenCopying) { + if (key != null) { + if (tempValuesMap.remove(key) != null) { + dataLength -= tempLengthMap.remove(key); + } + } + } + } } if (readOnly) { valuesMap = Collections.unmodifiableMap(tempValuesMap); @@ -166,7 +175,7 @@ public Headers put(Map> map) { // the put delegate that all puts call private void _put(String key, Collection values) { - if (key == null || key.length() == 0) { + if (key == null || key.isEmpty()) { throw new IllegalArgumentException("Key cannot be null or empty."); } if (values != null) { @@ -214,7 +223,7 @@ private void _remove(String key) { } /** - * Returns the number of keys (case sensitive) in the header. + * Returns the number of keys (case-sensitive) in the header. * * @return the number of keys */ @@ -232,7 +241,7 @@ public boolean isEmpty() { } /** - * Removes all of the keys The object map will be empty after this call returns. + * Removes all the keys The object map will be empty after this call returns. */ public void clear() { valuesMap.clear(); @@ -242,20 +251,20 @@ public void clear() { } /** - * Returns true if key (case sensitive) is present (has values) + * Returns true if key (case-sensitive) is present (has values) * * @param key key whose presence is to be tested - * @return true if the key (case sensitive) is present (has values) + * @return true if the key (case-sensitive) is present (has values) */ public boolean containsKey(String key) { return valuesMap.containsKey(key); } /** - * Returns true if key (case insensitive) is present (has values) + * Returns true if key (case-insensitive) is present (has values) * * @param key exact key whose presence is to be tested - * @return true if the key (case insensitive) is present (has values) + * @return true if the key (case-insensitive) is present (has values) */ public boolean containsKeyIgnoreCase(String key) { for (String k : valuesMap.keySet()) { @@ -267,7 +276,7 @@ public boolean containsKeyIgnoreCase(String key) { } /** - * Returns a {@link Set} view of the keys (case sensitive) contained in the object. + * Returns a {@link Set} view of the keys (case-sensitive) contained in the object. * * @return a read-only set the keys contained in this map */ @@ -276,7 +285,7 @@ public Set keySet() { } /** - * Returns a {@link Set} view of the keys (case insensitive) contained in the object. + * Returns a {@link Set} view of the keys (case-insensitive) contained in the object. * * @return a read-only set of keys (in lowercase) contained in this map */ @@ -289,10 +298,10 @@ public Set keySetIgnoreCase() { } /** - * Returns a {@link List} view of the values for the specific (case sensitive) key. + * Returns a {@link List} view of the values for the specific (case-sensitive) key. * Will be {@code null} if the key is not found. * - * @return a read-only list of the values for the case sensitive key. + * @return a read-only list of the values for the case-sensitive key. */ public List get(String key) { List values = valuesMap.get(key); @@ -300,10 +309,10 @@ public List get(String key) { } /** - * Returns the first value for the specific (case sensitive) key. + * Returns the first value for the specific (case-sensitive) key. * Will be {@code null} if the key is not found. * - * @return the first value for the case sensitive key. + * @return the first value for the case-sensitive key. */ public String getFirst(String key) { List values = valuesMap.get(key); @@ -311,10 +320,10 @@ public String getFirst(String key) { } /** - * Returns the last value for the specific (case sensitive) key. + * Returns the last value for the specific (case-sensitive) key. * Will be {@code null} if the key is not found. * - * @return the last value for the case sensitive key. + * @return the last value for the case-sensitive key. */ public String getLast(String key) { List values = valuesMap.get(key); @@ -322,10 +331,10 @@ public String getLast(String key) { } /** - * Returns a {@link List} view of the values for the specific (case insensitive) key. + * Returns a {@link List} view of the values for the specific (case-insensitive) key. * Will be {@code null} if the key is not found. * - * @return a read-only list of the values for the case insensitive key. + * @return a read-only list of the values for the case-insensitive key. */ public List getIgnoreCase(String key) { List values = new ArrayList<>(); @@ -334,11 +343,11 @@ public List getIgnoreCase(String key) { values.addAll(valuesMap.get(k)); } } - return values.size() == 0 ? null : Collections.unmodifiableList(values); + return values.isEmpty() ? null : Collections.unmodifiableList(values); } /** - * Performs the given action for each header entry (case sensitive keys) until all entries + * Performs the given action for each header entry (case-sensitive keys) until all entries * have been processed or the action throws an exception. * Any attempt to modify the values will throw an exception. * @@ -352,7 +361,7 @@ public void forEach(BiConsumer> action) { } /** - * Returns a {@link Set} read only view of the mappings contained in the header (case sensitive keys). + * Returns a {@link Set} read only view of the mappings contained in the header (case-sensitive keys). * The set is not modifiable and any attempt to modify will throw an exception. * * @return a set view of the mappings contained in this map @@ -458,7 +467,7 @@ public int serializeToArray(int destPosition, byte[] dest) { */ private void checkKey(String key) { // key cannot be null or empty and contain only printable characters except colon - if (key == null || key.length() == 0) { + if (key == null || key.isEmpty()) { throw new IllegalArgumentException(KEY_CANNOT_BE_EMPTY_OR_NULL); } @@ -511,7 +520,7 @@ private class Checker { } boolean hasValues() { - return list.size() > 0; + return !list.isEmpty(); } } diff --git a/src/test/java/io/nats/client/impl/JetStreamManagementTests.java b/src/test/java/io/nats/client/impl/JetStreamManagementTests.java index ed5bd2857..279ebe4aa 100644 --- a/src/test/java/io/nats/client/impl/JetStreamManagementTests.java +++ b/src/test/java/io/nats/client/impl/JetStreamManagementTests.java @@ -1193,6 +1193,11 @@ private void assertMessageInfo(TestingStreamContainer tsc, int subj, long seq, M String expectedData = "s" + subj + "-q" + seq; assertEquals("d-" + expectedData, new String(mi.getData())); assertEquals("h-" + expectedData, mi.getHeaders().getFirst("h")); + assertNull(mi.getHeaders().getFirst(NATS_SUBJECT)); + assertNull(mi.getHeaders().getFirst(NATS_SEQUENCE)); + assertNull(mi.getHeaders().getFirst(NATS_TIMESTAMP)); + assertNull(mi.getHeaders().getFirst(NATS_STREAM)); + assertNull(mi.getHeaders().getFirst(NATS_LAST_SEQUENCE)); } @SuppressWarnings("deprecation") diff --git a/src/test/java/io/nats/client/impl/NatsMessageTests.java b/src/test/java/io/nats/client/impl/NatsMessageTests.java index aeff74e94..5b8286357 100644 --- a/src/test/java/io/nats/client/impl/NatsMessageTests.java +++ b/src/test/java/io/nats/client/impl/NatsMessageTests.java @@ -241,10 +241,6 @@ public void miscCoverage() { assertEquals(23, m.getControlLineLength()); assertNotNull(m.toDetailString()); // COVERAGE - m.getHeaders().remove("key"); - assertFalse(m.hasHeaders()); - assertNotNull(m.getHeaders()); - m.headers = null; // we can do this because we have package access assertFalse(m.hasHeaders()); assertNull(m.getHeaders()); From acdc511451d2616a75cd2e80d23f6cce38ff8293 Mon Sep 17 00:00:00 2001 From: scottf Date: Wed, 13 Dec 2023 12:43:42 -0500 Subject: [PATCH 3/5] added read-only flag and tested it --- src/main/java/io/nats/client/impl/Headers.java | 10 ++++++++++ src/test/java/io/nats/client/impl/HeadersTests.java | 3 +++ 2 files changed, 13 insertions(+) diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index 47de40438..a46f0512d 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -36,6 +36,7 @@ public class Headers { private final Map> valuesMap; private final Map lengthMap; + private final boolean readOnly; private byte[] serialized; private int dataLength; @@ -64,6 +65,7 @@ public Headers(Headers headers, boolean readOnly, String... keysToRemoveWhenCopy } } } + this.readOnly = readOnly; if (readOnly) { valuesMap = Collections.unmodifiableMap(tempValuesMap); lengthMap = Collections.unmodifiableMap(tempLengthMap); @@ -524,6 +526,14 @@ boolean hasValues() { } } + /** + * Whether the entire Headers is read only + * @return the read only state + */ + public boolean isReadOnly() { + return readOnly; + } + @Override public boolean equals(Object o) { if (this == o) return true; diff --git a/src/test/java/io/nats/client/impl/HeadersTests.java b/src/test/java/io/nats/client/impl/HeadersTests.java index d95a40a45..f3c39bd07 100644 --- a/src/test/java/io/nats/client/impl/HeadersTests.java +++ b/src/test/java/io/nats/client/impl/HeadersTests.java @@ -213,13 +213,16 @@ private void assertContainsKeys(Headers headers, int countUniqueKeys, List headers1.put(KEY1, VAL2)); assertThrows(UnsupportedOperationException.class, () -> headers1.put(KEY2, VAL2)); assertEquals(VAL1, headers1.getFirst(KEY1)); Message m = new NatsMessage("subject", null, notRO, null); Headers headers2 = m.getHeaders(); + assertTrue(headers2.isReadOnly()); assertThrows(UnsupportedOperationException.class, () -> headers2.put(KEY1, VAL2)); assertThrows(UnsupportedOperationException.class, () -> headers2.put(KEY2, VAL2)); assertEquals(VAL1, headers2.getFirst(KEY1)); From f9a64f518dc1f7e7b08896fc46ec9fc27db8bc9e Mon Sep 17 00:00:00 2001 From: scottf Date: Wed, 13 Dec 2023 13:08:36 -0500 Subject: [PATCH 4/5] added read-only flag and tested it --- src/test/java/io/nats/client/impl/HeadersTests.java | 8 ++++++-- 1 file changed, 6 insertions(+), 2 deletions(-) diff --git a/src/test/java/io/nats/client/impl/HeadersTests.java b/src/test/java/io/nats/client/impl/HeadersTests.java index f3c39bd07..2fd5e31fb 100644 --- a/src/test/java/io/nats/client/impl/HeadersTests.java +++ b/src/test/java/io/nats/client/impl/HeadersTests.java @@ -217,14 +217,18 @@ public void testReadOnly() { Headers headers1 = new Headers(notRO, true); assertTrue(headers1.isReadOnly()); assertThrows(UnsupportedOperationException.class, () -> headers1.put(KEY1, VAL2)); - assertThrows(UnsupportedOperationException.class, () -> headers1.put(KEY2, VAL2)); + assertThrows(UnsupportedOperationException.class, () -> headers1.put(KEY1, VAL2)); + assertThrows(UnsupportedOperationException.class, () -> headers1.remove(KEY1)); + assertThrows(UnsupportedOperationException.class, headers1::clear); assertEquals(VAL1, headers1.getFirst(KEY1)); Message m = new NatsMessage("subject", null, notRO, null); Headers headers2 = m.getHeaders(); assertTrue(headers2.isReadOnly()); assertThrows(UnsupportedOperationException.class, () -> headers2.put(KEY1, VAL2)); - assertThrows(UnsupportedOperationException.class, () -> headers2.put(KEY2, VAL2)); + assertThrows(UnsupportedOperationException.class, () -> headers2.put(KEY1, VAL2)); + assertThrows(UnsupportedOperationException.class, () -> headers2.remove(KEY1)); + assertThrows(UnsupportedOperationException.class, headers2::clear); assertEquals(VAL1, headers2.getFirst(KEY1)); } From 499c6a62b5145492b03a4cbdb7308f63ce874fcc Mon Sep 17 00:00:00 2001 From: scottf Date: Wed, 13 Dec 2023 13:17:15 -0500 Subject: [PATCH 5/5] read-only better --- src/main/java/io/nats/client/api/MessageInfo.java | 2 +- src/main/java/io/nats/client/impl/Headers.java | 14 +++++++++----- src/main/java/io/nats/client/impl/NatsMessage.java | 11 +++++++---- .../client/support/NatsJetStreamConstants.java | 1 + 4 files changed, 18 insertions(+), 10 deletions(-) diff --git a/src/main/java/io/nats/client/api/MessageInfo.java b/src/main/java/io/nats/client/api/MessageInfo.java index 6a191aa4b..60374beed 100644 --- a/src/main/java/io/nats/client/api/MessageInfo.java +++ b/src/main/java/io/nats/client/api/MessageInfo.java @@ -78,7 +78,7 @@ public MessageInfo(Message msg, String streamName, boolean direct) { lastSeq = JsonUtils.safeParseLong(temp, -1); } // these are control headers, not real headers so don't give them to the user. - headers = new Headers(msgHeaders, true, NATS_SUBJECT, NATS_SEQUENCE, NATS_TIMESTAMP, NATS_STREAM, NATS_LAST_SEQUENCE); + headers = new Headers(msgHeaders, true, MESSAGE_INFO_HEADERS); } else if (hasError()) { subject = null; diff --git a/src/main/java/io/nats/client/impl/Headers.java b/src/main/java/io/nats/client/impl/Headers.java index a46f0512d..dba1d62ef 100644 --- a/src/main/java/io/nats/client/impl/Headers.java +++ b/src/main/java/io/nats/client/impl/Headers.java @@ -41,22 +41,26 @@ public class Headers { private int dataLength; public Headers() { - this(null, false, (String[])null); + this(null, false, null); } public Headers(Headers headers) { - this(headers, false, (String[])null); + this(headers, false, null); } - public Headers(Headers headers, boolean readOnly, String... keysToRemoveWhenCopying) { + public Headers(Headers headers, boolean readOnly) { + this(headers, readOnly, null); + } + + public Headers(Headers headers, boolean readOnly, String[] keysNotToCopy) { Map> tempValuesMap = new HashMap<>(); Map tempLengthMap = new HashMap<>(); if (headers != null) { tempValuesMap.putAll(headers.valuesMap); tempLengthMap.putAll(headers.lengthMap); dataLength = headers.dataLength; - if (keysToRemoveWhenCopying != null) { - for (String key : keysToRemoveWhenCopying) { + if (keysNotToCopy != null) { + for (String key : keysNotToCopy) { if (key != null) { if (tempValuesMap.remove(key) != null) { dataLength -= tempLengthMap.remove(key); diff --git a/src/main/java/io/nats/client/impl/NatsMessage.java b/src/main/java/io/nats/client/impl/NatsMessage.java index f4f4db203..8cc17fae5 100644 --- a/src/main/java/io/nats/client/impl/NatsMessage.java +++ b/src/main/java/io/nats/client/impl/NatsMessage.java @@ -90,7 +90,7 @@ public NatsMessage(String subject, String replyTo, Headers headers, byte[] data) this(data); this.subject = validateSubject(subject, true); this.replyTo = validateReplyTo(replyTo, false); - this.headers = copyOf(headers); + this.headers = readOnlyOf(headers); this.utf8mode = false; finishConstruct(); } @@ -99,13 +99,16 @@ public NatsMessage(Message message) { this(message.getData()); this.subject = message.getSubject(); this.replyTo = message.getReplyTo(); - this.headers = copyOf(message.getHeaders()); + this.headers = readOnlyOf(message.getHeaders()); this.utf8mode = message.isUtf8mode(); finishConstruct(); } - private static Headers copyOf(Headers headers) { - return headers == null ? null : new Headers(headers, true); + private static Headers readOnlyOf(Headers headers) { + if (headers == null || headers.isReadOnly()) { + return headers; + } + return new Headers(headers, true, null); } protected void finishConstruct() { diff --git a/src/main/java/io/nats/client/support/NatsJetStreamConstants.java b/src/main/java/io/nats/client/support/NatsJetStreamConstants.java index d15ff1e8c..bcf04d285 100644 --- a/src/main/java/io/nats/client/support/NatsJetStreamConstants.java +++ b/src/main/java/io/nats/client/support/NatsJetStreamConstants.java @@ -97,6 +97,7 @@ public interface NatsJetStreamConstants { String NATS_TIMESTAMP = "Nats-Time-Stamp"; String NATS_SUBJECT = "Nats-Subject"; String NATS_LAST_SEQUENCE = "Nats-Last-Sequence"; + String[] MESSAGE_INFO_HEADERS = new String[]{NATS_SUBJECT, NATS_SEQUENCE, NATS_TIMESTAMP, NATS_STREAM, NATS_LAST_SEQUENCE}; String NATS_PENDING_MESSAGES = "Nats-Pending-Messages"; String NATS_PENDING_BYTES = "Nats-Pending-Bytes";