stefanseifert commented on a change in pull request #11:
URL:
https://github.com/apache/sling-org-apache-sling-models-impl/pull/11#discussion_r762409049
##########
File path:
src/main/java/org/apache/sling/models/impl/model/ModelClassConstructor.java
##########
@@ -49,16 +50,49 @@ public ModelClassConstructor(Constructor<ModelType>
constructor, StaticInjectAnn
}
}
- public Constructor<ModelType> getConstructor() {
+ /**
+ * Proxies the call to {@link Constructor#newInstance(Object...)},
checking (and
+ * setting) accessibility first.
+ *
+ * @param parameters
+ * the constructor parameters that are passed to
+ * {@link Constructor#newInstance(Object...)}
+ * @return The constructed object
+ *
+ * @throws InstantiationException when {@link
Constructor#newInstance(Object...)} would throw
+ * @throws IllegalAccessException when {@link
Constructor#newInstance(Object...)} would throw
+ * @throws IllegalArgumentException when {@link
Constructor#newInstance(Object...)} would throw
+ * @throws InvocationTargetException when {@link
Constructor#newInstance(Object...)} would throw
+ *
+ * @see Constructor#newInstance(Object...)
+ */
+ @SuppressWarnings({"java:S3011","java:S1874"})
+ public M newInstance(Object... parameters) throws InstantiationException,
IllegalAccessException, IllegalArgumentException, InvocationTargetException {
+ synchronized (constructor) {
Review comment:
i was initially a bit hesitant to introduce new synchronized blocks like
this - even if the constructor is accessible. but as stated in the ticket
itself, the same pattern is already in place for InjectableField for a long
time, and that is probably even run through multiple times for each model
instance, so adding another check like this should not do additional harm.
--
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]
