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 02:47:38 UTC
[PR] WW-5381 Introduce extension point for CompoundRootAccessor [struts]
kusalk opened a new pull request, #824:
URL: https://github.com/apache/struts/pull/824
WW-5381
--
Fairly straightforward
--
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 CompoundRootAccessor [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #824:
URL: https://github.com/apache/struts/pull/824#issuecomment-1871694944
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824) **Quality Gate failed**
Failed conditions
[8 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=824&resolved=false&inNewCodePeriod=true)
[28.6% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=824&metric=new_coverage&view=list) (required ≥ 80%)
[4.1% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=824&metric=new_duplicated_lines_density&view=list) (required ≤ 3%)
[E Security Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824) (required ≥ A)
[E Reliability Rating on New Code](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824) (required ≥ A)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824)
 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 CompoundRootAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on PR #824:
URL: https://github.com/apache/struts/pull/824#issuecomment-1871698665
(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 CompoundRootAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #824:
URL: https://github.com/apache/struts/pull/824#discussion_r1440298370
##########
core/src/main/resources/struts-beans.xml:
##########
@@ -174,11 +174,9 @@
<bean type="com.opensymphony.xwork2.util.TextParser" name="struts"
class="com.opensymphony.xwork2.util.OgnlTextParser" scope="singleton"/>
- <bean type="ognl.ClassResolver" name="com.opensymphony.xwork2.util.CompoundRoot"
+ <bean type="com.opensymphony.xwork2.ognl.accessor.RootAccessor" name="struts"
class="com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor"/>
Review Comment:
Yep very true - looks like our bundled version of Guice is quite old and doesn't support binding beans to multiple interfaces. Although that might not work here either since the Bean name has also changed.
I can create a 2nd bean using the old interface and name to prevent injection failures but obviously this bean won't be the same one used by the `OgnlValueStackFactory` - it might be fine depending on what these classes intend to do with the `CompoundRootAccessor` bean, and if they really need it to share state with the bean injected into `OgnlValueStackFactory`
--
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 CompoundRootAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #824:
URL: https://github.com/apache/struts/pull/824#discussion_r1440303286
##########
core/src/main/resources/struts-beans.xml:
##########
@@ -174,11 +174,9 @@
<bean type="com.opensymphony.xwork2.util.TextParser" name="struts"
class="com.opensymphony.xwork2.util.OgnlTextParser" scope="singleton"/>
- <bean type="ognl.ClassResolver" name="com.opensymphony.xwork2.util.CompoundRoot"
+ <bean type="com.opensymphony.xwork2.ognl.accessor.RootAccessor" name="struts"
class="com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor"/>
Review Comment:
Done - reinstated the old beans for injection compatibility with any custom beans written for Struts 6.3 or earlier
--
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 CompoundRootAccessor [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #824:
URL: https://github.com/apache/struts/pull/824#issuecomment-1875278221
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824) **Quality Gate passed**
Kudos, no new issues were introduced!
[0 New issues](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=824&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=824&resolved=false&inNewCodePeriod=true)
[100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=824&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=824&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824)
--
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 CompoundRootAccessor [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #824:
URL: https://github.com/apache/struts/pull/824#issuecomment-1875165889
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824) **Quality Gate passed**
Kudos, no new issues were introduced!
[0 New issues](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=824&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=824&resolved=false&inNewCodePeriod=true)
[100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=824&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=824&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824)
--
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 CompoundRootAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk commented on code in PR #824:
URL: https://github.com/apache/struts/pull/824#discussion_r1437960453
##########
core/src/main/java/com/opensymphony/xwork2/ognl/OgnlValueStackFactory.java:
##########
@@ -50,9 +50,11 @@ protected void setXWorkConverter(XWorkConverter converter) {
this.xworkConverter = converter;
}
- @Inject(value = "com.opensymphony.xwork2.util.CompoundRoot")
- protected void setClassResolver(ClassResolver classResolver) {
- this.compoundRootAccessor = (RootAccessor) classResolver;
+ @Inject
+ protected void setCompoundRootAccessor(RootAccessor compoundRootAccessor) {
+ this.compoundRootAccessor = compoundRootAccessor;
+ OgnlRuntime.setPropertyAccessor(CompoundRoot.class, compoundRootAccessor);
Review Comment:
Unified the previously 3 separate instances of CompoundRootAccessor
--
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 CompoundRootAccessor [struts]
Posted by "lukaszlenart (via GitHub)" <gi...@apache.org>.
lukaszlenart commented on code in PR #824:
URL: https://github.com/apache/struts/pull/824#discussion_r1440101727
##########
core/src/main/resources/struts-beans.xml:
##########
@@ -174,11 +174,9 @@
<bean type="com.opensymphony.xwork2.util.TextParser" name="struts"
class="com.opensymphony.xwork2.util.OgnlTextParser" scope="singleton"/>
- <bean type="ognl.ClassResolver" name="com.opensymphony.xwork2.util.CompoundRoot"
+ <bean type="com.opensymphony.xwork2.ognl.accessor.RootAccessor" name="struts"
class="com.opensymphony.xwork2.ognl.accessor.CompoundRootAccessor"/>
Review Comment:
Could this be a breaking change? Maybe somehow mention that `ClassResolver` shouldn't be used anymore and implement `CompoundRoot` instead?
--
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 CompoundRootAccessor [struts]
Posted by "kusalk (via GitHub)" <gi...@apache.org>.
kusalk merged PR #824:
URL: https://github.com/apache/struts/pull/824
--
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 CompoundRootAccessor [struts]
Posted by "sonarcloud[bot] (via GitHub)" <gi...@apache.org>.
sonarcloud[bot] commented on PR #824:
URL: https://github.com/apache/struts/pull/824#issuecomment-1875174336
## [](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824) **Quality Gate passed**
Kudos, no new issues were introduced!
[0 New issues](https://sonarcloud.io/project/issues?id=apache_struts&pullRequest=824&resolved=false&inNewCodePeriod=true)
[0 Security Hotspots](https://sonarcloud.io/project/security_hotspots?id=apache_struts&pullRequest=824&resolved=false&inNewCodePeriod=true)
[100.0% Coverage on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=824&metric=new_coverage&view=list)
[0.0% Duplication on New Code](https://sonarcloud.io/component_measures?id=apache_struts&pullRequest=824&metric=new_duplicated_lines_density&view=list)
[See analysis details on SonarCloud](https://sonarcloud.io/dashboard?id=apache_struts&pullRequest=824)
--
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