Log4j 2
  1. Log4j 2
  2. LOG4J2-598

Support more data types in plugin attributes

    Details

    • Type: Improvement Improvement
    • Status: Closed
    • Priority: Minor Minor
    • Resolution: Implemented
    • Affects Version/s: 2.0-rc1
    • Fix Version/s: 2.0-rc2
    • Component/s: Core

      Description

      Currently, annotating plugin factory method parameters with @PluginAttribute only supports String types, but we use a lot of plugin attributes that could be automatically converted to primitive types. Now I'm not suggesting something as advanced as how Camel handles parameter injection (which does automatic non-trivial type conversion), but it would be great to support booleans, ints, longs, etc., to simplify quite a bit of extraneous string parsing.

        Issue Links

          Activity

          Hide
          Matt Sicker added a comment -

          Feature has been added. As of r1597368, this feature has been used and tested on a couple dozen plugins so far. I'm still going through plugins and converting them to use this, though.

          Show
          Matt Sicker added a comment - Feature has been added. As of r1597368, this feature has been used and tested on a couple dozen plugins so far. I'm still going through plugins and converting them to use this, though.
          Hide
          Matt Sicker added a comment -

          Something that would be neat to add later would be some annotations for default values.

          Show
          Matt Sicker added a comment - Something that would be neat to add later would be some annotations for default values.
          Hide
          Matt Sicker added a comment -

          It would be more like a plugin builder. Essentially, using reflection, we could find all the @PluginAttributes, @PluginElements, etc., inside a class. Then we'd instantiate it using the default constructor. We could inject values into fields either via direct reflection or by using the appropriate setFoo methods (whichever was annotated); this is similar to how @Inject works in Java EE. Then we can call an appropriately annotated method to create the object.

          Basically, the main difference here is that the plugin gets instantiated by a builder class instead of by a factory method. It's more object oriented and similar to what a lot of Java developers are familiar with nowadays with Spring's @Autowired annotation, CDI, EJB, etc.

          Show
          Matt Sicker added a comment - It would be more like a plugin builder. Essentially, using reflection, we could find all the @PluginAttributes, @PluginElements, etc., inside a class. Then we'd instantiate it using the default constructor. We could inject values into fields either via direct reflection or by using the appropriate setFoo methods (whichever was annotated); this is similar to how @Inject works in Java EE. Then we can call an appropriately annotated method to create the object. Basically, the main difference here is that the plugin gets instantiated by a builder class instead of by a factory method. It's more object oriented and similar to what a lot of Java developers are familiar with nowadays with Spring's @Autowired annotation, CDI, EJB, etc.
          Hide
          Gary Gregory added a comment -

          Yes, having immutable objects is better IMO.

          Show
          Gary Gregory added a comment - Yes, having immutable objects is better IMO.
          Hide
          Ralph Goers added a comment -

          Injected into what? Today they are all passed as parameters to the factory method. It then creates the actual instance. You can't inject until the object is created and today these are all passed as constructor arguments so they can be declared final.

          Show
          Ralph Goers added a comment - Injected into what? Today they are all passed as parameters to the factory method. It then creates the actual instance. You can't inject until the object is created and today these are all passed as constructor arguments so they can be declared final.
          Hide
          Matt Sicker added a comment -

          Something I think would be neat later on would be more injection support. Instead of a static factory method, allow factory classes where the @PluginFoo annotations can be used on setters, constructors, or fields, and then a factory (or builder) method is executed after all the relevant objects are injected. This would of course take a lot more work than the current method, so this is somewhat a longer term idea.

          Show
          Matt Sicker added a comment - Something I think would be neat later on would be more injection support. Instead of a static factory method, allow factory classes where the @PluginFoo annotations can be used on setters, constructors, or fields, and then a factory (or builder) method is executed after all the relevant objects are injected. This would of course take a lot more work than the current method, so this is somewhat a longer term idea.
          Hide
          Ralph Goers added a comment -

          I would have to look at the plugins we have created. We should do the normal strategy of aiming to cover the top 80% of what is happening in a typical plugin factory as far as parameter validation goes. I could envision that things like min and max values, or a list of valid values might be there but I would take these things one at a time. Starting with just doing the data conversion from Strings in XML or JSON to the correct data type goes a long way towards simplification.

          Show
          Ralph Goers added a comment - I would have to look at the plugins we have created. We should do the normal strategy of aiming to cover the top 80% of what is happening in a typical plugin factory as far as parameter validation goes. I could envision that things like min and max values, or a list of valid values might be there but I would take these things one at a time. Starting with just doing the data conversion from Strings in XML or JSON to the correct data type goes a long way towards simplification.
          Hide
          Matt Sicker added a comment -

          There's absolutely no question as to the awesome work Ralph has done so far! These are things that look like TODO style items that would eventually be addressed whenever someone had the itch.

          I like the @Required attribute idea very much. Are there any other validation annotations that may be useful to implement along with that? Also, what types should that support? I don't think we need to go as overkill as bean validation since we don't have such complex object graphs to validate, but a lightweight version of such could be handy. We could even borrow code from commons validation.

          Show
          Matt Sicker added a comment - There's absolutely no question as to the awesome work Ralph has done so far! These are things that look like TODO style items that would eventually be addressed whenever someone had the itch. I like the @Required attribute idea very much. Are there any other validation annotations that may be useful to implement along with that? Also, what types should that support? I don't think we need to go as overkill as bean validation since we don't have such complex object graphs to validate, but a lightweight version of such could be handy. We could even borrow code from commons validation.
          Hide
          Gary Gregory added a comment -

          YW. It would be nice to piggyback on top of Bean Validation but that's not in the JRE, only in EE.

          Show
          Gary Gregory added a comment - YW. It would be nice to piggyback on top of Bean Validation but that's not in the JRE, only in EE.
          Hide
          Ralph Goers added a comment -

          Thanks Gary. I agree improvements should be made here. In addition I'd like to see an @Required attribute to reduce verification checking.

          Show
          Ralph Goers added a comment - Thanks Gary. I agree improvements should be made here. In addition I'd like to see an @Required attribute to reduce verification checking.
          Hide
          Gary Gregory added a comment -

          Let's start off by saying that Ralph has done a great job writing all of this in the first place.

          Having done some work on some appenders, I do agree that converting Strings to booleans and ints is tiresome, error prone and feels weird.

          Show
          Gary Gregory added a comment - Let's start off by saying that Ralph has done a great job writing all of this in the first place. Having done some work on some appenders, I do agree that converting Strings to booleans and ints is tiresome, error prone and feels weird.

            People

            • Assignee:
              Matt Sicker
              Reporter:
              Matt Sicker
            • Votes:
              0 Vote for this issue
              Watchers:
              3 Start watching this issue

              Dates

              • Created:
                Updated:
                Resolved:

                Development