AuthMe/ConfigMe

MapProperty: add constructor for empty default, improve documentation

ljacqu opened this issue · 1 comments

ljacqu commented

Only one constructor exists where a default map has to be provided, but I guess in many cases we'd just want to have an empty map? So a constructor should exist to cater to that need.

Additionally, it might be good to promote this property type more, resp. to cover it with more use cases, e.g.:

    @Test
    void testForUseCase_discord() {

        Path configFile = TestUtils.createTemporaryFile(temporaryFolder);

        SettingsManager settingsManager = SettingsManagerBuilder
            .withYamlFile(configFile)
            .configurationData(ServerSettingHolder.class)
            .useDefaultMigrationService()
            .create();

        Map<String, ServerCollection> servers = new HashMap<>();
        servers.put("minigames", new ServerCollection());
        servers.get("minigames").setServers(Arrays.asList("bedwars", "skywars"));

        settingsManager.setProperty(ServerSettingHolder.SERVERS, servers);

        settingsManager.save();

        settingsManager.reload();

        System.out.println(settingsManager.getProperty(ServerSettingHolder.SERVERS));
    }

    public static class ServerSettingHolder implements SettingsHolder {

        public static final Property<Map<String, ServerCollection>> SERVERS =
            new MapProperty<>("", Collections.emptyMap(), BeanPropertyType.of(ServerCollection.class));

    }


    public static class ServerCollection {

        private List<String> servers = new ArrayList<>();


        public List<String> getServers() {
            return servers;
        }

        public void setServers(List<String> servers) {
            this.servers = servers;
        }
    }

To do

  • Add a new constructor MapProperty(@NotNull String path, @NotNull PropertyType<V> type) that has Collections.emptyMap() as default value
  • Based on the snippet above, create a unit test in its own class that covers the case of having a map property at root path (e.g. YamlFileResourceRootMapPropertyTest)
    • Change the snippet above to only use YamlFileResource, not SettingsManager
    • YamlFileResourceEscapingTest is a good example of a similar test with a nice structure

Hello

Just created this PR to address this issue.
I think it meets the criteria but I would love to hear your thoughts on this.

Thank you!