Skip to content

Commit 8c29224

Browse files
YutaLinYutaLin
committed
refactor: add comprehensive runTest in TestKeyPrefixContainerCodec
1 parent b878791 commit 8c29224

File tree

2 files changed

+98
-95
lines changed

2 files changed

+98
-95
lines changed

‎hadoop-ozone/recon/src/main/java/org/apache/hadoop/ozone/recon/spi/impl/KeyPrefixContainerCodec.java‎

Lines changed: 60 additions & 63 deletions
Original file line numberDiff line numberDiff line change
@@ -44,7 +44,7 @@ public final class KeyPrefixContainerCodec
4444
private static final String KEY_DELIMITER = "_";
4545
private static final byte[] KEY_DELIMITER_BYTES = KEY_DELIMITER.getBytes(UTF_8);
4646
private static final ByteBuffer KEY_DELIMITER_BUFFER = ByteBuffer.wrap(KEY_DELIMITER_BYTES).asReadOnlyBuffer();
47-
private static final int LONG_SERIALIZED_SIZE = KEY_DELIMITER_BYTES.length + Long.BYTES;
47+
public static final int LONG_SERIALIZED_SIZE = KEY_DELIMITER_BYTES.length + Long.BYTES;
4848

4949
public static Codec<KeyPrefixContainer> get() {
5050
return INSTANCE;
@@ -102,53 +102,59 @@ public KeyPrefixContainer fromCodecBuffer(@Nonnull CodecBuffer buffer) throws Co
102102
final int startPosition = byteBuffer.position();
103103
final int delimiterLength = KEY_DELIMITER_BYTES.length;
104104

105-
// Only support decoding when we have both version and containerId
106-
if (totalLength >= 2 * (delimiterLength + Long.BYTES)) {
107-
// Extract keyPrefix (everything except last 2 delimiters + 2 longs)
108-
int keyPrefixLength = totalLength - 2 * delimiterLength - 2 * Long.BYTES;
109-
byteBuffer.position(startPosition);
110-
String keyPrefix = decodeStringFromBuffer(byteBuffer, keyPrefixLength);
105+
// We expect: keyPrefix + delimiter + version(8 bytes) + delimiter + containerId(8 bytes)
106+
final int minimumLength = delimiterLength + Long.BYTES + delimiterLength + Long.BYTES;
111107

112-
// Skip delimiter and read version
113-
byteBuffer.position(startPosition + keyPrefixLength + delimiterLength);
114-
long version = byteBuffer.getLong();
115-
116-
// Skip delimiter and read containerId
117-
byteBuffer.position(startPosition + keyPrefixLength + delimiterLength + Long.BYTES + delimiterLength);
118-
long containerId = byteBuffer.getLong();
108+
if (totalLength < minimumLength) {
109+
throw new CodecException("Buffer too small to contain all required fields.");
110+
}
119111

120-
return KeyPrefixContainer.get(keyPrefix, version, containerId);
112+
int keyPrefixLength = totalLength - 2 * delimiterLength - 2 * Long.BYTES;
113+
if (keyPrefixLength < 0) {
114+
throw new CodecException("Invalid buffer format: negative key prefix length");
121115
}
122116

123-
// For backwards compatibility during transition, treat anything else as keyPrefix only
124-
// This should not be used in production as it cannot handle keys with underscores
125117
byteBuffer.position(startPosition);
126-
String keyPrefix = decodeStringFromBuffer(byteBuffer, totalLength);
127-
return KeyPrefixContainer.get(keyPrefix, -1, -1);
118+
byteBuffer.limit(startPosition + keyPrefixLength);
119+
String keyPrefix = decodeStringFromBuffer(byteBuffer);
120+
byteBuffer.limit(startPosition + totalLength);
121+
122+
byteBuffer.position(startPosition + keyPrefixLength);
123+
for (int i = 0; i < delimiterLength; i++) {
124+
if (byteBuffer.get() != KEY_DELIMITER_BYTES[i]) {
125+
throw new CodecException("Expected delimiter after keyPrefix at position " +
126+
(startPosition + keyPrefixLength));
127+
}
128+
}
129+
long version = byteBuffer.getLong();
130+
for (int i = 0; i < delimiterLength; i++) {
131+
if (byteBuffer.get() != KEY_DELIMITER_BYTES[i]) {
132+
throw new CodecException("Expected delimiter after version at position " +
133+
(startPosition + keyPrefixLength + delimiterLength + Long.BYTES));
134+
}
135+
}
136+
long containerId = byteBuffer.getLong();
137+
138+
return KeyPrefixContainer.get(keyPrefix, version, containerId);
128139
}
129140

130-
/**
131-
* Decode string from ByteBuffer without copying to intermediate byte array.
132-
* Uses CharsetDecoder for efficient decoding.
133-
*/
134-
private String decodeStringFromBuffer(ByteBuffer buffer, int length) throws CodecException {
135-
if (length == 0) {
141+
private static String decodeStringFromBuffer(ByteBuffer buffer) {
142+
if (buffer.remaining() == 0) {
136143
return "";
137144
}
138145

139-
try {
140-
ByteBuffer slice = buffer.duplicate();
141-
slice.limit(slice.position() + length);
142-
146+
final byte[] bytes;
147+
if (buffer.hasArray()) {
148+
int offset = buffer.arrayOffset() + buffer.position();
149+
int length = buffer.remaining();
150+
bytes = new byte[length];
151+
System.arraycopy(buffer.array(), offset, bytes, 0, length);
143152
buffer.position(buffer.position() + length);
144-
145-
CharsetDecoder decoder = UTF_8.newDecoder();
146-
CharBuffer charBuffer = decoder.decode(slice);
147-
return charBuffer.toString();
148-
149-
} catch (CharacterCodingException e) {
150-
throw new CodecException("Failed to decode UTF-8 string from buffer", e);
153+
} else {
154+
bytes = new byte[buffer.remaining()];
155+
buffer.get(bytes);
151156
}
157+
return new String(bytes, UTF_8);
152158
}
153159

154160
@Override
@@ -157,7 +163,7 @@ public byte[] toPersistedFormat(KeyPrefixContainer keyPrefixContainer) {
157163
"Null object can't be converted to byte array.");
158164
byte[] keyPrefixBytes = keyPrefixContainer.getKeyPrefix().getBytes(UTF_8);
159165

160-
//Prefix seek can be done only with keyPrefix. In that case, we can
166+
// Prefix seek can be done only with keyPrefix. In that case, we can
161167
// expect the version and the containerId to be undefined.
162168
if (keyPrefixContainer.getKeyVersion() != -1) {
163169
keyPrefixBytes = ArrayUtils.addAll(keyPrefixBytes, KEY_DELIMITER_BYTES);
@@ -176,32 +182,23 @@ public byte[] toPersistedFormat(KeyPrefixContainer keyPrefixContainer) {
176182

177183
@Override
178184
public KeyPrefixContainer fromPersistedFormat(byte[] rawData) {
179-
int totalLength = rawData.length;
180-
int delimiterLength = KEY_DELIMITER_BYTES.length;
181-
182-
// Only support decoding when we have both version and containerId
183-
if (totalLength >= 2 * (delimiterLength + Long.BYTES)) {
184-
// Extract keyPrefix (everything except last 2 delimiters + 2 longs)
185-
int keyPrefixLength = totalLength - 2 * delimiterLength - 2 * Long.BYTES;
186-
String keyPrefix = new String(ArrayUtils.subarray(rawData, 0, keyPrefixLength), UTF_8);
187-
188-
// Read version
189-
int versionStart = keyPrefixLength + delimiterLength;
190-
byte[] versionBytes = ArrayUtils.subarray(rawData, versionStart, versionStart + Long.BYTES);
191-
long version = ByteBuffer.wrap(versionBytes).getLong();
192-
193-
// Read containerId
194-
int containerIdStart = versionStart + Long.BYTES + delimiterLength;
195-
byte[] containerIdBytes = ArrayUtils.subarray(rawData, containerIdStart, containerIdStart + Long.BYTES);
196-
long containerId = ByteBuffer.wrap(containerIdBytes).getLong();
197-
198-
return KeyPrefixContainer.get(keyPrefix, version, containerId);
199-
}
200-
201-
// For backwards compatibility during transition, treat anything else as keyPrefix only
202-
// This should not be used in production as it cannot handle keys with underscores
203-
String keyPrefix = new String(rawData, UTF_8);
204-
return KeyPrefixContainer.get(keyPrefix, -1, -1);
185+
// When reading from byte[], we can always expect to have the key, version
186+
// and version parts in the byte array.
187+
byte[] keyBytes = ArrayUtils.subarray(rawData,
188+
0, rawData.length - Long.BYTES * 2 - 2);
189+
String keyPrefix = new String(keyBytes, UTF_8);
190+
191+
// Second 8 bytes is the key version.
192+
byte[] versionBytes = ArrayUtils.subarray(rawData,
193+
rawData.length - Long.BYTES * 2 - 1,
194+
rawData.length - Long.BYTES - 1);
195+
long version = ByteBuffer.wrap(versionBytes).getLong();
196+
197+
// Last 8 bytes is the containerId.
198+
long containerIdFromDB = ByteBuffer.wrap(ArrayUtils.subarray(rawData,
199+
rawData.length - Long.BYTES,
200+
rawData.length)).getLong();
201+
return KeyPrefixContainer.get(keyPrefix, version, containerIdFromDB);
205202
}
206203

207204
@Override

‎hadoop-ozone/recon/src/test/java/org/apache/hadoop/ozone/recon/spi/impl/TestKeyPrefixContainerCodec.java‎

Lines changed: 38 additions & 32 deletions
Original file line numberDiff line numberDiff line change
@@ -33,29 +33,12 @@ public class TestKeyPrefixContainerCodec {
3333

3434
private final Codec<KeyPrefixContainer> codec = KeyPrefixContainerCodec.get();
3535

36-
@Test
37-
public void testKeyPrefixVersionAndContainer() throws Exception {
38-
KeyPrefixContainer original = KeyPrefixContainer.get("testKey", 123L, 456L);
39-
testCodecBuffer(original);
40-
}
41-
42-
@Test
43-
public void testEmptyKeyPrefix() throws Exception {
44-
KeyPrefixContainer original = KeyPrefixContainer.get("");
45-
testCodecBuffer(original);
46-
}
47-
4836
@Test
4937
public void testKeyPrefixWithDelimiter() throws Exception {
50-
KeyPrefixContainer original = KeyPrefixContainer.get("test_key_with_underscores", 789L, 101112L);
51-
testCodecBuffer(original);
52-
}
53-
54-
@Test
55-
public void testLongKeyPrefix() throws Exception {
56-
KeyPrefixContainer originalWithBoth = KeyPrefixContainer.get(
57-
"test___________________________________Key", 123L, 456L);
58-
testCodecBuffer(originalWithBoth);
38+
runTest("testKey", 123L, 456L);
39+
runTest("test_key_with_underscores", 789L, 101112L);
40+
runTest("test___________________________________Key", 1L, 2L);
41+
runTest("", 0L, 0L);
5942
}
6043

6144
@Test
@@ -68,19 +51,42 @@ public void testTypeClass() {
6851
assertEquals(KeyPrefixContainer.class, codec.getTypeClass());
6952
}
7053

71-
private void testCodecBuffer(KeyPrefixContainer original) throws Exception {
54+
void runTest(String keyPrefix, long version, long containerId) throws Exception {
55+
final KeyPrefixContainer original = KeyPrefixContainer.get(keyPrefix, version, containerId);
56+
final KeyPrefixContainer keyAndVersion = KeyPrefixContainer.get(keyPrefix, version);
57+
final KeyPrefixContainer keyOnly = KeyPrefixContainer.get(keyPrefix);
58+
59+
final CodecBuffer.Allocator allocator = CodecBuffer.Allocator.getHeap();
60+
try (CodecBuffer originalBuffer = codec.toCodecBuffer(original, allocator);
61+
CodecBuffer keyOnlyBuffer = codec.toCodecBuffer(keyOnly, allocator);
62+
CodecBuffer keyAndVersionBuffer = codec.toCodecBuffer(keyAndVersion, allocator)) {
63+
assertEquals(original, codec.fromCodecBuffer(originalBuffer));
64+
assertTrue(originalBuffer.startsWith(keyAndVersionBuffer));
65+
assertTrue(originalBuffer.startsWith(keyOnlyBuffer));
66+
67+
final byte[] originalBytes = assertCodecBuffer(original, originalBuffer);
68+
assertEquals(original, codec.fromPersistedFormat(originalBytes));
69+
70+
final byte[] keyAndVersionBytes = assertCodecBuffer(keyAndVersion, keyAndVersionBuffer);
71+
assertPrefix(originalBytes.length - KeyPrefixContainerCodec.LONG_SERIALIZED_SIZE,
72+
originalBytes, keyAndVersionBytes);
73+
74+
final byte[] keyOnlyBytes = assertCodecBuffer(keyOnly, keyOnlyBuffer);
75+
assertPrefix(originalBytes.length - 2 * KeyPrefixContainerCodec.LONG_SERIALIZED_SIZE,
76+
originalBytes, keyOnlyBytes);
77+
}
78+
}
7279

73-
final CodecBuffer codecBuffer = codec.toCodecBuffer(
74-
original, CodecBuffer.Allocator.getHeap());
75-
final KeyPrefixContainer fromBuffer = codec.fromCodecBuffer(codecBuffer);
80+
static void assertPrefix(int expectedLength, byte[] array, byte[] prefix) {
81+
assertEquals(expectedLength, prefix.length);
82+
for (int i = 0; i < prefix.length; i++) {
83+
assertEquals(array[i], prefix[i]);
84+
}
85+
}
7686

87+
byte[] assertCodecBuffer(KeyPrefixContainer original, CodecBuffer buffer) throws Exception {
7788
final byte[] bytes = codec.toPersistedFormat(original);
78-
assertArrayEquals(bytes, codecBuffer.getArray());
79-
80-
codecBuffer.release();
81-
assertEquals(original, fromBuffer);
82-
83-
KeyPrefixContainer fromPersisted = codec.fromPersistedFormat(bytes);
84-
assertEquals(original, fromPersisted);
89+
assertArrayEquals(bytes, buffer.getArray());
90+
return bytes;
8591
}
8692
}

0 commit comments

Comments
 (0)