aliehsaeedii commented on code in PR #23233:
URL: https://github.com/apache/kafka/pull/23233#discussion_r3852901821
##########
streams/src/test/java/org/apache/kafka/streams/processor/internals/StateManagerUtilConverterTest.java:
##########
@@ -55,31 +76,135 @@
@MockitoSettings(strictness = Strictness.STRICT_STUBS)
public class StateManagerUtilConverterTest {
+ private static final long TIMESTAMP = 42L;
+ private static final long WINDOW_START = 0L;
+
@Test
public void
shouldReturnIdentityConverterForPlainToTimestampedPersistentKeyValueStore() {
Review Comment:
The 6 `shouldReturn*Converter*` tests and the 6 `shouldRestore*` tests each
differ only in the `Stores.*` factory and the expected converter/timestamp.
Consider folding each group into a `@ParameterizedTest` with `@MethodSource` —
the `timestampedKeyValueStore`/`timestampedWindowStore` helpers already make
that easy, and it cuts 12 near-identical methods to ~4.
##########
streams/src/main/java/org/apache/kafka/streams/processor/internals/StateManagerUtil.java:
##########
@@ -92,6 +94,18 @@ static RecordConverter converterForStore(final StateStore
store) {
current = ((WrappedStateStore<?, ?, ?>) current).wrapped();
}
+ // No adapter found: the inner store's binary format is whatever the
store chain advertises
+ if (isHeadersAware(store)) {
+ if (store instanceof SessionStore) {
+ return rawValueToSessionHeadersValue();
+ }
+ return rawValueToHeadersValue();
+ } else if (isTimestamped(store) && !isVersioned(store)) {
Review Comment:
This branch (`isTimestamped && !isVersioned`) has no test — none of the
`converterForStore` tests build a versioned store. Since this PR relocates this
branch, consider adding a versioned-store case so the `!isVersioned` guard is
locked.
--
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]
