search

OkaeriPoland / okaeri-configs

Simple Java/POJO config library written with love and Lombok
MIT License
83 stars 11 forks source link

Access to the parent Map in the ObjectSerializer #20

Closed Silthus closed 2 years ago

Silthus commented 2 years ago

How can I access the key of a Map<String, Object> when serializing a custom object?

I have a Channel object where I want to map the key of the map to the identifier of the channel. How can I do this?

    @Override
    public boolean supports(final @NonNull Class<? super Channel> type) {
        return Channel.class.isAssignableFrom(type);
    }

    @Override
    public void serialize(final @NonNull Channel object, final @NonNull SerializationData data) {
        data.add("id", object.uuid());
        data.add("identifier", object.identifier()); // <--- set the identifier as the Map.key instead
        data.add("name", object.displayName());
        data.add("format", Objects.requireNonNullElse(object.format(), Channel.DEFAULT_CHANNEL_MESSAGE_FORMAT).getOrNull(Format.MINI_MESSAGE_TEMPLATE));

        for (final Setting<?> setting : object.settings().settings()) {
            data.add(setting.key().asString(), object.getOrNull(setting));
        }
    }

    @Override
    public Channel deserialize(final @NonNull DeserializationData data, final @NonNull GenericsDeclaration generics) {
        final Channel.Builder builder = Channel.channel();

        if (data.containsKey("id"))
            builder.uuid(data.get("id", UUID.class));
        if (data.containsKey("identifier")) // <--- this should come from the Map.key
            builder.identifier(data.get("identifier", String.class));
        if (data.containsKey("name"))
            builder.displayName(data.get("name", Component.class));
        if (data.containsKey("format"))
            builder.format(Format.miniMessageFormat(data.get("format", String.class)));

        for (final Setting<?> setting : Channel.SETTINGS) {
            addSetting(data, builder, setting.key().asString().replace(SChat.NAMESPACE + ":", ""), setting);
        }

        return builder.build();
    }

    private <V> void addSetting(final DeserializationData data, final Channel.Builder builder, final String key, final Setting<V> setting) {
        if (data.containsKey(key))
            builder.setting(setting, data.get(key, setting.type()));
    }
dasavick commented 2 years ago

Could you please provide example OkaeriConfig where such elements are used with current and/or expected result file?

Silthus commented 2 years ago

Could you please provide example OkaeriConfig where such elements are used and current + expected result file?

Wow that was a quick reply :) Sure!

This is the part where I want to use the Map. If it doesn't work I would fall back to a List<Channel>.

    @Comment("Configure your channels here.")
    private Map<String, Channel> channels = new HashMap<>(Map.of(
        "global", Channel.channel()
            .identifier("global")
            .displayName(text("Global"))
            .setting(Channel.PRIVATE, false)
            .setting(Channel.FORCE, true)
            .setting(Channel.AUTO_JOIN, true)
            .setting(Channel.CONSOLE, true)
            .build()
    ));

And this is the section from the config that should be parsed.

channels:
  global: # <-- identifier of the channel
    name: Global
    private: false
    force: true
    auto_join: true
    console: true
    format: channel
  trade:
    name: Trade
    auto_join: true

Also another problem seems to be that the Object serializer does not use any other serializers I registered. I have one for the displayName which is a Component. The pure serialization here works fine:

    @Comment("Fine-tune the behaviour of the console chat in this section.")
    private Console console = new Console();

    /**
     * Configuration for the console.
     *
     * @since next
     */
    @Getter
    @Setter
    @Accessors(fluent = true)
    public static class Console extends OkaeriConfig {

        @Comment("The name players see when chatting from the console.")
        private Component name = text("Console", RED);
        @Comment({
            "The channel the console will send chat messages to.",
            "Is silently ignored if the channel does not exist."
        })
        private String defaultChannel = "global";
    }

This is the serializer for Component:

final class MiniMessageComponentSerializer extends BidirectionalTransformer<String, Component> {

    private final MiniMessage parser = MiniMessage.miniMessage();

    @Override
    public GenericsPair<String, Component> getPair() {
        return new GenericsPair<>(GenericsDeclaration.of(String.class), GenericsDeclaration.of(Component.class));
    }

    @Override
    public Component leftToRight(final @NonNull String data, final @NonNull SerdesContext serdesContext) {
        return parser.deserialize(data);
    }

    @Override
    public String rightToLeft(final @NonNull Component data, final @NonNull SerdesContext serdesContext) {
        return parser.serialize(data);
    }
}

But in the Channel serializer the following error is thrown:

failed to initialize dev.svoxel.chat.config.PluginConfig [class eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer]
eu.okaeri.configs.exception.OkaeriException: failed to initialize dev.svoxel.chat.config.PluginConfig [class eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer]
    at app//eu.okaeri.configs.ConfigManager.create(ConfigManager.java:55)
    at app//dev.svoxel.chat.SChatPlugin.setupAndLoadConfig(SChatPlugin.java:89)
    at app//dev.svoxel.chat.SChatPlugin.onEnable(SChatPlugin.java:82)
    at app//org.bukkit.plugin.java.JavaPlugin.setEnabled(JavaPlugin.java:264)
    at app//org.bukkit.plugin.java.JavaPluginUtils.setEnabled(JavaPluginUtils.java:25)
    at app//be.seeseemelk.mockbukkit.plugin.PluginManagerMock.enablePlugin(PluginManagerMock.java:506)
    at app//be.seeseemelk.mockbukkit.MockBukkit.load(MockBukkit.java:179)
    at app//be.seeseemelk.mockbukkit.MockBukkit.load(MockBukkit.java:163)
    at app//dev.svoxel.chat.BukkitTestBase.setup(BukkitTestBase.java:47)
    at java.base@16.0.2/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base@16.0.2/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
    at java.base@16.0.2/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base@16.0.2/java.lang.reflect.Method.invoke(Method.java:567)
    at app//org.junit.platform.commons.util.ReflectionUtils.invokeMethod(ReflectionUtils.java:725)
    at app//org.junit.jupiter.engine.execution.MethodInvocation.proceed(MethodInvocation.java:60)
    at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain$ValidatingInvocation.proceed(InvocationInterceptorChain.java:131)
    at app//org.junit.jupiter.engine.extension.TimeoutExtension.intercept(TimeoutExtension.java:149)
    at app//org.junit.jupiter.engine.extension.TimeoutExtension.interceptLifecycleMethod(TimeoutExtension.java:126)
    at app//org.junit.jupiter.engine.extension.TimeoutExtension.interceptBeforeEachMethod(TimeoutExtension.java:76)
    at app//org.junit.jupiter.engine.execution.ExecutableInvoker$ReflectiveInterceptorCall.lambda$ofVoidMethod$0(ExecutableInvoker.java:115)
    at app//org.junit.jupiter.engine.execution.ExecutableInvoker.lambda$invoke$0(ExecutableInvoker.java:105)
    at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain$InterceptedInvocation.proceed(InvocationInterceptorChain.java:106)
    at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain.proceed(InvocationInterceptorChain.java:64)
    at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain.chainAndInvoke(InvocationInterceptorChain.java:45)
    at app//org.junit.jupiter.engine.execution.InvocationInterceptorChain.invoke(InvocationInterceptorChain.java:37)
    at app//org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:104)
    at app//org.junit.jupiter.engine.execution.ExecutableInvoker.invoke(ExecutableInvoker.java:98)
    at app//org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.invokeMethodInExtensionContext(ClassBasedTestDescriptor.java:506)
    at app//org.junit.jupiter.engine.descriptor.ClassBasedTestDescriptor.lambda$synthesizeBeforeEachMethodAdapter$21(ClassBasedTestDescriptor.java:491)
    at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeBeforeEachMethods$3(TestMethodTestDescriptor.java:171)
    at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.lambda$invokeBeforeMethodsOrCallbacksUntilExceptionOccurs$6(TestMethodTestDescriptor.java:199)
    at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeBeforeMethodsOrCallbacksUntilExceptionOccurs(TestMethodTestDescriptor.java:199)
    at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.invokeBeforeEachMethods(TestMethodTestDescriptor.java:168)
    at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:131)
    at app//org.junit.jupiter.engine.descriptor.TestMethodTestDescriptor.execute(TestMethodTestDescriptor.java:66)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:151)
    at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at app//org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at java.base@16.0.2/java.util.ArrayList.forEach(ArrayList.java:1511)
    at app//org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
    at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at app//org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at java.base@16.0.2/java.util.ArrayList.forEach(ArrayList.java:1511)
    at app//org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.invokeAll(SameThreadHierarchicalTestExecutorService.java:41)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$6(NodeTestTask.java:155)
    at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$8(NodeTestTask.java:141)
    at app//org.junit.platform.engine.support.hierarchical.Node.around(Node.java:137)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.lambda$executeRecursively$9(NodeTestTask.java:139)
    at app//org.junit.platform.engine.support.hierarchical.ThrowableCollector.execute(ThrowableCollector.java:73)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.executeRecursively(NodeTestTask.java:138)
    at app//org.junit.platform.engine.support.hierarchical.NodeTestTask.execute(NodeTestTask.java:95)
    at app//org.junit.platform.engine.support.hierarchical.SameThreadHierarchicalTestExecutorService.submit(SameThreadHierarchicalTestExecutorService.java:35)
    at app//org.junit.platform.engine.support.hierarchical.HierarchicalTestExecutor.execute(HierarchicalTestExecutor.java:57)
    at app//org.junit.platform.engine.support.hierarchical.HierarchicalTestEngine.execute(HierarchicalTestEngine.java:54)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:108)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:88)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.lambda$execute$0(EngineExecutionOrchestrator.java:54)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.withInterceptedStreams(EngineExecutionOrchestrator.java:67)
    at org.junit.platform.launcher.core.EngineExecutionOrchestrator.execute(EngineExecutionOrchestrator.java:52)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:96)
    at org.junit.platform.launcher.core.DefaultLauncher.execute(DefaultLauncher.java:75)
    at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.processAllTestClasses(JUnitPlatformTestClassProcessor.java:99)
    at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor$CollectAllTestClassesExecutor.access$000(JUnitPlatformTestClassProcessor.java:79)
    at org.gradle.api.internal.tasks.testing.junitplatform.JUnitPlatformTestClassProcessor.stop(JUnitPlatformTestClassProcessor.java:75)
    at org.gradle.api.internal.tasks.testing.SuiteTestClassProcessor.stop(SuiteTestClassProcessor.java:61)
    at java.base@16.0.2/jdk.internal.reflect.NativeMethodAccessorImpl.invoke0(Native Method)
    at java.base@16.0.2/jdk.internal.reflect.NativeMethodAccessorImpl.invoke(NativeMethodAccessorImpl.java:78)
    at java.base@16.0.2/jdk.internal.reflect.DelegatingMethodAccessorImpl.invoke(DelegatingMethodAccessorImpl.java:43)
    at java.base@16.0.2/java.lang.reflect.Method.invoke(Method.java:567)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:36)
    at org.gradle.internal.dispatch.ReflectionDispatch.dispatch(ReflectionDispatch.java:24)
    at org.gradle.internal.dispatch.ContextClassLoaderDispatch.dispatch(ContextClassLoaderDispatch.java:33)
    at org.gradle.internal.dispatch.ProxyDispatchAdapter$DispatchingInvocationHandler.invoke(ProxyDispatchAdapter.java:94)
    at jdk.proxy2/jdk.proxy2.$Proxy5.stop(Unknown Source)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker$3.run(TestWorker.java:193)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker.executeAndMaintainThreadName(TestWorker.java:129)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:100)
    at org.gradle.api.internal.tasks.testing.worker.TestWorker.execute(TestWorker.java:60)
    at org.gradle.process.internal.worker.child.ActionExecutionWorker.execute(ActionExecutionWorker.java:56)
    at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:133)
    at org.gradle.process.internal.worker.child.SystemApplicationClassLoaderWorker.call(SystemApplicationClassLoaderWorker.java:71)
    at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.run(GradleWorkerMain.java:69)
    at app//worker.org.gradle.process.internal.worker.GradleWorkerMain.main(GradleWorkerMain.java:74)
Caused by: eu.okaeri.configs.exception.OkaeriException: failed to #setValue for channels
    at app//eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:298)
    at app//eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:249)
    at app//eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:263)
    at app//eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:235)
    at app//eu.okaeri.configs.OkaeriConfig.load(OkaeriConfig.java:366)
    at app//dev.svoxel.chat.SChatPlugin.lambda$setupAndLoadConfig$0(SChatPlugin.java:93)
    at app//eu.okaeri.configs.ConfigManager.create(ConfigManager.java:52)
    ... 95 more
Caused by: eu.okaeri.configs.exception.OkaeriException: cannot simplify type class net.kyori.adventure.text.TextComponentImpl (null): 'TextComponentImpl{content="Global", style=StyleImpl{color=null, obfuscated=not_set, bold=not_set, strikethrough=not_set, underlined=not_set, italic=not_set, clickEvent=null, hoverEvent=null, insertion=null, font=null}, children=[]}' [class net.kyori.adventure.text.TextComponentImpl]
    at app//eu.okaeri.configs.configurer.Configurer.simplify(Configurer.java:132)
    at app//eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer.simplify(YamlBukkitConfigurer.java:58)
    at app//eu.okaeri.configs.serdes.SerializationData.add(SerializationData.java:33)
    at app//dev.svoxel.chat.config.serializer.ChannelSerializer.serialize(ChannelSerializer.java:53)
    at app//dev.svoxel.chat.config.serializer.ChannelSerializer.serialize(ChannelSerializer.java:36)
    at app//eu.okaeri.configs.configurer.Configurer.simplify(Configurer.java:136)
    at app//eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer.simplify(YamlBukkitConfigurer.java:58)
    at app//eu.okaeri.configs.configurer.Configurer.simplifyMap(Configurer.java:80)
    at app//eu.okaeri.configs.configurer.Configurer.simplify(Configurer.java:129)
    at app//eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer.simplify(YamlBukkitConfigurer.java:58)
    at app//eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer.setValue(YamlBukkitConfigurer.java:75)
    at app//eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:296)
    ... 101 more

Oh and the config object itself is loaded like this:

        this.config = ConfigManager.create(PluginConfig.class, config -> config
            .withConfigurer(new YamlBukkitConfigurer(), new SerdesBukkit(), new SerdesCommons(), new SerdesSChatPack())
            .withBindFile("config.yml")
            .saveDefaults()
            .load(true));
dasavick commented 2 years ago

I see where such access to the parent object would be useful and may be a tempting design decision. However, it seems like this would make such serializer highly dependent on the parent context. Currently, serialized entities are considered as entirely separate, encapsulated map representations. Did you know that you can use whole object (map) as key in YAML?

The collection way is highly preferred instead of dynamic maps, especially in the cases where order matters. This approach encourages writing software that would be more storage agnostic. See: file backends that for some reason do not support map ordering, MySQL's JSON field.

If you still want to go with storing the data in the parent context, I think the best (and still nasty) way to approach this is through extending OkaeriConfig#load in the specific OkaeriConfig. By the way, I'm still open for suggestions of more clean, context-agnostic solutions if something comes to your mind.

@Override
public OkaeriConfig load() throws OkaeriException {
    super.load();
    this.getChannels().forEach((key, value) -> value.setIdentifier(key)); // or directly #set(key, val)
    return this;
}

For the transformer problem, I would ask you to create a new issue. Please include examples or preferably share a small demo project for debugging purposes. It seems like inheritance may be the issue (See: Component interface vs net.kyori.adventure.text.TextComponentImpl). Note, the issue probably does not affect serializers due to different nature of type matching.

Silthus commented 2 years ago

I see where such access to the parent object would be useful and may be a tempting design decision. However, it seems like this would make such serializer highly dependent on the parent context. Currently, serialized entities are considered as entirely separate, encapsulated map representations. Did you know that you can use whole object (map) as key in YAML?

What do you mean by that? I don't really understand. The reason why I want to use a Map<String,Channel> is that the key of the map should be the identifier of the channel which needs to be unique.

dasavick commented 2 years ago

ObjectSerializer is currently not aware of the parent context by design. There is no guarantee, the object would always have a parent of the same type, because object can be in different positions of the structure. The last question about using map as key in YAML reflects this issue: what data would be returned in that case? Key of the section? This makes the API really confusing.

dasavick commented 2 years ago

I feel like uniqueness by property would be great with SerdesContextAttachment mechanism (see: Duration serdes) or other in-house annotation. I will look into that. Seems like good feature to be included in serdes-commons.

Silthus commented 2 years ago

I feel like uniqueness by property would be great with SerdesContextAttachment mechanism (see: Duration serdes) or other in-house annotation. I will look into that. Seems like good feature to be included in serdes-commons.

How about an annotion for maps like KeyFrom("property/getter") that will automatically use the defined property as the map key.

dasavick commented 2 years ago

Sounds like a reasonable solution, but how such map would be defined? I feel like the only possible scenario is that the real (java) type is a list whilst being serialized to map internally in the writing process.

Would be a good extension for TargetType annotation which is already used for applying specific implementation for the map/collection interface. But this one does not allow changing the type completely in such implicit manner.

If map type is exposed in the POJO how is one expected to mutate the map? Adding new entities would allow data inconsistency - e.g. put("one", new MyData("two", 2)).

I would also like to take this opportunity to ask you about how you discovered okaeri-configs project if that is not a problem.

Silthus commented 2 years ago

I would also like to take this opportunity to ask you about how you discovered okaeri-configs project if that is not a problem.

I was looking for a new solution to load my configs via annotations and not the standard bukkit way. I simply googled and found your project. I find it quite nice to use, however I find it a little bit hard to learn with the completly missing javadcos ;)

Would it be possible to write a serializer for a Map.Entry<String,Channel> that would solve the mutation problem. Would this maybe already be possible? This would keep the Key and the Value together in the serialization process.

Mmh no with that I cannot set the key in the data object.

dasavick commented 2 years ago

I find it a little bit hard to learn with the completly missing javadcos ;)

I don't see much need for javadocs besides of the serdes data stuff, which already has pretty detailed documentation. OkaeriConfig as the main object that is accessed is also documented. Do you have any specific classes in mind?

Mmh no with that I cannot set the key in the data object.

I came to the conclusion that the map inside, list on the outside solution would be actually great and the best way to achieve this is through custom type. Furthermore, I'm going to implement IndexedSet in okaeri-commons project and provide serdes-okaeri for that purposes.

Another immediate solution is to create a custom Channels object in your codebase, then you would have full control over the data flow and be able to serialize it to map with full control of the keys. I think this is really simple and powerful at the same time.

Silthus commented 2 years ago

Do you have a some quick mockup example code? I have a hard time to understand the concepts as I am not that deep into the codebase ;)

dasavick commented 2 years ago

This is actually no different from the other serialization/deserialization that you have done before. The Channels would be a standard class with the Map field for data, and the methods to add/get values.

It would be required to create an ObjectSerializer that accesses the map inside the Channels object and passes it to the SerializationData. This is the very same concept as yours flattening of the channel settings.

Here is my current structure for the IndexedSet implementation:

image

dasavick commented 2 years ago

@Silthus IndexedSet support is now available as the beta (4.0.0-beta1) release: https://github.com/OkaeriPoland/okaeri-configs/tree/beta/serdes-okaeri. Please note that upgrading from 3.x to 4.x requires a new parameter in the ObjectSerializer#serialize.

import eu.okaeri.commons.indexedset.IndexedSet;
import eu.okaeri.configs.OkaeriConfig;
import eu.okaeri.configs.annotation.CustomKey;
import eu.okaeri.configs.schema.GenericsDeclaration;
import eu.okaeri.configs.serdes.DeserializationData;
import eu.okaeri.configs.serdes.ObjectSerializer;
import eu.okaeri.configs.serdes.SerializationData;
import eu.okaeri.configs.serdes.okaeri.SerdesOkaeri;
import eu.okaeri.configs.serdes.okaeri.indexedset.IndexedSetSpec;
import eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer;
import lombok.*;
import org.junit.jupiter.api.Test;

import java.util.UUID;

public class TestIndexedSetSerializer {

    @Data
    @NoArgsConstructor
    @AllArgsConstructor
    @EqualsAndHashCode(callSuper = false)
    static class Player extends OkaeriConfig {

        private UUID id;

        @CustomKey("test-name")
        private String name;
    }

    @Data
    @NoArgsConstructor
    @AllArgsConstructor
    static class CustomPlayer {
        private int id;
        private String name;
    }

    @Data
    @NoArgsConstructor
    @EqualsAndHashCode(callSuper = false)
    static class TestConfig extends OkaeriConfig {

        @IndexedSetSpec(key = "test-name")
        private IndexedSet<String, Player> players = IndexedSet.of(Player::getName,
                new Player(UUID.randomUUID(), "Player1"),
                new Player(UUID.randomUUID(), "Player2")
        );

        @IndexedSetSpec(key = "id")
        private IndexedSet<Integer, CustomPlayer> customPlayers = IndexedSet.of(CustomPlayer::getId,
                new CustomPlayer(1, "CPlayer1"),
                new CustomPlayer(2, "CPlayer2")
        );
    }

    static class CustomPlayerSerializer implements ObjectSerializer<CustomPlayer> {

        @Override
        public boolean supports(@NonNull Class<? super CustomPlayer> type) {
            return CustomPlayer.class.isAssignableFrom(type);
        }

        @Override
        public void serialize(@NonNull CustomPlayer object, @NonNull SerializationData data, @NonNull GenericsDeclaration generics) {
            data.add("id", object.getId());
            data.add("name", object.getName());
        }

        @Override
        public CustomPlayer deserialize(@NonNull DeserializationData data, @NonNull GenericsDeclaration generics) {
            int id = data.get("id", int.class);
            String name = data.get("name", String.class);
            return new CustomPlayer(id, name);
        }
    }

    @Test
    public void test_serializer() {

        TestConfig config = new TestConfig();
        config.withConfigurer(new YamlBukkitConfigurer(),
                new SerdesOkaeri(),
                registry -> registry.register(new CustomPlayerSerializer())
        );

        String configString = config.saveToString();
        System.out.println(config);
        System.out.println(configString);

        config.load(configString);
        System.out.println(config);
    }
}

Yields (formatted and added comments for readability):

# object before serialization
TestIndexedSetSerializer.TestConfig(
    players={
        Player1=TestIndexedSetSerializer.Player(id=f7617d8e-69c7-4407-b6e4-b07993bdec1e, name=Player1),
        Player2=TestIndexedSetSerializer.Player(id=597324c8-9e4a-405f-a7b8-03e269725f6b, name=Player2)
    },
    customPlayers={
        1=TestIndexedSetSerializer.CustomPlayer(id=1, name=CPlayer1),
        2=TestIndexedSetSerializer.CustomPlayer(id=2, name=CPlayer2)
    }
)

# bukkit yaml output
players:
  Player1:
    id: f7617d8e-69c7-4407-b6e4-b07993bdec1e
  Player2:
    id: 597324c8-9e4a-405f-a7b8-03e269725f6b
customPlayers:
  '1':
    name: CPlayer1
  '2':
    name: CPlayer1

# object from the yaml
TestIndexedSetSerializer.TestConfig(
    players={
        Player1=TestIndexedSetSerializer.Player(id=f7617d8e-69c7-4407-b6e4-b07993bdec1e, name=Player1),
        Player2=TestIndexedSetSerializer.Player(id=597324c8-9e4a-405f-a7b8-03e269725f6b, name=Player2)
    },
    customPlayers={
        1=TestIndexedSetSerializer.CustomPlayer(id=1, name=CPlayer1),
        2=TestIndexedSetSerializer.CustomPlayer(id=2, name=CPlayer2)
    }
)

Feel free to explore:

Let me know if this resolves your issue.

Silthus commented 2 years ago

Thank you!

I tried it out but get the following exception:

Caused by: eu.okaeri.configs.exception.OkaeriException: failed to #setValue for channels
    at app//eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:298)
    at app//eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:249)
    at app//eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:263)
    at app//eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:235)
    at app//eu.okaeri.configs.OkaeriConfig.saveDefaults(OkaeriConfig.java:156)
    at app//dev.svoxel.chat.SChatPlugin.lambda$loadConfig$0(SChatPlugin.java:190)
    at app//eu.okaeri.configs.ConfigManager.create(ConfigManager.java:52)
    ... 95 more
Caused by: java.lang.ClassCastException: class dev.svoxel.chat.target.ChannelImpl cannot be cast to class java.lang.String (dev.svoxel.chat.target.ChannelImpl is in unnamed module of loader 'app'; java.lang.String is in module java.base of loader 'bootstrap')
    at eu.okaeri.configs.serdes.standard.StringToStringTransformer.transform(StringToStringTransformer.java:8)
    at eu.okaeri.configs.configurer.Configurer.resolveType(Configurer.java:312)
    at eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer.resolveType(YamlBukkitConfigurer.java:70)
    at eu.okaeri.configs.configurer.Configurer.simplify(Configurer.java:121)
    at eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer.simplify(YamlBukkitConfigurer.java:58)
    at eu.okaeri.configs.configurer.Configurer.simplifyCollection(Configurer.java:65)
    at eu.okaeri.configs.configurer.Configurer.simplify(Configurer.java:125)
    at eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer.simplify(YamlBukkitConfigurer.java:58)
    at eu.okaeri.configs.yaml.bukkit.YamlBukkitConfigurer.setValue(YamlBukkitConfigurer.java:75)
    at eu.okaeri.configs.OkaeriConfig.save(OkaeriConfig.java:296)
    ... 101 more

This is my implementation:

    @IndexedSetSpec(key = "alias")
    private IndexedLinkedHashSet<String, Channel> channels = new IndexedLinkedHashSet<>(Channel::alias, new Channel[] {Channel.channel()
            .alias("global")
            .displayName(text("Global"))
            .setting(PRIVATE, false)
            .setting(FORCE, true)
            .setting(AUTO_JOIN, true)
            .setting(CONSOLE, true)
            .build(),
        Channel.channel()
            .alias("team")
            .displayName(text("Team"))
            .setting(PRIVATE, true)
            .setting(FORCE, false)
            .setting(AUTO_JOIN, true)
            .setting(CONSOLE, false)
            .build()
    });

final class ChannelBuilderSerializer implements ObjectSerializer<Channel> {

    private final Channels channels;

    ChannelBuilderSerializer(final @NonNull Channels channels) {
        this.channels = channels;
    }

    @Override
    public boolean supports(final @NonNull Class<? super Channel> type) {
        return Channel.class.isAssignableFrom(type);
    }

    @Override
    public void serialize(final @NonNull Channel object, final @NonNull SerializationData data, final @NonNull GenericsDeclaration generics) {
        data.add("id", object.uuid());
        data.add("alias", object.alias());
        data.add("name", object.displayName(), Component.class);
        data.add("format", object.format(), Format.class);

        for (final Setting<?> setting : channels.settings()) {
            data.add(setting.key().asString().replace(SChat.NAMESPACE + ":", ""), object.getOrNull(setting), setting.type());
        }
    }

    @Override
    public Channel deserialize(final @NonNull DeserializationData data, final @NonNull GenericsDeclaration generics) {
        final Channel.Builder builder = Channel.channel();

        if (data.containsKey("id"))
            builder.uuid(data.get("id", UUID.class));
        if (data.containsKey("alias"))
            builder.alias(data.get("alias", String.class));
        if (data.containsKey("name"))
            builder.displayName(data.get("name", Component.class));
        if (data.containsKey("format"))
            builder.messageFormat(Format.miniMessageFormat(data.get("format", String.class)));

        for (final Setting<?> setting : channels.settings()) {
            addSetting(data, builder, setting.key().asString().replace(SChat.NAMESPACE + ":", ""), setting);
        }

        return builder.build();
    }

    private <V> void addSetting(final DeserializationData data, final Channel.Builder builder, final String key, final Setting<V> setting) {
        if (data.containsKey(key))
            builder.setting(setting, data.get(key, setting.type()));
    }
}
dasavick commented 2 years ago

I think you may have data.add("xyz", channel, String.class) somewhere. This method currently does not allow such type conversion and is intended as a type guide for the current value only (as in #21). However, feel free to open a separate issue with your use case.

At the moment, you may want to use Configurer#resolveType API manually:

data.add("xyz", data.getConfigurer().resolveType(
    channel, GenericsDeclaration.of(Channel.class),
    String.class, GenericsDeclaration.of(String.class),
    SerdesContext.of(data.getConfigurer()))
);

Also, is there any particular reason for using IndexedLinkedHashSet instead of IndexedSet as the field type? I feel like your config would gain from using the included LinkedSet#builder too.

@IndexedSetSpec(key = "alias")
private IndexedSet<String, Channel> channels = IndexedSet.builder(String.class, Channel.class)
    .keyFunction(Channel::alias)
    .add(Channel.channel()
        .alias("global")
        .displayName(text("Global"))
        .setting(PRIVATE, false)
        .setting(FORCE, true)
        .setting(AUTO_JOIN, true)
        .setting(CONSOLE, true)
        .build())
    .add(Channel.channel()
        .alias("team")
        .displayName(text("Team"))
        .setting(PRIVATE, true)
        .setting(FORCE, false)
        .setting(AUTO_JOIN, true)
        .setting(CONSOLE, false)
        .build())
    .build()
Silthus commented 2 years ago

Thank you for the tip with the builder and IndexSet.

However I am not quite sure what you mean with the first think and resolverType?

This is my current Channel Serialization and I dont really know where I would pass a channel somewhere here.

final class ChannelBuilderSerializer implements ObjectSerializer<Channel> {

    private final Channels channels;

    ChannelBuilderSerializer(final @NonNull Channels channels) {
        this.channels = channels;
    }

    @Override
    public boolean supports(final @NonNull Class<? super Channel> type) {
        return Channel.class.isAssignableFrom(type);
    }

    @Override
    public void serialize(final @NonNull Channel object, final @NonNull SerializationData data, final @NonNull GenericsDeclaration generics) {
        data.add("id", object.uuid(), UUID.class);
        data.add("alias", object.alias(), String.class);
        data.add("name", object.displayName(), Component.class);
        data.add("format", object.format(), Format.class);

        for (final Setting<?> setting : channels.settings()) {
            data.add(setting.key().asString().replace(SChat.NAMESPACE + ":", ""), object.getOrNull(setting), setting.type());
        }
    }

    @Override
    public Channel deserialize(final @NonNull DeserializationData data, final @NonNull GenericsDeclaration generics) {
        final Channel.Builder builder = Channel.channel();

        if (data.containsKey("id"))
            builder.uuid(data.get("id", UUID.class));
        if (data.containsKey("alias"))
            builder.alias(data.get("alias", String.class));
        if (data.containsKey("name"))
            builder.displayName(data.get("name", Component.class));
        if (data.containsKey("format"))
            builder.messageFormat(Format.miniMessageFormat(data.get("format", String.class)));

        for (final Setting<?> setting : channels.settings()) {
            addSetting(data, builder, setting.key().asString().replace(SChat.NAMESPACE + ":", ""), setting);
        }

        return builder.build();
    }

    private <V> void addSetting(final DeserializationData data, final Channel.Builder builder, final String key, final Setting<V> setting) {
        if (data.containsKey(key))
            builder.setting(setting, data.get(key, setting.type()));
    }
}
dasavick commented 2 years ago

The core of your current issue is that somewhere the type of the element is assumed incorrectly. I figured the most likely cause would be incorrect use of SerializationData#add. But after a second look, I see that this is simplifyCollection related.

What other serializers/transformers do you have? It seems like you may have a collection of Channel, but somewhere it is defined as it was a String collection. I feel like there is a small chance that Channel-to-String transformer could mess something like that up.

And that got me thinking… IndexedSet<String, Channel> is a Set where the first generic parameter is a String. Seems like I should have made it IndexedSet<Channel, String> by design. Anyway, just making sure: have you registered SerdesOkaeri in your configurer?

The issue seems pretty odd but not unlikely, as I was surprised it actually worked with such custom handling for something that is "just a set" from the beginning.

Silthus commented 2 years ago

Anyway, just making sure: have you registered SerdesOkaeri in your configurer?

That was the problem, now everything works 👍🏻

dasavick commented 2 years ago

By the way, version 4.0.0-beta2 is available with the support for other/custom implementations of IndexedSet:

// recommended
@TargetType(IndexedConcurrentHashSet.class)
private IndexedSet<MyData, K> set = ...
// or without annotation
private IndexedConcurrentHashSet<MyData, K> set = ...

Note that builder and fields of type IndexedSet without @TargetType still use IndexedLinkedHashSet by default. If building set of custom type, it is required to use IndexedSetBuilder#type.

Also note that from beta1 to beta2 generic signature of classes in indexedset package was changed (from <KV, VT> like maps to <VT, KT>) for better compatibility with APIs relying on generic type position like okaeri-configs.

Thanks to the change in the signature, using IndexedSet without SerdesOkaeri now yields a lot more appropriate exception and everything without the core knowing about any of those APIs:

eu.okaeri.configs.exception.OkaeriException: failed to #getValue for players
    ...
1. (with target type) Caused by: eu.okaeri.configs.exception.OkaeriException: failed to create instance of class eu.okaeri.commons.indexedset.IndexedConcurrentHashSet
2. (without target type) Caused by: eu.okaeri.configs.exception.OkaeriException: failed to create instance of interface eu.okaeri.commons.indexedset.IndexedSet
    ...