You are viewing a plain text version of this content. The canonical link for it is here.
Posted to github@beam.apache.org by GitBox <gi...@apache.org> on 2022/06/04 19:08:27 UTC
[GitHub] [beam] damccorm opened a new issue, #20752: AutoValueSchema does not support arbitrary named getters.
damccorm opened a new issue, #20752:
URL: https://github.com/apache/beam/issues/20752
AutoValueSchema fails to detect getters with non-pojo names.
Example:
```
@DefaultSchema(AutoValueSchema.class)
@AutoValue
class Example
{
abstract int id();
static Example create(int id) \\{ return new AutoValue_Example(id); }
}
```
Imported from Jira [BEAM-11609](https://issues.apache.org/jira/browse/BEAM-11609). Original Jira may contain additional context.
Reported by: dpcollins-google.
--
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: github-unsubscribe@beam.apache.org.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] SteveNewson commented on issue #20752: AutoValueSchema does not support arbitrary named getters.
Posted by GitBox <gi...@apache.org>.
SteveNewson commented on issue #20752:
URL: https://github.com/apache/beam/issues/20752#issuecomment-1294855339
And we'll need to do an assortment of other changes to make it find the methods properly.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] SteveNewson commented on issue #20752: AutoValueSchema does not support arbitrary named getters.
Posted by GitBox <gi...@apache.org>.
SteveNewson commented on issue #20752:
URL: https://github.com/apache/beam/issues/20752#issuecomment-1294853929
```java
List<Method> methods =
ReflectUtils.getMethods(targetClass).stream()
// All AutoValue getters are marked abstract.
.filter(m -> Modifier.isAbstract(m.getModifiers()))
.filter(m -> !Modifier.isPrivate(m.getModifiers()))
.filter(m -> !Modifier.isProtected(m.getModifiers()))
.filter(m -> !m.isAnnotationPresent(SchemaIgnore.class))
.filter(m -> !m.getName().equals("toString") && !m.getName().equals("equals") && !m.getName().equals("hashCode"))
.collect(Collectors.toList());
```
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
[GitHub] [beam] SteveNewson commented on issue #20752: AutoValueSchema does not support arbitrary named getters.
Posted by GitBox <gi...@apache.org>.
SteveNewson commented on issue #20752:
URL: https://github.com/apache/beam/issues/20752#issuecomment-1294845137
The "problem" is here:
```java
@Override
public List<FieldValueTypeInformation> get(Class<?> clazz) {
// If the generated class is passed in, we want to look at the base class to find the getters.
Class<?> targetClass = AutoValueUtils.getBaseAutoValueClass(clazz);
List<Method> methods =
ReflectUtils.getMethods(targetClass).stream()
.filter(ReflectUtils::isGetter)
// All AutoValue getters are marked abstract.
.filter(m -> Modifier.isAbstract(m.getModifiers()))
.filter(m -> !Modifier.isPrivate(m.getModifiers()))
.filter(m -> !Modifier.isProtected(m.getModifiers()))
.filter(m -> !m.isAnnotationPresent(SchemaIgnore.class))
.collect(Collectors.toList());
List<FieldValueTypeInformation> types = Lists.newArrayListWithCapacity(methods.size());
for (int i = 0; i < methods.size(); ++i) {
types.add(FieldValueTypeInformation.forGetter(methods.get(i), i));
}
types.sort(Comparator.comparing(FieldValueTypeInformation::getNumber));
validateFieldNumbers(types);
return types;
}
```
The first filter removes anything that does not start with "get" or "is":
```java
public static boolean isGetter(Method method) {
if (Void.TYPE.equals(method.getReturnType())) {
return false;
}
if (method.getName().startsWith("get") && method.getName().length() > 3) {
return true;
}
return (method.getName().startsWith("is")
&& method.getName().length() > 2
&& method.getParameterCount() == 0
&& (Boolean.TYPE.equals(method.getReturnType())
|| Boolean.class.equals(method.getReturnType())));
}
```
Clearly a trivial change to correct this. However, that might be controversial as changing this behaviour may pick up methods that previously would not have been detected. If there's another Abstract method on the class that is not a getter then this would cause a problem. However, I can't see how that could occur without also breaking AutoValue itself.
```java
@AutoValue
public abstract Bob {
abstract int id();
abstract int notARealGetter();
static Bob create(int id) {};
}
```
AutoValue would interpret the `notARealGetter` method as a getter, so this is fine.
The problem would be `toString` (or any `Object` method that AutoValue can redefine - currently only `toString`, `hashCode` and `equals`):
```java
@AutoValue
class PleaseOverrideExample extends SuperclassThatDefinesToString {
...
// cause AutoValue to generate this even though the superclass has it
@Override public abstract String toString();
}
```
Removing the `isGetter` filter would pick this method up as a property, which it clearly is not. It is likely that people have done this in existing code, so changing this behaviour would potentially break their code (without a compilation or runtime warning of the fact).
So, I propose we remove the filter `isGetter` and also explicitly exclude the `toString`, `hashCode` and `equals` methods.
--
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: github-unsubscribe@beam.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org