[GitHub] [jclouds] gaul commented on pull request #78: Replace embedded and repackaged GSON library
gaul commented on pull request #78: URL: https://github.com/apache/jclouds/pull/78#issuecomment-650440462 > In any case, this deserves a proper discussion in the jclouds dev/users mailing lists so the community is aware of the impact of the change. It would be great too, to trigger such a discussion cross-posting to jclouds-dev and karaf-dev. The Karaf community kindly took over the jclouds-karaf project and is now maintaining the Karaf and some OSGi bits. I think it makes a lot of sense to involve them in this topic and see also what is in their opinion the best way to keep OSGi support in jclouds from now on. @gurkerl83 I support this PR but we should give the community a chance to respond to a breaking change. I don't have the background to explain the tradeoffs with the current OSGi support; would you mind leading this? 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: us...@infra.apache.org
[GitHub] [jclouds] gurkerl83 edited a comment on pull request #78: Replace embedded and repackaged GSON library
gurkerl83 edited a comment on pull request #78: URL: https://github.com/apache/jclouds/pull/78#issuecomment-650395849 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: us...@infra.apache.org
[GitHub] [jclouds] gurkerl83 edited a comment on pull request #78: Replace embedded and repackaged GSON library
gurkerl83 edited a comment on pull request #78: URL: https://github.com/apache/jclouds/pull/78#issuecomment-650395849 The integration of OSGi into JClouds is really only superficial. Not OSGi near component models like Declarative Services or Blueprint were chosen as CDI but simply Guice. Don't be frightened if OSGi Metadata declarations are present in all bundle POMs. These aspects are still relevant. Here we should switch to the use of the BND Maven plugin, but it's best to outsource the OSGi rules contained in the pom to bnd files. Especially good is already the use of the Service Provider Interface (SPI) as a plugin framework, which is not OSGi specific and has been supported by Java for a long time. The OSGi package contained in the core module and its classes, especially MetadataBundleListener through the implementation of the interface BundleListener react when a bundle with an entry in /META-INF/services/ is loaded into the OSGi runtime environment. In short, you can move the bundle to a new module and the function will be preserved without problems. At this point you have to integrate e.g. Aries SPI-fly bundle into the OSGi runtime environment so that the bundle listener reacts to this kind of events. Then there is only the part included in the script-builder module, which I will have a look at tomorrow. OSGi is really not integrated in more modules as core and script-builder. Over the weekend I will describe the procedure, which components are affected and how a solution could look like as a JIRA ticket. I'm also willing to help out with the JClouds aspects handed over to Karaf. I have opened an issue in this project to learn more about the future development of the project. 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: us...@infra.apache.org
[GitHub] [jclouds] gurkerl83 edited a comment on pull request #78: Replace embedded and repackaged GSON library
gurkerl83 edited a comment on pull request #78: URL: https://github.com/apache/jclouds/pull/78#issuecomment-650395849 The integration of OSGi into JClouds is really only superficial. Not OSGi near component models like Declarative Services or Blueprint were chosen as CDI but simply Guice. Don't be frightened if OSGi Metadata declarations are present in all bundle POMs. These aspects are still relevant. Here we should switch to the use of the BND Maven plugin, but it's best to outsource the OSGi rules contained in the pom to bnd files. Especially good is already the use of the Service Provider Interface (SPI) as a plugin framework, which is not OSGi specific and has been supported by Java for a long time. The OSGi package contained in the core module and its classes, especially MetadataBundleListener through the implementation of the interface BundleListener react when a bundle with an entry in /META-INF/services/ is loaded into the OSGi runtime environment. In short, you can move the bundle to a new module and the function will be preserved without problems. At this point you have to integrate e.g. Aries SPI-fly bundle into the OSGi runtime environment so that the bundle listener reacts to this kind of events. Then there is only the part included in ScriptBuilder module, which I will have a look at tomorrow. Over the weekend I will describe the procedure, which components are affected and how a solution could look like as a JIRA ticket. I'm also willing to help out with the JClouds aspects handed over to Karaf. I have opened an issue in this project to learn more about the future development of the project. 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: us...@infra.apache.org
[GitHub] [jclouds] gurkerl83 commented on pull request #78: Replace embedded and repackaged GSON library
gurkerl83 commented on pull request #78: URL: https://github.com/apache/jclouds/pull/78#issuecomment-650395849 The integration of OSGi into JClouds is really only superficial. Not OSGi near component models like Declarative Services or Blueprint were chosen as CDI but simply Guice. Especially good is already the use of the Service Provider Interface (SPI) as a plugin framework, which is not OSGi specific and has been supported by Java for a long time. The OSGi package contained in the core module and its classes, especially MetadataBundleListener through the implementation of the interface BundleListener react when a bundle with an entry in /META-INF/services/ is loaded into the OSGi runtime environment. In short, you can move the bundle to a new module and the function will be preserved without problems. At this point you have to integrate e.g. Aries SPI-fly bundle into the OSGi runtime environment so that the bundle listener reacts to this kind of events. Then there is only the part included in ScriptBuilder module, which I will have a look at tomorrow. Over the weekend I will describe the procedure, which components are affected and how a solution could look like as a JIRA ticket. I'm also willing to help out with the JClouds aspects handed over to Karaf. I have opened an issue in this project to learn more about the future development of the project. 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: us...@infra.apache.org
[GitHub] [jclouds] JnRouvignac opened a new pull request #79: JCLOUDS-1542 follow-up: check whether canAccess() the members
JnRouvignac opened a new pull request #79: URL: https://github.com/apache/jclouds/pull/79 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: us...@infra.apache.org
[GitHub] [jclouds] JnRouvignac edited a comment on pull request #76: JCLOUDS-1542 Java 11 warns of illegal reflective access
JnRouvignac edited a comment on pull request #76: URL: https://github.com/apache/jclouds/pull/76#issuecomment-650299955 No problem. Thank you for taking the time to review it! I'll follow up with the another PR with the `!canAccess()` when this one is merged. 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: us...@infra.apache.org
[GitHub] [jclouds] nacx commented on pull request #78: Replace embedded and repackaged GSON library
nacx commented on pull request #78: URL: https://github.com/apache/jclouds/pull/78#issuecomment-650318652 So, with the last update, this PR reverts back to the official gson library, which makes jclouds incompatible with OSGi. As mentioned in the different JIRA issues, we need to make a decision and find the balance between the cost of supporting OSGi in jclouds itself and being able to move forward faster. In my opinion, if we lack the OSGi expertise to do a proper job there, we should try to stop owning that and delegate that to end-users or to another community that can do it properly. In any case, this deserves a proper discussion in the jclouds dev/users mailing lists so the community is aware of the impact of the change. It would be great too, to trigger such a discussion cross-posting to jclouds-dev and karaf-dev. The Karaf community kindly took over the jclouds-karaf project and is now maintaining the Karaf and some OSGi bits. I think it makes a lot of sense to involve them in this topic and see also what is in their opinion the best way to keep OSGi support in jclouds from now on. I think reverting back to official gson should be put on hold until we have clarity on this, and we have a proper deprecation path and plan to make the community aware of the change. 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: us...@infra.apache.org
[jira] [Updated] (JCLOUDS-1542) Java 11 warns of illegal reflective access
[
https://issues.apache.org/jira/browse/JCLOUDS-1542?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ignasi Barrera updated JCLOUDS-1542:
Fix Version/s: 2.3.0
> Java 11 warns of illegal reflective access
> --
>
> Key: JCLOUDS-1542
> URL: https://issues.apache.org/jira/browse/JCLOUDS-1542
> Project: jclouds
> Issue Type: Improvement
> Components: jclouds-core
>Affects Versions: 2.2.0
>Reporter: ctranxuan
>Priority: Major
> Fix For: 2.3.0
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> When using JClouds BlobStore for Google Cloud Storage with Java 11, some
> warning are displayed by the JVM about illegal reflective access:
>
> {quote}WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by org.jclouds.reflect.Reflection2$1
> ([file:/.../jclouds-core.jar|file:///.../jclouds-core.jar]) to constructor
> java.lang.String(char[],int,int,java.lang.Void)
> WARNING: Please consider reporting this to the maintainers of
> org.jclouds.reflect.Reflection2$1
> WARNING: Use --illegal-access=warn to enable warnings of further illegal
> reflective access operations
> WARNING: All illegal access operations will be denied in a future release
> {quote}
>
> JClouds works but as stated, these illegal reflective accesses may be denied
> in upper Java versions.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Resolved] (JCLOUDS-1542) Java 11 warns of illegal reflective access
[
https://issues.apache.org/jira/browse/JCLOUDS-1542?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ignasi Barrera resolved JCLOUDS-1542.
-
Resolution: Fixed
> Java 11 warns of illegal reflective access
> --
>
> Key: JCLOUDS-1542
> URL: https://issues.apache.org/jira/browse/JCLOUDS-1542
> Project: jclouds
> Issue Type: Improvement
> Components: jclouds-core
>Affects Versions: 2.2.0
>Reporter: ctranxuan
>Priority: Major
> Fix For: 2.3.0
>
> Time Spent: 2h
> Remaining Estimate: 0h
>
> When using JClouds BlobStore for Google Cloud Storage with Java 11, some
> warning are displayed by the JVM about illegal reflective access:
>
> {quote}WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by org.jclouds.reflect.Reflection2$1
> ([file:/.../jclouds-core.jar|file:///.../jclouds-core.jar]) to constructor
> java.lang.String(char[],int,int,java.lang.Void)
> WARNING: Please consider reporting this to the maintainers of
> org.jclouds.reflect.Reflection2$1
> WARNING: Use --illegal-access=warn to enable warnings of further illegal
> reflective access operations
> WARNING: All illegal access operations will be denied in a future release
> {quote}
>
> JClouds works but as stated, these illegal reflective accesses may be denied
> in upper Java versions.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[jira] [Updated] (JCLOUDS-1542) Java 11 warns of illegal reflective access
[
https://issues.apache.org/jira/browse/JCLOUDS-1542?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel
]
Ignasi Barrera updated JCLOUDS-1542:
Component/s: (was: jclouds-labs-google)
(was: jclouds-blobstore)
jclouds-core
> Java 11 warns of illegal reflective access
> --
>
> Key: JCLOUDS-1542
> URL: https://issues.apache.org/jira/browse/JCLOUDS-1542
> Project: jclouds
> Issue Type: Improvement
> Components: jclouds-core
>Affects Versions: 2.2.0
>Reporter: ctranxuan
>Priority: Major
> Time Spent: 2h
> Remaining Estimate: 0h
>
> When using JClouds BlobStore for Google Cloud Storage with Java 11, some
> warning are displayed by the JVM about illegal reflective access:
>
> {quote}WARNING: An illegal reflective access operation has occurred
> WARNING: Illegal reflective access by org.jclouds.reflect.Reflection2$1
> ([file:/.../jclouds-core.jar|file:///.../jclouds-core.jar]) to constructor
> java.lang.String(char[],int,int,java.lang.Void)
> WARNING: Please consider reporting this to the maintainers of
> org.jclouds.reflect.Reflection2$1
> WARNING: Use --illegal-access=warn to enable warnings of further illegal
> reflective access operations
> WARNING: All illegal access operations will be denied in a future release
> {quote}
>
> JClouds works but as stated, these illegal reflective accesses may be denied
> in upper Java versions.
--
This message was sent by Atlassian Jira
(v8.3.4#803005)
[GitHub] [jclouds] nacx merged pull request #76: JCLOUDS-1542 Java 11 warns of illegal reflective access
nacx merged pull request #76: URL: https://github.com/apache/jclouds/pull/76 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: us...@infra.apache.org
[jira] [Commented] (JCLOUDS-1542) Java 11 warns of illegal reflective access
[ https://issues.apache.org/jira/browse/JCLOUDS-1542?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel=17146545#comment-17146545 ] ASF subversion and git services commented on JCLOUDS-1542: -- Commit 238c97507816606a0aeca2fafd7a3196bfb7f1dc in jclouds's branch refs/heads/master from Jean-Noël Rouvignac [ https://gitbox.apache.org/repos/asf?p=jclouds.git;h=238c975 ] JCLOUDS-1542 Java 11 warns of illegal reflective access (#76) With Java 11, an illegal reflective access is output for the google cloud storage blobstore. WARNING: An illegal reflective access operation has occurred WARNING: Illegal reflective access by org.jclouds.reflect.Reflection2$1 (file:/.../jclouds-core.jar) to constructor java.lang.String(char[],int,int,java.lang.Void) WARNING: Please consider reporting this to the maintainers of org.jclouds.reflect.Reflection2$1 WARNING: Use --illegal-access=warn to enable warnings of further illegal reflective access operations WARNING: All illegal access operations will be denied in a future release Indeed, JClouds calls `setAccessible(true)` on the package protected constructor `java.lang.String(char[],int,int,java.lang.Void)`. Investigating the code, it turns out it is looking for constructors annotated with any of: * java.beans.ConstructorProperties * org.jclouds.json.SerializedNames * com.google.inject.Inject But `String` being defined in `java.base` module, it is impossible that it will be annotated with any of these annotation. This commit is complementary to JClouds commit db4e4af931ef19f582b85f02efb93e50f1c5d01c . Reflection2.java: Do not call `setAccessible(true)` on core java constructors and methods. For reference, here is the stacktrace of this illegal access warning: java.lang.String.(String.java:3208) java.lang.String.(String.java:251) java.util.StringJoiner.compactElts(StringJoiner.java:250) java.util.StringJoiner.toString(StringJoiner.java:173) jdk.internal.module.IllegalAccessLogger.loudWarning(IllegalAccessLogger.java:339) jdk.internal.module.IllegalAccessLogger.log(IllegalAccessLogger.java:288) jdk.internal.module.IllegalAccessLogger.log(IllegalAccessLogger.java:261) jdk.internal.module.IllegalAccessLogger.logIfOpenedForIllegalAccess(IllegalAccessLogger.java:226) java.lang.reflect.AccessibleObject.logIfOpenedForIllegalAccess(AccessibleObject.java:366) java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:325) java.lang.reflect.AccessibleObject.checkCanSetAccessible(AccessibleObject.java:280) java.lang.reflect.Constructor.checkCanSetAccessible(Constructor.java:189) java.lang.reflect.Constructor.setAccessible(Constructor.java:182) org.jclouds.reflect.Reflection2$1.load(Reflection2$1.java:157) org.jclouds.reflect.Reflection2$1.load(Reflection2$1.java:153) com.google.common.cache.LocalCache$LoadingValueReference.loadFuture(LocalCache$LoadingValueReference.java:3529) com.google.common.cache.LocalCache$Segment.loadSync(LocalCache$Segment.java:2278) com.google.common.cache.LocalCache$Segment.lockedGetOrLoad(LocalCache$Segment.java:2155) com.google.common.cache.LocalCache$Segment.get(LocalCache$Segment.java:2045) com.google.common.cache.LocalCache.get(LocalCache.java:3953) com.google.common.cache.LocalCache.getOrLoad(LocalCache.java:3976) com.google.common.cache.LocalCache$LocalLoadingCache.get(LocalCache$LocalLoadingCache.java:4960) org.jclouds.reflect.Reflection2.get(Reflection2.java:346) org.jclouds.reflect.Reflection2.constructors(Reflection2.java:100) org.jclouds.json.internal.NamingStrategies$AnnotationConstructorNamingStrategy.getDeserializer(NamingStrategies$AnnotationConstructorNamingStrategy.java:271) org.jclouds.json.internal.DeserializationConstructorAndReflectiveTypeAdapterFactory.create(DeserializationConstructorAndReflectiveTypeAdapterFactory.java:125) com.google.gson.Gson.getAdapter(Gson.java:458) org.jclouds.json.gson.internal.bind.ReflectiveTypeAdapterFactory.createBoundField(ReflectiveTypeAdapterFactory.java:117) org.jclouds.json.gson.internal.bind.ReflectiveTypeAdapterFactory.getBoundFields(ReflectiveTypeAdapterFactory.java:166) org.jclouds.json.gson.internal.bind.ReflectiveTypeAdapterFactory.create(ReflectiveTypeAdapterFactory.java:102) com.google.gson.Gson.getAdapter(Gson.java:458) com.google.gson.Gson.toJson(Gson.java:696) com.google.gson.Gson.toJson(Gson.java:683) com.google.gson.Gson.toJson(Gson.java:638) com.google.gson.Gson.toJson(Gson.java:618) org.jclouds.json.internal.GsonWrapper.toJson(GsonWrapper.java:65) org.jclouds.oauth.v2.functions.ClaimsToAssertion.apply(ClaimsToAssertion.java:59) org.jclouds.oauth.v2.functions.ClaimsToAssertion.apply(ClaimsToAssertion.java:43)
[GitHub] [jclouds] nacx commented on pull request #76: JCLOUDS-1542 Java 11 warns of illegal reflective access
nacx commented on pull request #76: URL: https://github.com/apache/jclouds/pull/76#issuecomment-650315218 Thanks! 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: us...@infra.apache.org
[GitHub] [jclouds] JnRouvignac commented on pull request #76: JCLOUDS-1542 Java 11 warns of illegal reflective access
JnRouvignac commented on pull request #76: URL: https://github.com/apache/jclouds/pull/76#issuecomment-650299955 No problem. Thank you for taking the time to review it! I'll follow up with the another Pr with the `!canAccess()` when this one is merged. 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: us...@infra.apache.org
[GitHub] [jclouds] JnRouvignac commented on a change in pull request #76: JCLOUDS-1542 Java 11 warns of illegal reflective access
JnRouvignac commented on a change in pull request #76:
URL: https://github.com/apache/jclouds/pull/76#discussion_r446048154
##
File path: core/src/main/java/org/jclouds/reflect/Reflection2.java
##
@@ -153,15 +153,23 @@
.newBuilder().build(new CacheLoader, Set>>() {
public Set> load(TypeToken key) {
ImmutableSet.Builder> builder =
ImmutableSet.> builder();
- for (Constructor ctor :
key.getRawType().getDeclaredConstructors()) {
- ctor.setAccessible(true);
+ Class raw = key.getRawType();
+ for (Constructor ctor : raw.getDeclaredConstructors()) {
+ if (!coreJavaClass(raw)) {
+ // In JDK 11 up to 14, the only uses for
`java.beans.ConstructorProperties` annotation
+ // are in the `java.awt`, `java.beans` and `javax.swing`
packages.
+ // Since these constructors are public, there is no need
to call `setAccessible()`
Review comment:
Note 1: `isAccessible()` is not very useful. I think you meant
`canAccess()`.
Note 2: if if understand correctly, this code does not look performance
sensitive: it is only called once for each `TypeToken` (i.e. for each class?)
in the lifetime of an application, since I see no expiration for this cache.
> I think it makes sense to consider that potential improvement - here and
also in methodsForTypeToken - as a separate PR if desired, though.
I would also prefer to have the original PR in as it fixes the problem, then
I am also happy to follow-up with another PR to add the `canAccess()` checks
everywhere.
Let me know how you want to proceed.
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:
us...@infra.apache.org
[GitHub] [jclouds] nacx commented on a change in pull request #76: JCLOUDS-1542 Java 11 warns of illegal reflective access
nacx commented on a change in pull request #76:
URL: https://github.com/apache/jclouds/pull/76#discussion_r446095291
##
File path: core/src/main/java/org/jclouds/reflect/Reflection2.java
##
@@ -153,15 +153,23 @@
.newBuilder().build(new CacheLoader, Set>>() {
public Set> load(TypeToken key) {
ImmutableSet.Builder> builder =
ImmutableSet.> builder();
- for (Constructor ctor :
key.getRawType().getDeclaredConstructors()) {
- ctor.setAccessible(true);
+ Class raw = key.getRawType();
+ for (Constructor ctor : raw.getDeclaredConstructors()) {
+ if (!coreJavaClass(raw)) {
+ // In JDK 11 up to 14, the only uses for
`java.beans.ConstructorProperties` annotation
+ // are in the `java.awt`, `java.beans` and `javax.swing`
packages.
+ // Since these constructors are public, there is no need
to call `setAccessible()`
Review comment:
Thanks for the replies, and yes, that makes sense. Let's keep the
original proposed change as it addresses the issue and it aligns with the
existing approach in the other method Andrew mentioned.
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:
us...@infra.apache.org
[GitHub] [jclouds] JnRouvignac commented on a change in pull request #76: JCLOUDS-1542 Java 11 warns of illegal reflective access
JnRouvignac commented on a change in pull request #76:
URL: https://github.com/apache/jclouds/pull/76#discussion_r446048154
##
File path: core/src/main/java/org/jclouds/reflect/Reflection2.java
##
@@ -153,15 +153,23 @@
.newBuilder().build(new CacheLoader, Set>>() {
public Set> load(TypeToken key) {
ImmutableSet.Builder> builder =
ImmutableSet.> builder();
- for (Constructor ctor :
key.getRawType().getDeclaredConstructors()) {
- ctor.setAccessible(true);
+ Class raw = key.getRawType();
+ for (Constructor ctor : raw.getDeclaredConstructors()) {
+ if (!coreJavaClass(raw)) {
+ // In JDK 11 up to 14, the only uses for
`java.beans.ConstructorProperties` annotation
+ // are in the `java.awt`, `java.beans` and `javax.swing`
packages.
+ // Since these constructors are public, there is no need
to call `setAccessible()`
Review comment:
Note 1: `isAccessible()` is not useful. I think you meant `canAccess()`.
Note 2: if if understand correctly, this code does not look performance
sensitive: it is only called once for each `TypeToken` (i.e. for each class?)
in the lifetime of an application, since I see no expiration for this cache.
> I think it makes sense to consider that potential improvement - here and
also in methodsForTypeToken - as a separate PR if desired, though.
I would also prefer to have the original PR in as it fixes the problem, then
I am also happy to follow-up with another to add the `canAccess()` checks
everywhere.
Let me know how you want to proceed.
##
File path: core/src/main/java/org/jclouds/reflect/Reflection2.java
##
@@ -153,15 +153,23 @@
.newBuilder().build(new CacheLoader, Set>>() {
public Set> load(TypeToken key) {
ImmutableSet.Builder> builder =
ImmutableSet.> builder();
- for (Constructor ctor :
key.getRawType().getDeclaredConstructors()) {
- ctor.setAccessible(true);
+ Class raw = key.getRawType();
+ for (Constructor ctor : raw.getDeclaredConstructors()) {
+ if (!coreJavaClass(raw)) {
+ // In JDK 11 up to 14, the only uses for
`java.beans.ConstructorProperties` annotation
+ // are in the `java.awt`, `java.beans` and `javax.swing`
packages.
+ // Since these constructors are public, there is no need
to call `setAccessible()`
Review comment:
Note 1: `isAccessible()` is not very useful. I think you meant
`canAccess()`.
Note 2: if if understand correctly, this code does not look performance
sensitive: it is only called once for each `TypeToken` (i.e. for each class?)
in the lifetime of an application, since I see no expiration for this cache.
> I think it makes sense to consider that potential improvement - here and
also in methodsForTypeToken - as a separate PR if desired, though.
I would also prefer to have the original PR in as it fixes the problem, then
I am also happy to follow-up with another to add the `canAccess()` checks
everywhere.
Let me know how you want to proceed.
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:
us...@infra.apache.org
