mjsax commented on code in PR #21409: URL: https://github.com/apache/kafka/pull/21409#discussion_r2776336487
########## streams/src/test/java/org/apache/kafka/streams/state/HeadersBytesStoreTest.java: ########## @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.streams.state; + +import org.apache.kafka.common.utils.ByteUtils; + +import org.junit.jupiter.api.Test; + +import java.nio.ByteBuffer; + +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class HeadersBytesStoreTest { + + @Test + public void shouldReturnNullWhenConvertingNullValue() { + final byte[] result = HeadersBytesStore.convertToHeaderFormat(null); + assertNull(result); + } + + @Test + public void shouldConvertLegacyValueToHeaderFormat() { + final byte[] legacyValue = "test-value".getBytes(); + + final byte[] converted = HeadersBytesStore.convertToHeaderFormat(legacyValue); + + assertNotNull(converted); + assertTrue(converted.length > legacyValue.length, "converted bytes should have empty header bytes"); + } + + @Test + public void shouldConvertTimestampedValueToHeaderFormat() { Review Comment: Not sure if this gives much signal -- in the end, `convertToHeaderFormat` just add a single prefix `0x00` bytes -- it does know anything about the original "type" of the passed in `byte[]` array... So seems this test the same as `shouldConvertLegacyValueToHeaderFormat` ? ########## streams/src/test/java/org/apache/kafka/streams/state/HeadersBytesStoreTest.java: ########## @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.streams.state; + +import org.apache.kafka.common.utils.ByteUtils; + +import org.junit.jupiter.api.Test; + +import java.nio.ByteBuffer; + +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class HeadersBytesStoreTest { + + @Test + public void shouldReturnNullWhenConvertingNullValue() { + final byte[] result = HeadersBytesStore.convertToHeaderFormat(null); + assertNull(result); + } + + @Test + public void shouldConvertLegacyValueToHeaderFormat() { + final byte[] legacyValue = "test-value".getBytes(); + + final byte[] converted = HeadersBytesStore.convertToHeaderFormat(legacyValue); + + assertNotNull(converted); + assertTrue(converted.length > legacyValue.length, "converted bytes should have empty header bytes"); Review Comment: We know it should be exactly one byte larger, right? We could also check if `converted[0] == 0x00` and if the remainder is `converted[i] == legacyValue[i-1]` ? ########## streams/src/test/java/org/apache/kafka/streams/state/HeadersBytesStoreTest.java: ########## @@ -0,0 +1,110 @@ +/* + * Licensed to the Apache Software Foundation (ASF) under one or more + * contributor license agreements. See the NOTICE file distributed with + * this work for additional information regarding copyright ownership. + * The ASF licenses this file to You under the Apache License, Version 2.0 + * (the "License"); you may not use this file except in compliance with + * the License. You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.apache.kafka.streams.state; + +import org.apache.kafka.common.utils.ByteUtils; + +import org.junit.jupiter.api.Test; + +import java.nio.ByteBuffer; + +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.assertNull; +import static org.junit.jupiter.api.Assertions.assertTrue; + +public class HeadersBytesStoreTest { + + @Test + public void shouldReturnNullWhenConvertingNullValue() { + final byte[] result = HeadersBytesStore.convertToHeaderFormat(null); + assertNull(result); + } + + @Test + public void shouldConvertLegacyValueToHeaderFormat() { + final byte[] legacyValue = "test-value".getBytes(); + + final byte[] converted = HeadersBytesStore.convertToHeaderFormat(legacyValue); + + assertNotNull(converted); + assertTrue(converted.length > legacyValue.length, "converted bytes should have empty header bytes"); + } + + @Test + public void shouldConvertTimestampedValueToHeaderFormat() { + // Legacy timestamped format: [timestamp(8)][value] + final long timestamp = 123456789L; + final byte[] value = "test-value".getBytes(); + + final ByteBuffer legacyBuffer = ByteBuffer.allocate(Long.BYTES + value.length); + legacyBuffer.putLong(timestamp); + legacyBuffer.put(value); + final byte[] legacyValue = legacyBuffer.array(); + + final byte[] converted = HeadersBytesStore.convertToHeaderFormat(legacyValue); + + assertNotNull(converted); + + // Verify format: [headersSize(varint)][headersBytes][payload] + final ByteBuffer buffer = ByteBuffer.wrap(converted); + + final int headersSize = ByteUtils.readVarint(buffer); + assertEquals(0, headersSize, "Empty headers should have headersSize = 0"); + final byte[] payload = new byte[buffer.remaining()]; + buffer.get(payload); + assertArrayEquals(legacyValue, payload, "should keep original payload"); + } + + @Test + public void shouldConvertEmptyValueToHeaderFormat() { + final byte[] emptyValue = new byte[0]; + + final byte[] converted = HeadersBytesStore.convertToHeaderFormat(emptyValue); + + assertNotNull(converted); + assertTrue(converted.length > 0, "Converted value should have headers metadata"); + + final ByteBuffer buffer = ByteBuffer.wrap(converted); + final int headersSize = ByteUtils.readVarint(buffer); + assertEquals(0, headersSize, "Empty headers should have headersSize = 0"); + assertEquals(0, buffer.remaining(), "No payload bytes for empty value"); + } + + @Test + public void shouldHandleLargeValues() { Review Comment: Doesn't seem to give much signal? Do We need this? -- This is an automated message from the Apache Git Service. To respond to the message, please log on to GitHub and use the URL above to go to the specific comment. To unsubscribe, e-mail: [email protected] For queries about this service, please contact Infrastructure at: [email protected]
