You are viewing a plain text version of this content. The canonical link for it is here.
Posted to notifications@struts.apache.org by "kusalk (via GitHub)" <gi...@apache.org> on 2023/12/29 03:07:37 UTC
[PR] WW-5381 Introduce extension point for MethodAccessor [struts]
kusalk opened a new pull request, #825:
URL: https://github.com/apache/struts/pull/825
WW-5381
--
Fairly straight forward again
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on code in PR #825:
URL: https://github.com/apache/struts/pull/825#discussion_r1441370609
##########
core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java:
##########
@@ -87,12 +92,6 @@ protected void setContainer(Container container) throws ClassNotFoundException {
OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name));
}
- names = container.getInstanceNames(MethodAccessor.class);
Review Comment:
Hmm... so maybe just mark this functionality as `@deprecated` or remove - once someone starts complain we can re-think how to put back such functionality.
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #825:
URL: https://github.com/apache/struts/pull/825#discussion_r1437961845
##########
core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java:
##########
@@ -87,12 +92,6 @@ protected void setContainer(Container container) throws ClassNotFoundException {
OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name));
}
- names = container.getInstanceNames(MethodAccessor.class);
Review Comment:
Now there is one drawback here - applications lose the ability to specify custom method accessors for different types. They will now instead need to use the extension point and do the delegation within this "global" method accessor. I'm not sure there's a way to retain this capability and make it compatible with the current extension point system.
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on code in PR #825:
URL: https://github.com/apache/struts/pull/825#discussion_r1443768942
##########
core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java:
##########
@@ -87,12 +92,6 @@ protected void setContainer(Container container) throws ClassNotFoundException {
OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name));
}
- names = container.getInstanceNames(MethodAccessor.class);
Review Comment:
Looks good to me, 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.
To unsubscribe, e-mail: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk merged PR #825:
URL: https://github.com/apache/struts/pull/825
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #825:
URL: https://github.com/apache/struts/pull/825#issuecomment-1871699001
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) **Quality Gate failed**
Failed conditions
[8 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=825&resolved=false&inNewCodePeriod=true)
[28.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_coverage&view=list) (required ≥ 80%)
[4.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_duplicated_lines_density&view=list) (required ≤ 3%)
[E Reliability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) (required ≥ A)
[E Security Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) (required ≥ A)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on PR #825:
URL: https://github.com/apache/struts/pull/825#issuecomment-1871698686
(SonarCloud report is comparing against wrong base branch)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #825:
URL: https://github.com/apache/struts/pull/825#issuecomment-1878884217
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) **Quality Gate failed**
Failed conditions
[10 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=825&resolved=false&inNewCodePeriod=true)
[28.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_coverage&view=list) (required ≥ 80%)
[4.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_duplicated_lines_density&view=list) (required ≤ 3%)
[E Reliability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) (required ≥ A)
[E Security Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) (required ≥ A)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #825:
URL: https://github.com/apache/struts/pull/825#discussion_r1443034231
##########
core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java:
##########
@@ -87,12 +92,6 @@ protected void setContainer(Container container) throws ClassNotFoundException {
OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name));
}
- names = container.getInstanceNames(MethodAccessor.class);
Review Comment:
I think we are good now :) Hopefully the code makes sense, added some JavaDoc
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #825:
URL: https://github.com/apache/struts/pull/825#issuecomment-1875172925
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) **Quality Gate passed**
The SonarCloud Quality Gate passed, but some issues were introduced.
[1 New issue](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=825&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=825&resolved=false&inNewCodePeriod=true)
[93.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #825:
URL: https://github.com/apache/struts/pull/825#issuecomment-1875174217
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) **Quality Gate passed**
The SonarCloud Quality Gate passed, but some issues were introduced.
[1 New issue](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=825&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=825&resolved=false&inNewCodePeriod=true)
[93.3% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #825:
URL: https://github.com/apache/struts/pull/825#discussion_r1442639229
##########
core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java:
##########
@@ -87,12 +92,6 @@ protected void setContainer(Container container) throws ClassNotFoundException {
OgnlRuntime.setPropertyAccessor(cls, container.getInstance(PropertyAccessor.class, name));
}
- names = container.getInstanceNames(MethodAccessor.class);
Review Comment:
Hmm actually just had another think and we should be able to retain backwards compatibility, the code will be a little ugly though - let me give it a shot
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org
Re: [PR] WW-5381 Introduce extension point for MethodAccessor [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #825:
URL: https://github.com/apache/struts/pull/825#issuecomment-1878891329
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) **Quality Gate failed**
Failed conditions
[10 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=825&resolved=false&inNewCodePeriod=true)
[28.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_coverage&view=list) (required ≥ 80%)
[4.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=825&metric=new_duplicated_lines_density&view=list) (required ≤ 3%)
[E Security Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) (required ≥ A)
[E Reliability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825) (required ≥ A)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=825)
 Catch issues before they fail your Quality Gate with our IDE extension  [SonarLint](https://www.sonarsource.com/products/sonarlint/features/connected-mode/?referrer=pull-request)
--
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: notifications-unsubscribe@struts.apache.org
For queries about this service, please contact Infrastructure at:
users@infra.apache.org