Config value POST load handler support

[zvyap]

In Minecraft configuration, there's always a field for "display message" or "display name", and most of the time, it supports colors.

Problem I met

class MyConfig {
     String testMessage;
}

testMessage: "&aHi, or even §cHi, maybe <red>Hi?</red>" # The last one is MiniMessage support by PaperMC btw

While every time I want to use it, I need to

MyUtilsClass.translateColor(MyConfig.testMessage);

Which is frustrating to do. Although you can manually or loop through all the fields and translate them once your plugin starts, it's still frustrating to do so when I'm using a config library that aims to make my code easier and simpler.

If a additional POST load handler is support using annotation

class MyConfig {
     @ImMessage
     String testMessage;
}

In somewhere else

class FieldPostLoadHandler extends XXXXXX<String> {
       public String postProcess(String value) {
              MyUtilsClass.translateColor(MyConfig.testMessage);
       }
}

And register the handler to ConfigLib configLib.registerPostLoadHandler(FieldFilter.matchAnnotation(ImMessage.class), new FieldPostLoadHandler()); //Just demo, idk where to put this registration

I know that we can use custom deserializer make to job done, but why I need to @SerializeWith and create a class for String (in this case) when I just need to post process the value. If I need multiple post process at the same time, Ex: @SerializeWith - StringWithColorSerializer or StringWithColorAndFormatSerializer or StringWithFormatSerializer Instead of Ex: @ColoredString, @FormattedString

What I state here is just an example, Im using Adventure API TextComponent as my colored string object, but you get my point about why POST load handler is needed.

[Exlll]

Hey, thanks also for your second suggestion. :slightly_smiling_face:

My first approach to this would be to make the following three additions to this library:

First, some annotation that is used to filter fields:

public @interface PostProcess {
    String key();
}

Second, a new method added to the ConfigurationProperties.Builder class that is used to add postprocessors:

public final <T> B addPostProcessor(
    FieldFilter filter,
    Function<T, T> processor
)

Third, a new static factory to create FieldFilter objects:

public static FieldFilter filterByPostProcessAnnotation(String key) {
    return field -> {
        PostProcess annotation = field.getAnnotation(PostProcess.class);
        return (annotation != null) && (annotation.key().equals(key));
    };
}

With these three additions you could then write

@Configuration
class MyConfig {
    @PostProcess(key = "my_mini_message")
    String message1 = "...";
    @PostProcess(key = "my_mini_message")
    String message2 = "...";
}

ConfigurationProperties properties = ConfigurationProperties.newBuilder()
        .addFieldPostProcessor(
                filterByPostProcessAnnotation("my_mini_message"),
                MyUtilsClass::translateColor
        )
        .build();    

What do you think about this implementation? Does it meet your requirements?

[zvyap]

Sound great!

[Exlll]

I was thinking a little bit more about your request and I wonder whether this additional complexity in the API is really necessary.

Instead of using annotations and adding postprocessors, how about you simply define a method in your configuration class and call that method once your configuration has been loaded:

@Configuration
class MyConfig {
    String message1 = "...";
    String message2 = "...";

    public void postProcess() {
        this.message1 = MyUtilsClass.translateColor(this.message1);
        this.message2 = MyUtilsClass.translateColor(this.message2);
    }
}

MyConfig config = // load config ...
config.postProcess();
// ... use config

In my opinion, this approach is (a) simpler because you don't have to learn an additional API but can simply stick to normal Java code and (b) much more flexible because you don't have to define a new processor for each distinct field type or transformation that you want to apply.

All in all, I don't think this API is necessary, but maybe I'm missing something?

[zvyap]

In my opinion, this approach is (a) simpler because you don't have to learn an additional API but can simply stick to normal Java code and (b) much more flexible because you don't have to define a new processor for each distinct field type or transformation that you want to apply.

All in all, I don't think this API is necessary, but maybe I'm missing something?

I agree with your point that this approach has made the library easier to use and requires a less steep learning curve. However:

a) It ultimately creates difficulties for users of the library because they need to manually write the postProcess method where necessary.

b) Consider a configuration file like customNPC.yml that contains many nested classes. Users will be required to invoke the postProcess method of each subclass within the postProcess method of the main class. This is also a pain point that I have encountered.

c) As for data class complexity - as someone who appreciates the fundamentals of OOP, I prefer to separate data classes and actual code logic into different instances. Adding a postProcess method introduces additional complexity to the class. While I am comfortable with including data processing within the data class, it does make the class more complex, especially when dealing with nested objects.

d) Reducing code errors - manually calling a method after the file has been parsed into an object is an unnecessary line of code and can easily be overlooked. Although you can use a wrapper instance to call the method after the library has parsed the file, the question remains: why should this be necessary?

e) The most important issue is the FINAL modifier. What if the field is final? Methods are not allowed to rewrite final fields.

This is the problem whether the library offers a better solution for the user or requires the user to resolve issues with the library itself. Accually Im pretty okay at opening a PR contributing this library, but regarding to adding a new API / structure change, I still think you are the best to add/decides as you created this library.

[Exlll]

It ultimately creates difficulties for users of the library because they need to manually write the postProcess method where necessary.

I don't really see that as a problem. To postprocess your fields you'll have to write at least some code. With the new approach, instead of having to write a single postProcess method you'd have to write one or multiple postprocessor functions and annotate each and every field to which you want to apply these functions.

The most important issue is the FINAL modifier. What if the field is final? Methods are not allowed to rewrite final fields.

That's not an issue because this library ignores final fields as they are, as their name says, final. So there would be no difference to the current behavior in this case.

Reducing code errors - manually calling a method after the file has been parsed into an object is an unnecessary line of code and can easily be overlooked.

I don't really agree with this. Unless you don't test your code at all, you should immediately spot that the values are not correct and then add the missing call. In your example above, you'd see that the colors are all messed up.

Consider a configuration file like customNPC.yml that contains many nested classes.

I agree that postprocessing can be cumbersome if you are working with nested classes. I see two alternative approaches that should solve this issue:

The first is to introduce a PostProcess annotation that can be applied to a method in your configuration class. If such an annotated method exists, it is called after an instance of that class has been initialized. In the following example, s would be converted to uppercase and i would be multiplied by 10 when initializing MainConfig:

@Configuration
static final class MainConfig {
    private String s = "a";
    private NestedConfig nested = new NestedConfig();

    @PostProcess
    private void postProcess() {
        s = s.toUpperCase();
    }
}

@Configuration
static final class NestedConfig {
    private int i = 10;

    @PostProcess
    private void postProcess() {
        i = i * 10;
    }
}

The second approach is to introduce a PostProcess annotation that can be applied at the type level (i.e. to some configuration class) and that requires one to provide another class that resembles some kind of processor (for example, a class implements the Consumer interface):

@Configuration
@PostProcess(MainConfigPostProcessor.class)
static final class MainConfig {
    private String s = "a";
    private NestedConfig nested = new NestedConfig();
}

@Configuration
@PostProcess(NestedConfigPostProcessor.class)
static final class NestedConfig {
    private int i = 10;
}

static final class MainConfigPostProcessor implements Consumer<MainConfig> {
    @Override
    public void accept(MainConfig mainConfig) {
        // requires fields to be accessible or, alternatively, setters
        mainConfig.s = mainConfig.s.toUpperCase();
    }
}

static final class NestedConfigPostProcessor implements Consumer<NestedConfig> {
    @Override
    public void accept(NestedConfig nestedConfig) {
        nestedConfig.i = nestedConfig.i * 10;
    }
}

Alternatively, instead of using the PostProcess annotation, defining postprocessors for types could also be possible through a ConfigurationProperties object:

ConfigurationProperties properties = ConfigurationProperties.newBuilder()
        .addPostProcessor(MainConfig.class, new MainConfigPostProcessor())
        .addPostProcessor(NestedConfig.class, new NestedConfigPostProcessor())
        .build().

The first of these three approaches looks much simpler to me which is also why I prefer it. The second and third approach take into consideration your argument (c) which I agree is also a valid point (although you'd probably have to write setters to not break encapsulation).

[zvyap]

That's not an issue because this library ignores final fields as they are, as their name says, final. So there would be no difference to the current behavior in this case.

But postProcess method dont. It cant rewrite the field

   final String message; //Library load well

   public void postProcess() {
        message = MyUtilsClass.color(message); //Compiler error :(
   }

I don't really agree with this. Unless you don't test your code at all, you should immediately spot that the values are not correct and then add the missing call. In your example above, you'd see that the colors are all messed up.

It just increase the rate you misslook the code. Easier to see whether the field got post process than write the code down in a method.

In your solution, the final field cannot be rewritten in the method we use @PostProcess. The solution I hope for is to put the @PostProcess annotation on the field, and the library will go through the post processors first before setting the value into the field. As you said, this library ignores final, which solves the problem of "final field cannot be rewritten".

For the third solution (alternative solution) you provided, I still hope there is a matcher(with interface) to allow user to do their own matching or use default provided matcher. User may need to match field type, annotation matcher, field name matcher if post process run with field. Same as class if you choose to use your solution.

Regarding your solution, there is a fact that while users are doing the same post-process across their config:

a) the problem I mentioned above (final cannot be rewritten)

b) Sometimes users are doing the same thing across their config, for example, translating colors:

class ConfigA {
     String message;
}

class ConfigB {
    String message;
}

Users will be required to code the same thing twice:

class ConfigA {
     String message;

     @PostProcess
     void post() {
          message = XXXXXXXX;
     }
}

class ConfigB {
    String message;

     @PostProcess
     void post() {
          message = XXXXXXXX;
     }
}

It ends up with a lot of copying and pasting code (and modifying the variable name themselves), which makes no sense compared to the first solution I suggested above.

Actually, pre-processors also have their own use case, but I just requested a post processor (which I need this a lot). Different people had their own usage, this is why those famous libraries (e.g, Jackson - Converter class) exist this feature.

But! Your solution is still acceptable. I will just create a global PostProcessor consumer class and implement 'suggestions in my first comment' (using annotation per field), but instead of making it easier for me to use this library, your solution just gave me availability to use my own code to use this library easier (instead of forking). I respect your choice.

[Exlll]

When I said that the library ignores final fields I didn't mean that it ignores the final modifier. If a field is final it is neither written when saving a configuration instance nor set/updated when loading a configuration. This is documented in the README under the Ignoring and filtering fields section.

In any case, I see that there can be value in custom matchers. Perhaps offering both solutions is the way to go then.

[zvyap]

When I said that the library ignores final fields I didn't mean that it ignores the final modifier. If a field is final it is neither written when saving a configuration instance nor set/updated when loading a configuration. This is documented in the README under the Ignoring and filtering fields section.

Oh! Sorry for my miss understanding.

In any case, I see that there can be value in custom matchers. Perhaps offering both solutions is the way to go then.

Sure! That will be nice!

[Exlll]

Hey, would you be interested in giving the new functionality a try before I finish implementing it? If yes, you need to build this library yourself from the feature/postprocessors branch.

What I did is I implemented both our suggestions: You can use either way to post-process a class (or record) or both at the same time, like in the following example:

@Configuration
class MyConfig {
    @PostProcess(key = "my_mini_message")
    String message1 = "...";
    @PostProcess(key = "my_mini_message")
    String message2 = "...";

    // method is executed AFTER post-processors from annotation
    @PostProcess
    private void postProcess() { // method name can be anything
        message2 = message2.repeat(3);
        // ...     
    }
}

ConfigurationProperties.newBuilder()
        .addPostProcessor(
                ConfigurationElementFilter.byPostProcessKey("my_mini_message"),
                (String input) -> input.repeat(2) // or what ever post-processor you want
        )
        .build();

I'm not done testing everything but I'm already confident that there won't be any API changes. However, I still have a few TODOs open that will cause some expections to be thrown earlier (in order to catch misconfiguration from developers).

I hope that I'll find time to finish this request this weekend, although I can't promise anything since I'm quite busy with my work these days.

[zvyap]

Hey, would you be interested in giving the new functionality a try before I finish implementing it? If yes, you need to build this library yourself from the feature/postprocessors branch.

What I did is I implemented both our suggestions: You can use either way to post-process a class (or record) or both at the same time, like in the following example:

@Configuration
class MyConfig {
    @PostProcess(key = "my_mini_message")
    String message1 = "...";
    @PostProcess(key = "my_mini_message")
    String message2 = "...";

    // method is executed AFTER post-processors from annotation
    @PostProcess
    private void postProcess() { // method name can be anything
        message2 = message2.repeat(3);
        // ...     
    }
}

ConfigurationProperties.newBuilder()
        .addPostProcessor(
                ConfigurationElementFilter.byPostProcessKey("my_mini_message"),
                (String input) -> input.repeat(2) // or what ever post-processor you want
        )
        .build();

I'm not done testing everything but I'm already confident that there won't be any API changes. However, I still have a few TODOs open that will cause some expections to be thrown earlier (in order to catch misconfiguration from developers).

I hope that I'll find time to finish this request this weekend, although I can't promise anything since I'm quite busy with my work these days.

Sure, I also busy in these day, I will give a try for the new feature when I free

[Exlll]

Hey, have you had time to have a look? I also now (more or less) finished the documentation, if you want to have a look: Post-processing

[Exlll]

Now available in version 4.5.0.