paul-bjorkstrand commented on a change in pull request #17: SLING-8706 -
Injections for java.util.Optional<> should be automatically optional
URL:
https://github.com/apache/sling-org-apache-sling-models-impl/pull/17#discussion_r347150037
##########
File path:
src/main/java/org/apache/sling/models/impl/injectors/ValueMapInjector.java
##########
@@ -98,6 +99,11 @@ public Object getValue(@NotNull Object adaptable, String
name, @NotNull Type typ
return null;
}
Class<?> collectionType = (Class<?>) pType.getRawType();
+
+ if (collectionType.equals(Optional.class)) {
+ return Optional.ofNullable(map.get(name));
Review comment:
In addition to [my other concern], this call to `map.get(..)` would need to
use the actual type extracted from the `ParameterizedType` of the field,
similar to how it is done on [line 70 in this PR]. You can see the extraction
of the actual type argument in lines [95][line 95 in this PR]-101 in this file
in this PR, or starting on [line 1004][line 1004 of ModelAdapterFactory].
This implementation would likely fail for situations where the type in the
JCR does not match the expected injection site type. Example: if you have a
`String` field in the model, but the value stored in the JCR is `String[]` or
`Long`. Using just `map.get(name)` returns the actual type stored, upcasted to
`Object`. If that is a `String[]`, you cannot (in java) cast to `String`.
[my other concern]:
https://github.com/apache/sling-org-apache-sling-models-impl/pull/17#issuecomment-554768731
[line 70 in this PR]:
https://github.com/apache/sling-org-apache-sling-models-impl/pull/17/files#diff-c249fe75556c30b251535a7eb4309b7dR70
[line 95 in this PR]:
https://github.com/apache/sling-org-apache-sling-models-impl/pull/17/files#diff-c249fe75556c30b251535a7eb4309b7dR95
[line 1004 of ModelAdapterFactory]:
https://github.com/apache/sling-org-apache-sling-models-impl/blob/master/src/main/java/org/apache/sling/models/impl/ModelAdapterFactory.java#L1004
----------------------------------------------------------------
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.
For queries about this service, please contact Infrastructure at:
[email protected]
With regards,
Apache Git Services
