Uploaded image for project: 'Struts 2'
  1. Struts 2
  2. WW-4827

Not fully initialized ObjectFactory tries to create beans

    Details

    • Type: Bug
    • Status: Closed
    • Priority: Critical
    • Resolution: Fixed
    • Affects Version/s: 2.5.12
    • Fix Version/s: 2.5.13
    • Component/s: None
    • Labels:
      None

      Description

      This leads to issues when properties aren't injected in some cases, for example in custom type converters.

      The problem happens when ObjectFactory tries to create a bean in the same time not being fully initialized itself (e.g. ConverterFactory injected before Container).

      The issue happens more often under linux (all the time basically) than under windows, so it cannot be reproduced 100%. This behavior boils down to the fact that clazz.getDeclaredMethods() is used to get methods which needs to be injected and clazz.getDeclaredMethods() returned elements are - The elements in the array returned are not sorted and are not in any particular order.

      Proposed solution moves Container injection from method to constructor in ObjectFactory - https://github.com/aleksandr-m/struts/commit/6f91d0776a545c911ca4f2875ed9976614711ef9.

      The downside is it isn't backward-compatible, custom object factories must be updated.

        Issue Links

          Activity

          Hide
          lukaszlenart Lukasz Lenart added a comment -

          But this means that the buildConverter method is called before the object was fully initialised, is that right?

          Show
          lukaszlenart Lukasz Lenart added a comment - But this means that the buildConverter method is called before the object was fully initialised, is that right?
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Another solution is to drop all those *Factory fields with setters and use container directly to get an instance of the class, e.g.:

              public TypeConverter buildConverter(Class<? extends TypeConverter> converterClass, Map<String, Object> extraContext) throws Exception {
                  ConverterFactory converterFactory = container.getInstance(ConverterFactory.class);
                  return converterFactory.buildConverter(converterClass, extraContext);
              }
          

          Those fields are private anyway, so nobody could use them.

          Show
          lukaszlenart Lukasz Lenart added a comment - Another solution is to drop all those *Factory fields with setters and use container directly to get an instance of the class, e.g.: public TypeConverter buildConverter( Class <? extends TypeConverter> converterClass, Map< String , Object > extraContext) throws Exception { ConverterFactory converterFactory = container.getInstance(ConverterFactory.class); return converterFactory.buildConverter(converterClass, extraContext); } Those fields are private anyway, so nobody could use them.
          Hide
          aleksandr-m Aleksandr Mashchenko added a comment -

          You mean buildConverter in ObjectFactory? Not sure.

          One particular case when the injection fails goes like that: ObjectFactory is created and injections begins: InterceptorFactory -> ReflectionProvider -> OgnlUtil -> XWorkConverter -> ConversionPropertiesProcessor -> TypeConverterCreator -> ObjectFactory. Then ObjectFactory#buildBean is called, next ObjectFactory#injectInternalBeans and container is null. This is because injection in ObjectFactory started with setInterceptorFactory and setContainer wasn't called yet.

          Show
          aleksandr-m Aleksandr Mashchenko added a comment - You mean buildConverter in ObjectFactory ? Not sure. One particular case when the injection fails goes like that: ObjectFactory is created and injections begins: InterceptorFactory -> ReflectionProvider -> OgnlUtil -> XWorkConverter -> ConversionPropertiesProcessor -> TypeConverterCreator -> ObjectFactory . Then ObjectFactory#buildBean is called, next ObjectFactory#injectInternalBeans and container is null. This is because injection in ObjectFactory started with setInterceptorFactory and setContainer wasn't called yet.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          This part is from XWorkConverter

              @Inject
              public void setConversionPropertiesProcessor(ConversionPropertiesProcessor propertiesProcessor) {
                  propertiesProcessor.processRequired("struts-default-conversion.properties");
                  propertiesProcessor.process("xwork-conversion.properties");
              }
          

          and I recall having some issues with this as well ... but I cannot recall any details :\

          Show
          lukaszlenart Lukasz Lenart added a comment - This part is from XWorkConverter @Inject public void setConversionPropertiesProcessor(ConversionPropertiesProcessor propertiesProcessor) { propertiesProcessor.processRequired( "struts- default -conversion.properties" ); propertiesProcessor.process( "xwork-conversion.properties" ); } and I recall having some issues with this as well ... but I cannot recall any details :\
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          This a more issue about having a PostConstruct initialisation logic ...

          Show
          lukaszlenart Lukasz Lenart added a comment - This a more issue about having a PostConstruct initialisation logic ...
          Hide
          quaff zhouyanming added a comment -

          Aleksandr Mashchenko You can add a default constructor to keep backward-compatible

          Show
          quaff zhouyanming added a comment - Aleksandr Mashchenko You can add a default constructor to keep backward-compatible
          Hide
          githubbot ASF GitHub Bot added a comment -

          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 <amashchenko@apache.org>
          Date: 2017-07-27T19:22:33Z

          Inject Container in constructor of the ObjectFactory


          Show
          githubbot ASF GitHub Bot added a comment - 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 <amashchenko@apache.org> Date: 2017-07-27T19:22:33Z Inject Container in constructor of the ObjectFactory
          Hide
          githubbot ASF GitHub Bot added a comment -

          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.

          Show
          githubbot ASF GitHub Bot added a comment - 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.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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.

          Show
          githubbot ASF GitHub Bot added a comment - 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.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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.

          Show
          githubbot ASF GitHub Bot added a comment - 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.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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

          Show
          githubbot ASF GitHub Bot added a comment - 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
          Hide
          githubbot ASF GitHub Bot added a comment -

          GitHub user lukaszlenart opened a pull request:

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

          WW-4827: post init

          Resolves WW-4827(https://issues.apache.org/jira/browse/WW-4827)

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

          $ git pull https://github.com/lukaszlenart/struts post-init

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

          https://github.com/apache/struts/pull/155.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 #155


          commit 14d7a6672ca8fadf7d09ee932933326e8db9597b
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-08-01T14:29:00Z

          Defines an interface to allow init an object after constructing it

          commit 8549f446306187d187e94de86422cb0ad5a2d96f
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-08-01T14:30:16Z

          Implements PostInit logic

          commit 32fed918c33961e42f29982c00cb4c5b9a178895
          Author: Lukasz Lenart <lukaszlenart@apache.org>
          Date: 2017-08-01T14:30:38Z

          Uses post init logic to delay initialisation of resources


          Show
          githubbot ASF GitHub Bot added a comment - GitHub user lukaszlenart opened a pull request: https://github.com/apache/struts/pull/155 WW-4827 : post init Resolves WW-4827 ( https://issues.apache.org/jira/browse/WW-4827 ) You can merge this pull request into a Git repository by running: $ git pull https://github.com/lukaszlenart/struts post-init Alternatively you can review and apply these changes as the patch at: https://github.com/apache/struts/pull/155.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 #155 commit 14d7a6672ca8fadf7d09ee932933326e8db9597b Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-08-01T14:29:00Z Defines an interface to allow init an object after constructing it commit 8549f446306187d187e94de86422cb0ad5a2d96f Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-08-01T14:30:16Z Implements PostInit logic commit 32fed918c33961e42f29982c00cb4c5b9a178895 Author: Lukasz Lenart <lukaszlenart@apache.org> Date: 2017-08-01T14:30:38Z Uses post init logic to delay initialisation of resources
          Hide
          githubbot ASF GitHub Bot added a comment -

          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.

          Show
          githubbot ASF GitHub Bot added a comment - 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.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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 ""; }

          }
          ```

          Show
          githubbot ASF GitHub Bot added a comment - 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 ""; } } ```
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          Show
          githubbot ASF GitHub Bot added a comment - 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 ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

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

          (on OSX)

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/153 (on OSX)
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

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

          One more question: JDK version?

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/153 One more question: JDK version?
          Hide
          githubbot ASF GitHub Bot added a comment -

          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); }
          Show
          githubbot ASF GitHub Bot added a comment - 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); }
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

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

          See these screenshots

                1. master
                  ![2017-08-01_2119](https://user-images.githubusercontent.com/170103/28842993-f915533e-76ff-11e7-8434-09d24777704f.png)
                1. 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.

          Show
          githubbot ASF GitHub Bot added a comment - 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.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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.

          Show
          githubbot ASF GitHub Bot added a comment - 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.
          Hide
          githubbot ASF GitHub Bot added a comment -

          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?

          Show
          githubbot ASF GitHub Bot added a comment - 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?
          Hide
          githubbot ASF GitHub Bot added a comment -

          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?

          Show
          githubbot ASF GitHub Bot added a comment - 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?
          Hide
          githubbot ASF GitHub Bot added a comment -

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

          Show
          githubbot ASF GitHub Bot added a comment - 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 ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          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!

          Show
          githubbot ASF GitHub Bot added a comment - 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!
          Hide
          githubbot ASF GitHub Bot added a comment -

          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

          Show
          githubbot ASF GitHub Bot added a comment - 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
          Hide
          githubbot ASF GitHub Bot added a comment -

          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

          Show
          githubbot ASF GitHub Bot added a comment - 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
          Hide
          githubbot ASF GitHub Bot added a comment -

          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

          Show
          githubbot ASF GitHub Bot added a comment - 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
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

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

          Thanks a lot I see now what's going on but have no idea why :\

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/153 Thanks a lot I see now what's going on but have no idea why :\
          Hide
          jira-bot ASF subversion and git services added a comment -

          Commit 81718c62bdf4b22867cc92087b350fd1c2873979 in struts's branch refs/heads/master from Lukasz Lenart
          [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=81718c6 ]

          WW-4827 Injects Container in constructor

          Show
          jira-bot ASF subversion and git services added a comment - Commit 81718c62bdf4b22867cc92087b350fd1c2873979 in struts's branch refs/heads/master from Lukasz Lenart [ https://git-wip-us.apache.org/repos/asf?p=struts.git;h=81718c6 ] WW-4827 Injects Container in constructor
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/struts/pull/153
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          PR got merged, thanks!

          Show
          lukaszlenart Lukasz Lenart added a comment - PR got merged, thanks!
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user asfgit closed the pull request at:

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

          Show
          githubbot ASF GitHub Bot added a comment - Github user asfgit closed the pull request at: https://github.com/apache/struts/pull/155
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on the issue:

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

          @aleksandr-m , I modified `ObjectFactory` as below

          ```java
          ...
          private List<Object> myPrivateInjectQueue;
          ...
          @Inject
          public void setContainer(Container container) {
          this.container = container;
          if(null Unable to render embedded object: File (= this.container && null) not found.=myPrivateInjectQueue && myPrivateInjectQueue.size()>0)

          { for(Object obj : myPrivateInjectQueue) this.container.inject(obj); myPrivateInjectQueue.clear(); myPrivateInjectQueue=null; }

          }
          ...
          protected Object injectInternalBeans(Object obj) {
          if (obj != null) {
          if(container != null) container.inject(obj);
          else

          { if(null==myPrivateInjectQueue)myPrivateInjectQueue=new ArrayList<Object>(); myPrivateInjectQueue.add(obj);}

          }
          return obj;
          }

          ```
          in your sample I got following ok result

          ```
          moving setContainer method to the last element of the array
          DateConverter.DateConverter()
          moving setContainer method to the last element of the array
          DateConverter.DateConverter()
          DateConverter.convertToString() localizedTextProvider=com.opensymphony.xwork2.util.StrutsLocalizedTextProvider@3f6f10fa

          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on the issue: https://github.com/apache/struts/pull/153 @aleksandr-m , I modified `ObjectFactory` as below ```java ... private List<Object> myPrivateInjectQueue; ... @Inject public void setContainer(Container container) { this.container = container; if(null Unable to render embedded object: File (= this.container && null) not found. =myPrivateInjectQueue && myPrivateInjectQueue.size()>0) { for(Object obj : myPrivateInjectQueue) this.container.inject(obj); myPrivateInjectQueue.clear(); myPrivateInjectQueue=null; } } ... protected Object injectInternalBeans(Object obj) { if (obj != null) { if(container != null) container.inject(obj); else { if(null==myPrivateInjectQueue)myPrivateInjectQueue=new ArrayList<Object>(); myPrivateInjectQueue.add(obj);} } return obj; } ``` in your sample I got following ok result ``` moving setContainer method to the last element of the array DateConverter.DateConverter() moving setContainer method to the last element of the array DateConverter.DateConverter() DateConverter.convertToString() localizedTextProvider=com.opensymphony.xwork2.util.StrutsLocalizedTextProvider@3f6f10fa ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user aleksandr-m commented on the issue:

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

          @yasserzamani Looks like a hack I would rather leave as it is, then to introduce that. If container is needed then it *should* be injected in the constructor. Take a look at spring object factory.

          Show
          githubbot ASF GitHub Bot added a comment - Github user aleksandr-m commented on the issue: https://github.com/apache/struts/pull/153 @yasserzamani Looks like a hack I would rather leave as it is, then to introduce that. If container is needed then it * should * be injected in the constructor. Take a look at spring object factory.
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on the issue:

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

          Yes I know, just a bit worry about backward compatibility between minor versions.

          I also am studying your sample and @lukaszlenart 's pr to understand what is going on as I think same issues may exists because it seems an object's injections is not an atomic operations! shouldn't be?

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on the issue: https://github.com/apache/struts/pull/153 Yes I know, just a bit worry about backward compatibility between minor versions. I also am studying your sample and @lukaszlenart 's pr to understand what is going on as I think same issues may exists because it seems an object's injections is not an atomic operations! shouldn't be?
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user yasserzamani commented on the issue:

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

          > Thanks a lot I see now what's going on but have no idea why :\

          @lukaszlenart , as I debug, the trace is `object factory creation` then `set1 (inject it's 1st property)` then `set2 (inject it's 2nd property)` ... then ... then `setK (inject it's Knd property)` then `your PostInit.init()` which reaches this object factory with null container ... then `setN (inject it's Nnd property)` finally `setContainer (inject it's container property)`.

          As you see, if you would like to make it works, the `ObjectFactory` itself has to implement `PostInit` and do followings in it's `init()`:

          ```java
          processRequired("struts-default-conversion.properties");
          process("xwork-conversion.properties");
          ```

          Show
          githubbot ASF GitHub Bot added a comment - Github user yasserzamani commented on the issue: https://github.com/apache/struts/pull/153 > Thanks a lot I see now what's going on but have no idea why :\ @lukaszlenart , as I debug, the trace is `object factory creation` then `set1 (inject it's 1st property)` then `set2 (inject it's 2nd property)` ... then ... then `setK (inject it's Knd property)` then `your PostInit.init()` which reaches this object factory with null container ... then `setN (inject it's Nnd property)` finally `setContainer (inject it's container property)`. As you see, if you would like to make it works, the `ObjectFactory` itself has to implement `PostInit` and do followings in it's `init()`: ```java processRequired("struts-default-conversion.properties"); process("xwork-conversion.properties"); ```
          Hide
          githubbot ASF GitHub Bot added a comment -

          Github user lukaszlenart commented on the issue:

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

          The case is that on second container initialisation we do not create singletons upfront ... I have tried to change that but got some problems, not sure why. I must re-think how we initialise Struts internals ...

          Show
          githubbot ASF GitHub Bot added a comment - Github user lukaszlenart commented on the issue: https://github.com/apache/struts/pull/153 The case is that on second container initialisation we do not create singletons upfront ... I have tried to change that but got some problems, not sure why. I must re-think how we initialise Struts internals ...

            People

            • Assignee:
              lukaszlenart Lukasz Lenart
              Reporter:
              aleksandr-m Aleksandr Mashchenko
            • Votes:
              0 Vote for this issue
              Watchers:
              5 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development