[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

classic Classic list List threaded Threaded
20 messages Options
Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
GitHub user yasserzamani opened a pull request:

    https://github.com/apache/struts/pull/135

    WW-4105 Unwraps Spring proxy in actions chain

    Resolves [WW-4105](https://issues.apache.org/jira/browse/WW-4105).

You can merge this pull request into a Git repository by running:

    $ git pull https://github.com/yasserzamani/struts WW-4105_2

Alternatively you can review and apply these changes as the patch at:

    https://github.com/apache/struts/pull/135.patch

To close this pull request, make a commit to your master/trunk branch
with (at least) the following in the commit message:

    This closes #135
   
----
commit c2f2de06ce3c5545878a3b421fdde3ca5495d3cc
Author: Yasser Zamani <[hidden email]>
Date:   2017-04-30T04:50:57Z

    WW-4105 Unwraps Spring proxy in actions chain

----


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
Github user aleksandr-m commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114109147
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    --- End diff --
   
    Maybe name it `ProxyUtil` or something like that. This class can be used to deal with known proxies and not only spring proxies.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user aleksandr-m commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114109235
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    +    /**
    +     * Get the ultimate <em>target</em> object of the supplied {@code candidate}
    +     * object, unwrapping not only a top-level proxy but also any number of
    +     * nested proxies.
    +     * <p>If the supplied {@code candidate} is a Spring proxy, the ultimate target of all
    +     * nested proxies will be returned; otherwise, the {@code candidate}
    +     * will be returned <em>as is</em>.
    +     * @param candidate the instance to check (potentially a Spring AOP proxy;
    +     * never {@code null})
    +     * @return the target object or the {@code candidate} (never {@code null})
    +     * @throws IllegalStateException if an error occurs while unwrapping a proxy
    +     */
    +    public static <T> T getUltimateTargetObject(Object candidate) {
    +        try {
    +            if (isAopProxy(candidate) &&
    +                    implementsInterface(candidate.getClass(), "org.springframework.aop.framework.Advised")) {
    +                Object targetSource = MethodUtils.invokeMethod(candidate, "getTargetSource");
    --- End diff --
   
    What is `targetSource` here?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user aleksandr-m commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114109357
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    +    /**
    +     * Get the ultimate <em>target</em> object of the supplied {@code candidate}
    +     * object, unwrapping not only a top-level proxy but also any number of
    +     * nested proxies.
    +     * <p>If the supplied {@code candidate} is a Spring proxy, the ultimate target of all
    +     * nested proxies will be returned; otherwise, the {@code candidate}
    +     * will be returned <em>as is</em>.
    +     * @param candidate the instance to check (potentially a Spring AOP proxy;
    +     * never {@code null})
    +     * @return the target object or the {@code candidate} (never {@code null})
    +     * @throws IllegalStateException if an error occurs while unwrapping a proxy
    +     */
    +    public static <T> T getUltimateTargetObject(Object candidate) {
    +        try {
    +            if (isAopProxy(candidate) &&
    +                    implementsInterface(candidate.getClass(), "org.springframework.aop.framework.Advised")) {
    +                Object targetSource = MethodUtils.invokeMethod(candidate, "getTargetSource");
    +                Object target = MethodUtils.invokeMethod(targetSource, "getTarget");
    +                return getUltimateTargetObject(target);
    +            }
    +        }
    +        catch (Throwable ex) {
    +            throw new IllegalStateException("Failed to unwrap proxied object", ex);
    +        }
    +        return (T) candidate;
    +    }
    +
    +    /**
    +     * Check whether the given object is a Spring proxy.
    +     * @param object the object to check
    +     */
    +    public static boolean isAopProxy(Object object) {
    +        Class<?> clazz = object.getClass();
    +        return (implementsInterface(clazz, "org.springframework.aop.SpringProxy") &&
    --- End diff --
   
    Extract strings to constants.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user aleksandr-m commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114109604
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    --- End diff --
   
    Another maybe. This can be turned to injectable bean so it can be easily overridable if needed. Wdyt?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user yasserzamani commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114115352
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    +    /**
    +     * Get the ultimate <em>target</em> object of the supplied {@code candidate}
    +     * object, unwrapping not only a top-level proxy but also any number of
    +     * nested proxies.
    +     * <p>If the supplied {@code candidate} is a Spring proxy, the ultimate target of all
    +     * nested proxies will be returned; otherwise, the {@code candidate}
    +     * will be returned <em>as is</em>.
    +     * @param candidate the instance to check (potentially a Spring AOP proxy;
    +     * never {@code null})
    +     * @return the target object or the {@code candidate} (never {@code null})
    +     * @throws IllegalStateException if an error occurs while unwrapping a proxy
    +     */
    +    public static <T> T getUltimateTargetObject(Object candidate) {
    +        try {
    +            if (isAopProxy(candidate) &&
    +                    implementsInterface(candidate.getClass(), "org.springframework.aop.framework.Advised")) {
    +                Object targetSource = MethodUtils.invokeMethod(candidate, "getTargetSource");
    --- End diff --
   
    It is what Spring himself does to unwrap it's proxies. Please see [AopTestUtils.java](https://github.com/spring-projects/spring-framework/blob/master/spring-test/src/main/java/org/springframework/test/util/AopTestUtils.java)


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user yasserzamani commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114115548
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    --- End diff --
   
    Why someone may need to override such specific methods?
   
    As all methods could be static, I thought normally it should be a static utils class rather than a bean object.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user yasserzamani commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114115598
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    +    /**
    +     * Get the ultimate <em>target</em> object of the supplied {@code candidate}
    +     * object, unwrapping not only a top-level proxy but also any number of
    +     * nested proxies.
    +     * <p>If the supplied {@code candidate} is a Spring proxy, the ultimate target of all
    +     * nested proxies will be returned; otherwise, the {@code candidate}
    +     * will be returned <em>as is</em>.
    +     * @param candidate the instance to check (potentially a Spring AOP proxy;
    +     * never {@code null})
    +     * @return the target object or the {@code candidate} (never {@code null})
    +     * @throws IllegalStateException if an error occurs while unwrapping a proxy
    +     */
    +    public static <T> T getUltimateTargetObject(Object candidate) {
    +        try {
    +            if (isAopProxy(candidate) &&
    +                    implementsInterface(candidate.getClass(), "org.springframework.aop.framework.Advised")) {
    +                Object targetSource = MethodUtils.invokeMethod(candidate, "getTargetSource");
    +                Object target = MethodUtils.invokeMethod(targetSource, "getTarget");
    +                return getUltimateTargetObject(target);
    +            }
    +        }
    +        catch (Throwable ex) {
    +            throw new IllegalStateException("Failed to unwrap proxied object", ex);
    +        }
    +        return (T) candidate;
    +    }
    +
    +    /**
    +     * Check whether the given object is a Spring proxy.
    +     * @param object the object to check
    +     */
    +    public static boolean isAopProxy(Object object) {
    +        Class<?> clazz = object.getClass();
    +        return (implementsInterface(clazz, "org.springframework.aop.SpringProxy") &&
    --- End diff --
   
    Thank you! I will do in next commit.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user yasserzamani commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114115750
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    --- End diff --
   
    Thank you! I will do in next commit. However, I also try my effort to remove it to commons-lang if guys there accepted.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user aleksandr-m commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114148402
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    +    /**
    +     * Get the ultimate <em>target</em> object of the supplied {@code candidate}
    +     * object, unwrapping not only a top-level proxy but also any number of
    +     * nested proxies.
    +     * <p>If the supplied {@code candidate} is a Spring proxy, the ultimate target of all
    +     * nested proxies will be returned; otherwise, the {@code candidate}
    +     * will be returned <em>as is</em>.
    +     * @param candidate the instance to check (potentially a Spring AOP proxy;
    +     * never {@code null})
    +     * @return the target object or the {@code candidate} (never {@code null})
    +     * @throws IllegalStateException if an error occurs while unwrapping a proxy
    +     */
    +    public static <T> T getUltimateTargetObject(Object candidate) {
    +        try {
    +            if (isAopProxy(candidate) &&
    +                    implementsInterface(candidate.getClass(), "org.springframework.aop.framework.Advised")) {
    +                Object targetSource = MethodUtils.invokeMethod(candidate, "getTargetSource");
    --- End diff --
   
    Hmm... Never used `AopTestUtils`. But `Advised` extends `TargetClassAware`. Can `getTargetClass` just be invoked instead of two method calls? See [`AopUtils`](https://github.com/spring-projects/spring-framework/blob/master/spring-aop/src/main/java/org/springframework/aop/support/AopUtils.java).


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user aleksandr-m commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114148973
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    --- End diff --
   
    To support other possible proxies. Or in case something changes in spring-aop.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user yasserzamani commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114155980
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    +    /**
    +     * Get the ultimate <em>target</em> object of the supplied {@code candidate}
    +     * object, unwrapping not only a top-level proxy but also any number of
    +     * nested proxies.
    +     * <p>If the supplied {@code candidate} is a Spring proxy, the ultimate target of all
    +     * nested proxies will be returned; otherwise, the {@code candidate}
    +     * will be returned <em>as is</em>.
    +     * @param candidate the instance to check (potentially a Spring AOP proxy;
    +     * never {@code null})
    +     * @return the target object or the {@code candidate} (never {@code null})
    +     * @throws IllegalStateException if an error occurs while unwrapping a proxy
    +     */
    +    public static <T> T getUltimateTargetObject(Object candidate) {
    +        try {
    +            if (isAopProxy(candidate) &&
    +                    implementsInterface(candidate.getClass(), "org.springframework.aop.framework.Advised")) {
    +                Object targetSource = MethodUtils.invokeMethod(candidate, "getTargetSource");
    --- End diff --
   
    `getTargetClass` just returns a `Class` but getting thae target `Object` (i.e. actual action object) let us pass it directly to `ReflectionProvider` without any needed modification of `ReflectionProvider` codes.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user yasserzamani commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r114156448
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    --- End diff --
   
    I think we should update Struts itself in both cases when reported by a user.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user lukaszlenart commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r115664014
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    --- End diff --
   
    `SpringUtils` doesn't sound good but I have no better name for now. Also it's the only case we have now so we can always refactor this code and introduce an interface with few implementation latter on.
   
    For future reference: it would be good to have a `ProxyAware` interface in the `core` with a default implementation (that does nothing) and then in the Spring Plugin provide a real implementation.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user yasserzamani commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r115767804
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    --- End diff --
   
    Thank you! Unfortunately I did not understand well. Do you mean that the user has to be careful to implement `ProxyAware` or inherit Spring Plugin's default implementation in his proxied actions? If so, there are two disadvantages, first he has to be careful about which actions may be proxied, and second he has to add Spring Plugin dependency or implement it manually.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user lukaszlenart commented on a diff in the pull request:

    https://github.com/apache/struts/pull/135#discussion_r115808166
 
    --- Diff: core/src/main/java/com/opensymphony/xwork2/spring/SpringUtils.java ---
    @@ -0,0 +1,89 @@
    +/*
    + * Copyright 2017 The Apache Software Foundation.
    + *
    + * Licensed under the Apache License, Version 2.0 (the "License");
    + * you may not use this file except in compliance with the License.
    + * You may obtain a copy of the License at
    + *
    + *      http://www.apache.org/licenses/LICENSE-2.0
    + *
    + * Unless required by applicable law or agreed to in writing, software
    + * distributed under the License is distributed on an "AS IS" BASIS,
    + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
    + * See the License for the specific language governing permissions and
    + * limitations under the License.
    + */
    +package com.opensymphony.xwork2.spring;
    +
    +import com.opensymphony.xwork2.util.ClassLoaderUtil;
    +import org.apache.commons.lang3.reflect.MethodUtils;
    +
    +import java.lang.reflect.Proxy;
    +
    +/**
    + * <code>SpringUtils</code>
    + * <p>
    + * Various utility methods dealing with spring framework
    + * </p>
    + *
    + */
    +public class SpringUtils {
    --- End diff --
   
    no, no ... it was just for a future reference when we will want turn this into a bean


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts issue #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user yasserzamani commented on the issue:

    https://github.com/apache/struts/pull/135
 
    @lukaszlenart , I updated with @aleksandr-m reviews. I hope it is ready for merge if you pals agree.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts issue #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user lukaszlenart commented on the issue:

    https://github.com/apache/struts/pull/135
 
    I think with can go ahead and merge this, @aleksandr-m any objections?


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts issue #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user aleksandr-m commented on the issue:

    https://github.com/apache/struts/pull/135
 
    @lukaszlenart I think it is ok.


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]

Reply | Threaded
Open this post in threaded view
|

[GitHub] struts pull request #135: WW-4105 Unwraps Spring proxy in actions chain

yasserzamani
In reply to this post by yasserzamani
Github user asfgit closed the pull request at:

    https://github.com/apache/struts/pull/135


---
If your project is set up for it, you can reply to this email and have your
reply appear on GitHub as well. If your project does not have this feature
enabled and wishes so, or if the feature is enabled but not working, please
contact infrastructure at [hidden email] or file a JIRA ticket
with INFRA.
---

---------------------------------------------------------------------
To unsubscribe, e-mail: [hidden email]
For additional commands, e-mail: [hidden email]