Skip to content

Commit bd5a070

Browse files
committed
Address review comments: Refactor ByteBuddyDoFnInvokerFactory methods
- Replace String.format in toString() with MoreObjects.toStringHelper for consistency. - Update hashCode() and equals() to use java.util.Objects utility methods. - Extract type suffix generation logic into a reusable static method to avoid duplication in tests.
1 parent f0271f0 commit bd5a070

2 files changed

Lines changed: 37 additions & 23 deletions

File tree

sdks/java/core/src/main/java/org/apache/beam/sdk/transforms/reflect/ByteBuddyDoFnInvokerFactory.java

Lines changed: 36 additions & 18 deletions
Original file line numberDiff line numberDiff line change
@@ -27,6 +27,7 @@
2727
import java.util.ArrayList;
2828
import java.util.List;
2929
import java.util.Map;
30+
import java.util.Objects;
3031
import java.util.concurrent.ConcurrentHashMap;
3132
import net.bytebuddy.ByteBuddy;
3233
import net.bytebuddy.description.field.FieldDescription;
@@ -106,6 +107,7 @@
106107
import org.apache.beam.sdk.util.UserCodeException;
107108
import org.apache.beam.sdk.values.TypeDescriptor;
108109
import org.apache.beam.sdk.values.TypeDescriptors;
110+
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.base.MoreObjects;
109111
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.collect.Maps;
110112
import org.apache.beam.vendor.guava.v32_1_2_jre.com.google.common.primitives.Primitives;
111113
import org.checkerframework.checker.nullness.qual.MonotonicNonNull;
@@ -192,24 +194,23 @@ public boolean equals(@Nullable Object o) {
192194
return false;
193195
}
194196
InvokerCacheKey that = (InvokerCacheKey) o;
195-
return fnClass.equals(that.fnClass)
196-
&& inputType.equals(that.inputType)
197-
&& outputType.equals(that.outputType);
197+
return Objects.equals(fnClass, that.fnClass)
198+
&& Objects.equals(inputType, that.inputType)
199+
&& Objects.equals(outputType, that.outputType);
198200
}
199201

200202
@Override
201203
public int hashCode() {
202-
int result = fnClass.hashCode();
203-
result = 31 * result + inputType.hashCode();
204-
result = 31 * result + outputType.hashCode();
205-
return result;
204+
return Objects.hash(fnClass, inputType, outputType);
206205
}
207206

208207
@Override
209208
public String toString() {
210-
return String.format(
211-
"InvokerCacheKey{fnClass=%s, inputType=%s, outputType=%s}",
212-
fnClass.getName(), inputType, outputType);
209+
return MoreObjects.toStringHelper(this)
210+
.add("fnClass", fnClass.getName())
211+
.add("inputType", inputType)
212+
.add("outputType", outputType)
213+
.toString();
213214
}
214215
}
215216

@@ -317,9 +318,10 @@ public <InputT, OutputT> DoFnInvoker<InputT, OutputT> newByteBuddyInvoker(
317318
signature.fnClass(),
318319
fn.getClass());
319320

320-
// Extract input and output type descriptors from the DoFn instance
321-
// Fall back to Object.class if the type descriptors are null or unavailable (e.g., MapElements
322-
// after serialization)
321+
// Extract input and output type descriptors to distinguish generic instantiations.
322+
// Fall back to Object.class if unavailable. When type info is lost, different generic
323+
// instantiations share an invoker, which is acceptable since the DoFn class in the cache
324+
// key prevents collisions between different DoFn classes.
323325
TypeDescriptor<InputT> inputType;
324326
try {
325327
inputType = fn.getInputTypeDescriptor();
@@ -540,18 +542,34 @@ public static double validateSize(double size) {
540542
}
541543
}
542544

545+
/**
546+
* Generates a type suffix string for use in invoker class names.
547+
*
548+
* <p>This creates a unique suffix based on the input and output type descriptors to avoid class
549+
* name collisions when the same DoFn class is used with different generic types.
550+
*
551+
* <p>The format is: {@code DoFnInvoker$<8-digit hex hash>}
552+
*
553+
* @param inputType the input type descriptor
554+
* @param outputType the output type descriptor
555+
* @return a string suffix for the invoker class name
556+
*/
557+
public static String generateTypeSuffix(
558+
TypeDescriptor<?> inputType, TypeDescriptor<?> outputType) {
559+
return String.format(
560+
"%s$%08x",
561+
DoFnInvoker.class.getSimpleName(),
562+
(inputType.toString() + "|" + outputType.toString()).hashCode());
563+
}
564+
543565
/** Generates a {@link DoFnInvoker} class for the given {@link DoFnSignature}. */
544566
private static Class<? extends DoFnInvoker<?, ?>> generateInvokerClass(
545567
DoFnSignature signature, TypeDescriptor<?> inputType, TypeDescriptor<?> outputType) {
546568
Class<? extends DoFn<?, ?>> fnClass = signature.fnClass();
547569

548570
// Create a unique suffix based on the type descriptors to avoid class name collisions
549571
// when the same DoFn class is used with different generic types.
550-
String typeSuffix =
551-
String.format(
552-
"%s$%08x",
553-
DoFnInvoker.class.getSimpleName(),
554-
(inputType.toString() + "|" + outputType.toString()).hashCode());
572+
String typeSuffix = generateTypeSuffix(inputType, outputType);
555573

556574
final TypeDescription clazzDescription = new TypeDescription.ForLoadedType(fnClass);
557575

sdks/java/core/src/test/java/org/apache/beam/sdk/transforms/reflect/DoFnInvokersTest.java

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1389,11 +1389,7 @@ public void testStableName() {
13891389
// different generic instantiations of the same DoFn class.
13901390
// Format: <DoFn class name>$<DoFnInvoker>$<type hash>
13911391
TypeDescriptor<Void> voidType = new StableNameTestDoFn().getInputTypeDescriptor();
1392-
String expectedTypeSuffix =
1393-
String.format(
1394-
"%s$%08x",
1395-
DoFnInvoker.class.getSimpleName(),
1396-
(voidType.toString() + "|" + voidType.toString()).hashCode());
1392+
String expectedTypeSuffix = ByteBuddyDoFnInvokerFactory.generateTypeSuffix(voidType, voidType);
13971393
assertThat(
13981394
invoker.getClass().getName(),
13991395
equalTo(String.format("%s$%s", StableNameTestDoFn.class.getName(), expectedTypeSuffix)));

0 commit comments

Comments
 (0)