[GitHub] struts pull request #153: WW-4827 Not fully initialized ObjectFactory tries ...

classic Classic list List threaded Threaded
27 messages Options
12
Reply | Threaded
Open this post in threaded view
|  
Report Content as Inappropriate

[GitHub] struts pull request #153: WW-4827 Not fully initialized ObjectFactory tries ...

cnenning
GitHub user aleksandr-m opened a pull request:

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

    WW-4827 Not fully initialized ObjectFactory tries to create beans

     Inject `Container` in constructor of the `ObjectFactory`.
   
    Can someone test `cdi`, `osgi` and `plexus` plugins.
   
    Better ideas how to fix the issue are welcome. :)

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

    $ git pull https://github.com/aleksandr-m/struts feature/container_injection

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

    https://github.com/apache/struts/pull/153.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 #153
   
----
commit 6f91d0776a545c911ca4f2875ed9976614711ef9
Author: Aleksandr Mashchenko <[hidden email]>
Date:   2017-07-27T19:22:33Z

    Inject Container in constructor of the ObjectFactory

----


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

cnenning
Github user lukaszlenart commented on the issue:

    https://github.com/apache/struts/pull/153
 
    No better idea and I think we can assume when you need to inject the `Container` you should always do it in a constructor.


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    To keep `backward-compatible` if is very important, I think we may have following:
   
   
    ```java
    protected Object injectInternalBeans(Object obj) {
        if (obj != null) {
            if(container != null) container.inject(obj);
            else myPrivateInjectQueue.add(obj);
        }
        return obj;
    }
       
    @Inject
    public void setContainer(Container container) {
        this.container = container;
        if(null != this.container && myPrivateInjectQueue.getCount()>0)
        {
            foreach(Object obj in myPrivateInjectQueue) this.container.inject(obj);
            myPrivateInjectQueue.clear();
        }
    }
    ```
   
    However I never tried and tested it but I can if you would like.


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    I'm not sure if this is a good idea ... basically we need a `@PostConstruct` mechanism in Struts DI - this can be easily achieved by switching to Guice 3 and using CDI annotations.


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    Hm... give me few minutes, maybe I will be able add support for it ;-)


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    I have opened #155 - what do you think? @aleksandr-m can you test it locally as I do not have the exact setup.


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    Nope, #155 doesn't seem have any effect.
   
    Add following files to project, run under *nix, execute some action which lead to the JSP,  `localizedTextProvider` should not be `null` in system outs.
   
    xwork-conversion.properties;
   
        java.util.Date=com.DateConverter
   
    JSP:
     
        <s:bean var="d" name="java.util.Date" />
        <s:property value="#d" />
   
    DateConverter:
    ```
    public class DateConverter extends StrutsTypeConverter {
   
        private LocalizedTextProvider localizedTextProvider;
   
        @Inject
        public void setLocalizedTextProvider(LocalizedTextProvider localizedTextProvider) {
            this.localizedTextProvider = localizedTextProvider;
        }
   
        @Override
        public Object convertFromString(Map context, String[] values, Class toClass) {
            System.out.println("DateConverter.convertFromString() localizedTextProvider=" +
                                          localizedTextProvider);
            return null;
        }
   
        @Override
        public String convertToString(Map context, Object obj) {
            System.out.println("DateConverter.convertToString() localizedTextProvider=" +
                                          localizedTextProvider);
            return "";
        }
    }
    ```
   



---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    hm... but this example works for both branches - `master` and this:
    ```
    DateConverter.convertToString() localizedTextProvider=com.opensymphony.xwork2.util.StrutsLocalizedTextProvider@342e1b8b
    ```


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    (on OSX)


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    One more question: JDK version?


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    Ubuntu, oracle jdk8.
   
    If following code prints `setContainer` before other setters (especially before `setInterceptorFactory`) it will work. On Ubuntu using oracle jdk8 `setContainer` is printed way below other setters and injection fails.
   
        Method[] methods = ObjectFactory.class.getDeclaredMethods();
        for (Method m : methods) {
            System.out.println(m);
        }



---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    See these screenshots
   
    #### master
    ![2017-08-01_2119](https://user-images.githubusercontent.com/170103/28842993-f915533e-76ff-11e7-8434-09d24777704f.png)
   
    #### this branch
    ![2017-08-01_2124](https://user-images.githubusercontent.com/170103/28843001-ff0a8e30-76ff-11e7-8c3f-b9be85140241.png)
   
    as you see, on the `master` branch the object isn't fully initialised but when `init()` method is called when using this branch, the object got fully initialised.
   
    Maybe you have discovered another place where there is some work done in setter which should be moved into the `init()` method with the `PostInit` interface.
   
   



---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    It isn't about `XWorkConverter` per se. In #155 the injection of `ConversionPropertiesProcessor` still happens and `TypeConverterCreator` is injected next and `ObjectFactory#buildBean` is called and `container` is `null` in `ObjectFactory`.
   
    BTW `ObjectFactory.class.getDeclaredMethods()` on Ubuntu, openJDK 7 prints `setContainer` as the last method.


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    ... but where the `ObjectFactory#buildBean` is called? It should only be called when building non-internal beans and when all the internal beans were already instantiated. Can you post a call trace?


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    I have changed code a bit, can you try to test it on your side?


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    With the new code in #155 the custom type converter isn't initialized at all. :(
   
    Call trace of previous code in #155
    ```


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    > With the new code in #155 the custom type converter isn't initialized at all. :(
   
    w00t!


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    Would it be possible to share an application that replicates this behaviour? Sorry for bothering you but I want to understand what's going on :)


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    I don't see any other options now to merge this. @aleksandr-m could you prepare that example app? I will be able to investigate what's wrong with my solution ;-)


---
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
|  
Report Content as Inappropriate

[GitHub] struts issue #153: WW-4827 Not fully initialized ObjectFactory tries to crea...

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

    https://github.com/apache/struts/pull/153
 
    Sure. No problem.
   
    Test project - https://github.com/aleksandr-m/struts2-objectfactory-container/
   
    This commit adds custom date converter - https://github.com/aleksandr-m/struts2-objectfactory-container/commit/49406c1d8e816acd1f227d55db6729cd92a5de99.
   
    This commit adds local copy of `ContainerImpl` in order to reproduce the issue in various environments - https://github.com/aleksandr-m/struts2-objectfactory-container/commit/e3fb324c56c8517491ea336f8682601ae33a5f0b


---
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]

12
Loading...