[
https://issues.apache.org/jira/browse/BEAM-6300?focusedWorklogId=178262&page=com.atlassian.jira.plugin.system.issuetabpanels:worklog-tabpanel#worklog-178262
]
ASF GitHub Bot logged work on BEAM-6300:
----------------------------------------
Author: ASF GitHub Bot
Created on: 22/Dec/18 15:58
Start Date: 22/Dec/18 15:58
Worklog Time Spent: 10m
Work Description: reuvenlax closed pull request #7345: [BEAM-6300] Put
generated getter/setter/creator classes in the same package as the class they
access
URL: https://github.com/apache/beam/pull/7345
This is a PR merged from a forked repository.
As GitHub hides the original diff on merge, it is displayed below for
the sake of provenance:
As this is a foreign pull request (from a fork), the diff is supplied
below (as it won't show otherwise due to GitHub magic):
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroByteBuddyUtils.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroByteBuddyUtils.java
index a1adb25ae6e2..22719a1668a5 100644
---
a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroByteBuddyUtils.java
+++
b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/AvroByteBuddyUtils.java
@@ -36,6 +36,7 @@
import org.apache.beam.sdk.schemas.Schema;
import org.apache.beam.sdk.schemas.SchemaUserTypeCreator;
import org.apache.beam.sdk.schemas.utils.ByteBuddyUtils.ConvertType;
+import org.apache.beam.sdk.schemas.utils.ByteBuddyUtils.InjectPackageStrategy;
import org.apache.beam.sdk.schemas.utils.ReflectUtils.ClassWithSchema;
import org.apache.beam.sdk.util.common.ReflectHelpers;
import org.apache.beam.sdk.values.TypeDescriptor;
@@ -76,6 +77,7 @@
try {
DynamicType.Builder<SchemaUserTypeCreator> builder =
BYTE_BUDDY
+ .with(new InjectPackageStrategy(clazz))
.subclass(SchemaUserTypeCreator.class)
.method(ElementMatchers.named("create"))
.intercept(construct);
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java
index df910bac97d4..1be78837ea67 100644
---
a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java
+++
b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ByteBuddyUtils.java
@@ -25,6 +25,8 @@
import java.util.List;
import java.util.Map;
import net.bytebuddy.ByteBuddy;
+import net.bytebuddy.NamingStrategy;
+import net.bytebuddy.NamingStrategy.SuffixingRandom.BaseNameResolver;
import net.bytebuddy.description.type.TypeDescription;
import net.bytebuddy.description.type.TypeDescription.ForLoadedType;
import net.bytebuddy.dynamic.DynamicType;
@@ -38,6 +40,7 @@
import net.bytebuddy.implementation.bytecode.collection.ArrayFactory;
import net.bytebuddy.implementation.bytecode.member.MethodInvocation;
import net.bytebuddy.matcher.ElementMatchers;
+import net.bytebuddy.utility.RandomString;
import org.apache.avro.generic.GenericFixed;
import org.apache.beam.sdk.schemas.FieldValueGetter;
import org.apache.beam.sdk.schemas.FieldValueSetter;
@@ -59,6 +62,40 @@
private static final ForLoadedType READABLE_INSTANT_TYPE =
new ForLoadedType(ReadableInstant.class);
+ /**
+ * A naming strategy for ByteBuddy classes.
+ *
+ * <p>We always inject the generatter classes in the same same package as
the user's target class.
+ *
+ * @kanterov kanterov 20 hours ago Contributor nit: s/generatter/generated/
+ * @reuvenlax Reply… This way, if the class fields or methods are package
private, our generated
+ * class can still access them.
+ */
+ static class InjectPackageStrategy extends NamingStrategy.AbstractBase {
+ /** A resolver for the base name for naming the unnamed type. */
+ private static final BaseNameResolver baseNameResolver =
+ BaseNameResolver.ForUnnamedType.INSTANCE;
+
+ private static final String SUFFIX = "SchemaCodeGen";
+
+ private final RandomString randomString;
+
+ private final String targetPackage;
+
+ InjectPackageStrategy(Class<?> baseType) {
+ randomString = new RandomString();
+ this.targetPackage = baseType.getPackage().getName();
+ }
+
+ @Override
+ protected String name(TypeDescription superClass) {
+ String baseName = baseNameResolver.resolve(superClass);
+ int lastDot = baseName.lastIndexOf('.');
+ String className = baseName.substring(lastDot, baseName.length());
+ return targetPackage + className + "$" + SUFFIX + "$" +
randomString.nextString();
+ }
+ };
+
// Create a new FieldValueGetter subclass.
@SuppressWarnings("unchecked")
static DynamicType.Builder<FieldValueGetter> subclassGetterInterface(
@@ -67,7 +104,8 @@
TypeDescription.Generic.Builder.parameterizedType(
FieldValueGetter.class, objectType, fieldType)
.build();
- return (DynamicType.Builder<FieldValueGetter>)
byteBuddy.subclass(getterGenericType);
+ return (DynamicType.Builder<FieldValueGetter>)
+ byteBuddy.with(new InjectPackageStrategy((Class)
objectType)).subclass(getterGenericType);
}
// Create a new FieldValueSetter subclass.
@@ -78,7 +116,8 @@
TypeDescription.Generic.Builder.parameterizedType(
FieldValueSetter.class, objectType, fieldType)
.build();
- return (DynamicType.Builder<FieldValueSetter>)
byteBuddy.subclass(setterGenericType);
+ return (DynamicType.Builder<FieldValueSetter>)
+ byteBuddy.with(new InjectPackageStrategy((Class)
objectType)).subclass(setterGenericType);
}
// Base class used below to convert types.
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/POJOUtils.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/POJOUtils.java
index 370b3ccc0548..2c22490787e7 100644
---
a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/POJOUtils.java
+++
b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/POJOUtils.java
@@ -55,6 +55,7 @@
import org.apache.beam.sdk.schemas.SchemaUserTypeCreator;
import org.apache.beam.sdk.schemas.utils.ByteBuddyUtils.ConvertType;
import org.apache.beam.sdk.schemas.utils.ByteBuddyUtils.ConvertValueForGetter;
+import org.apache.beam.sdk.schemas.utils.ByteBuddyUtils.InjectPackageStrategy;
import org.apache.beam.sdk.schemas.utils.ReflectUtils.ClassWithSchema;
import org.apache.beam.sdk.util.common.ReflectHelpers;
import org.apache.beam.sdk.values.TypeDescriptor;
@@ -129,6 +130,7 @@ public static Schema schemaFromPojoClass(Class<?> clazz) {
try {
DynamicType.Builder<SchemaUserTypeCreator> builder =
BYTE_BUDDY
+ .with(new InjectPackageStrategy(clazz))
.subclass(SchemaUserTypeCreator.class)
.method(ElementMatchers.named("create"))
.intercept(new CreateInstruction(fields, clazz));
diff --git
a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ReflectUtils.java
b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ReflectUtils.java
index c9d943b6ee6b..26b3d7073d1c 100644
---
a/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ReflectUtils.java
+++
b/sdks/java/core/src/main/java/org/apache/beam/sdk/schemas/utils/ReflectUtils.java
@@ -30,7 +30,6 @@
import java.util.Map;
import java.util.Objects;
import java.util.stream.Collectors;
-import javax.annotation.Nullable;
import org.apache.beam.sdk.schemas.Schema;
/** A set of reflection helper methods. */
@@ -71,7 +70,8 @@ public int hashCode() {
clazz,
c -> {
return Arrays.stream(c.getDeclaredMethods())
- .filter(m -> Modifier.isPublic(m.getModifiers()))
+ .filter(m -> !Modifier.isPrivate(m.getModifiers()))
+ .filter(m -> !Modifier.isProtected(m.getModifiers()))
.filter(m -> !Modifier.isStatic(m.getModifiers()))
.collect(Collectors.toList());
});
@@ -89,8 +89,7 @@ public int hashCode() {
}
for (java.lang.reflect.Field field : c.getDeclaredFields()) {
if ((field.getModifiers() & (Modifier.TRANSIENT |
Modifier.STATIC)) == 0) {
- if ((field.getModifiers() & Modifier.PUBLIC) != 0) {
- boolean nullable = field.getAnnotation(Nullable.class) !=
null;
+ if ((field.getModifiers() & (Modifier.PRIVATE |
Modifier.PROTECTED)) == 0) {
checkArgument(
types.put(field.getName(), field) == null,
c.getSimpleName() + " contains two fields named: " +
field);
----------------------------------------------------------------
This is an automated message from the Apache Git Service.
To respond to the message, please log on GitHub and use the
URL above to go to the specific comment.
For queries about this service, please contact Infrastructure at:
[email protected]
Issue Time Tracking
-------------------
Worklog Id: (was: 178262)
Time Spent: 0.5h (was: 20m)
> Generated row object for POJOs, Avros, and JavaBeans should work if the
> wrapped class is package private
> --------------------------------------------------------------------------------------------------------
>
> Key: BEAM-6300
> URL: https://issues.apache.org/jira/browse/BEAM-6300
> Project: Beam
> Issue Type: Sub-task
> Components: sdk-java-core
> Reporter: Reuven Lax
> Assignee: Reuven Lax
> Priority: Major
> Time Spent: 0.5h
> Remaining Estimate: 0h
>
> We should put the byte-buddy generated classes in the same package as the
> class they are accessing.
--
This message was sent by Atlassian JIRA
(v7.6.3#76005)