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

The TextProvider injection in ActionSupport isn't quite integrated into the framework's core DI

    Details

      Description

      The injection of the TextProvider into ActionSupport occurs via a lazy initialization in the getTextProvider() method. This method obtains the TextProvider from a factory that has the implementation injected into it via the core di mechanism. The problem with this is that ActionSupport programmatically does the injection using it's reference to the core ContainerImpl. This makes it impossible to use the Spring plugin's SpringObjectFactory to manage this TextProvider.

        Issue Links

          Activity

          Hide
          lukaszlenart Lukasz Lenart added a comment - - edited

          My solution is like this, you can test it by overriding getTextProvider() in your actions

          protected TextProvider getTextProvider() {
              if (textProvider == null) {
                  ObjectFactory objectFactory = ActionContext.getContext().getInstance(ObjectFactory.class);
                  if (objectFactory == null) {
                      TextProviderFactory tpf = new TextProviderFactory();
                      textProvider = tpf.createInstance(getClass(), this);
                  } else {
                      try {
                          Map<String, Object> context = Collections.emptyMap();
                          textProvider = (TextProvider) objectFactory.buildBean(TextProvider.class, context);
                      } catch (Exception e) {
                          throw new ConfigurationException("Cannot create TextProvider with ObjectFactory!", e);
                      }
                  }
              }
              return textProvider;
          }
          

          The only problem is this will work only with actions, in other places a different instance of TextProvider will be used.

          Show
          lukaszlenart Lukasz Lenart added a comment - - edited My solution is like this, you can test it by overriding getTextProvider() in your actions protected TextProvider getTextProvider() { if (textProvider == null ) { ObjectFactory objectFactory = ActionContext.getContext().getInstance(ObjectFactory.class); if (objectFactory == null ) { TextProviderFactory tpf = new TextProviderFactory(); textProvider = tpf.createInstance(getClass(), this ); } else { try { Map< String , Object > context = Collections.emptyMap(); textProvider = (TextProvider) objectFactory.buildBean(TextProvider.class, context); } catch (Exception e) { throw new ConfigurationException( "Cannot create TextProvider with ObjectFactory!" , e); } } } return textProvider; } The only problem is this will work only with actions, in other places a different instance of TextProvider will be used.
          Hide
          chadmichael chad davis added a comment -

          So, if we do it this way, which looks great to me . . . then how do I configure the DI to inject the current Action. The TextProviderSupport needs contextual stuff from the current action, clazz and localeProvider. Also, my CustomTextProviderSupport use case requires the action, as another type of provider similar to localeProvider ( a "component" provider that provides access to the component from our system related to the current request ). So, wiring these current action bits into the textSupport still eludes me.

          With this proposed fix, I would hope to enable all of this wiring as injections into my CustomTextSupport configured in my DI meta data. But I don't know how it works to reference the "current action" from this pre-runtime situation. Perhaps this is the wrong approach . . . perhaps I should be writing my CustomTextSupport so that it know's how to find it's resources in the ActionContext or ValueStack, similar to how you obtain the ObjectFactory with the getInstance method above. Can I do something like this to obtain the current Action?

          Show
          chadmichael chad davis added a comment - So, if we do it this way, which looks great to me . . . then how do I configure the DI to inject the current Action. The TextProviderSupport needs contextual stuff from the current action, clazz and localeProvider. Also, my CustomTextProviderSupport use case requires the action, as another type of provider similar to localeProvider ( a "component" provider that provides access to the component from our system related to the current request ). So, wiring these current action bits into the textSupport still eludes me. With this proposed fix, I would hope to enable all of this wiring as injections into my CustomTextSupport configured in my DI meta data. But I don't know how it works to reference the "current action" from this pre-runtime situation. Perhaps this is the wrong approach . . . perhaps I should be writing my CustomTextSupport so that it know's how to find it's resources in the ActionContext or ValueStack, similar to how you obtain the ObjectFactory with the getInstance method above. Can I do something like this to obtain the current Action?
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          I'm not sure if I follow you, using ActionContext isn't the best practise as thus tides your code with framework internals and can be hard to test.

          You can use dedicated interceptor (interceptors are created by ObjectFactory as well) and used it to inject your CustomTextProvider onto action. The interceptor has access to request and action.

          Show
          lukaszlenart Lukasz Lenart added a comment - I'm not sure if I follow you, using ActionContext isn't the best practise as thus tides your code with framework internals and can be hard to test. You can use dedicated interceptor (interceptors are created by ObjectFactory as well) and used it to inject your CustomTextProvider onto action. The interceptor has access to request and action.
          Hide
          lukaszlenart Lukasz Lenart added a comment - - edited

          It'll will not work :\ ObjectFactory can create concrete class not interfaces :/ Need to find a better solution.

          Show
          lukaszlenart Lukasz Lenart added a comment - - edited It'll will not work :\ ObjectFactory can create concrete class not interfaces :/ Need to find a better solution.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          I found out a solution - use TextProviderFactory instead directly call to create TextProvider. I'm going extract TextProviderFactory to be an interface with default implementation and then will change all the code to use TextProviderFactory instead of TextProvider.

          Show
          lukaszlenart Lukasz Lenart added a comment - I found out a solution - use TextProviderFactory instead directly call to create TextProvider. I'm going extract TextProviderFactory to be an interface with default implementation and then will change all the code to use TextProviderFactory instead of TextProvider.
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          Need to postpone, it's a huge refactor

          Show
          lukaszlenart Lukasz Lenart added a comment - Need to postpone, it's a huge refactor
          Hide
          rgielen Rene Gielen added a comment -

          Got burned once when I wanted to start refactoring here This is a refactoring we might want to address in Struts NEXT, including possible breaking changes ...

          Show
          rgielen Rene Gielen added a comment - Got burned once when I wanted to start refactoring here This is a refactoring we might want to address in Struts NEXT, including possible breaking changes ...
          Hide
          lukaszlenart Lukasz Lenart added a comment -

          I think this can be marked as resolved but changes introduced in WW-4756

          Show
          lukaszlenart Lukasz Lenart added a comment - I think this can be marked as resolved but changes introduced in WW-4756

            People

            • Assignee:
              lukaszlenart Lukasz Lenart
              Reporter:
              chadmichael chad davis
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development