diff --git a/gradle.properties b/gradle.properties index cceb45b..270db81 100644 --- a/gradle.properties +++ b/gradle.properties @@ -1,3 +1,3 @@ major = 0 -minor = 23 +minor = 24 patch = 0 \ No newline at end of file diff --git a/src/main/java/io/github/fablabsmc/fablabs/api/fiber/v1/builder/ConfigLeafBuilder.java b/src/main/java/io/github/fablabsmc/fablabs/api/fiber/v1/builder/ConfigLeafBuilder.java index 027065e..30735db 100644 --- a/src/main/java/io/github/fablabsmc/fablabs/api/fiber/v1/builder/ConfigLeafBuilder.java +++ b/src/main/java/io/github/fablabsmc/fablabs/api/fiber/v1/builder/ConfigLeafBuilder.java @@ -189,9 +189,6 @@ public ConfigLeaf build() { built.getAttributes().putAll(this.attributes); if (parent != null) { - // We don't know what kind of evil collection we're about to add a node to. - // Though, we don't really want to throw an exception on this method because no developer likes try-catching every setting they build. - // Let's tread with caution. try { parent.getItems().add(built); } catch (RuntimeFiberException e) { diff --git a/src/main/java/io/github/fablabsmc/fablabs/impl/fiber/annotation/AnnotatedSettingsImpl.java b/src/main/java/io/github/fablabsmc/fablabs/impl/fiber/annotation/AnnotatedSettingsImpl.java index 0206986..a4b72b4 100644 --- a/src/main/java/io/github/fablabsmc/fablabs/impl/fiber/annotation/AnnotatedSettingsImpl.java +++ b/src/main/java/io/github/fablabsmc/fablabs/impl/fiber/annotation/AnnotatedSettingsImpl.java @@ -46,6 +46,7 @@ import io.github.fablabsmc.fablabs.api.fiber.v1.schema.type.derived.ConfigType; import io.github.fablabsmc.fablabs.api.fiber.v1.schema.type.derived.ConfigTypes; import io.github.fablabsmc.fablabs.api.fiber.v1.tree.ConfigBranch; +import io.github.fablabsmc.fablabs.api.fiber.v1.tree.ConfigLeaf; import io.github.fablabsmc.fablabs.api.fiber.v1.tree.ConfigTree; import io.github.fablabsmc.fablabs.impl.fiber.annotation.magic.TypeMagic; @@ -157,12 +158,16 @@ public void processSetting(Object pojo, Field setting) throws ProcessingMemberEx private void processSetting(Object pojo, Field setting, ConfigType type) throws FiberException { String name = this.findName(setting); List listeners = this.listenerMap.getOrDefault(name, Collections.emptyList()); - ConfigLeafBuilder leaf = this.builder + setting.setAccessible(true); + ConfigLeafBuilder leafBuilder = this.builder .beginValue(name, type, this.findDefaultValue(pojo, setting)) .withComment(this.findComment(setting)) - .withListener(this.constructListener(pojo, setting, listeners, type)); - this.applyAnnotationProcessors(pojo, setting, leaf, AnnotatedSettingsImpl.this.valueSettingProcessors); - leaf.build(); + .withListener(this.constructListener(pojo, listeners, type)); + this.applyAnnotationProcessors(pojo, setting, leafBuilder, AnnotatedSettingsImpl.this.valueSettingProcessors); + ConfigLeaf leaf = leafBuilder.build(); + builder.getItems().remove(leaf); + BackedConfigLeaf deferred = new BackedConfigLeaf<>(leaf, type, pojo, setting); + builder.getItems().add(deferred); // This will also attach deferred } @Nonnull @@ -180,19 +185,23 @@ private String findComment(Field field) { } @Nonnull - private BiConsumer constructListener(Object pojo, Field setting, List listeners, ConfigType type) throws FiberException { - BiConsumer ret = (t, newValue) -> { - try { - setting.setAccessible(true); - setting.set(pojo, newValue); - } catch (IllegalAccessException e) { - throw new RuntimeFiberException("Failed to update field value", e); - } - }; + private BiConsumer constructListener(Object pojo, List listeners, ConfigType type) throws FiberException { + BiConsumer ret = null; for (Member listener : listeners) { BiConsumer consumer = this.constructListenerFromMember(pojo, listener, type.getRuntimeType()); - if (consumer != null) ret = ret.andThen(consumer); + + if (consumer != null) { + if (ret == null) { + ret = consumer; + } else { + ret = ret.andThen(consumer); + } + } + } + + if (ret == null) { + ret = (r, r2) -> { }; } return ret; @@ -306,8 +315,6 @@ private void applyAnnotationProcessors(Object pojo, Field field, C sub, Map< @SuppressWarnings("unchecked") private T findDefaultValue(Object pojo, Field field) throws FiberException { - boolean accessible = field.isAccessible(); - field.setAccessible(true); T value; try { @@ -320,7 +327,6 @@ private T findDefaultValue(Object pojo, Field field) throws FiberException { throw new FiberException("Couldn't get value for field '" + field.getName() + "'", e); } - field.setAccessible(accessible); return value; } diff --git a/src/main/java/io/github/fablabsmc/fablabs/impl/fiber/annotation/BackedConfigLeaf.java b/src/main/java/io/github/fablabsmc/fablabs/impl/fiber/annotation/BackedConfigLeaf.java new file mode 100644 index 0000000..0358e28 --- /dev/null +++ b/src/main/java/io/github/fablabsmc/fablabs/impl/fiber/annotation/BackedConfigLeaf.java @@ -0,0 +1,173 @@ +package io.github.fablabsmc.fablabs.impl.fiber.annotation; + +import java.lang.reflect.Field; +import java.util.Map; +import java.util.Objects; +import java.util.Optional; +import java.util.function.BiConsumer; + +import javax.annotation.Nonnull; +import javax.annotation.Nullable; + +import io.github.fablabsmc.fablabs.api.fiber.v1.FiberId; +import io.github.fablabsmc.fablabs.api.fiber.v1.exception.IllegalTreeStateException; +import io.github.fablabsmc.fablabs.api.fiber.v1.exception.RuntimeFiberException; +import io.github.fablabsmc.fablabs.api.fiber.v1.schema.type.SerializableType; +import io.github.fablabsmc.fablabs.api.fiber.v1.schema.type.derived.ConfigType; +import io.github.fablabsmc.fablabs.api.fiber.v1.tree.ConfigAttribute; +import io.github.fablabsmc.fablabs.api.fiber.v1.tree.ConfigBranch; +import io.github.fablabsmc.fablabs.api.fiber.v1.tree.ConfigLeaf; + +/** + * A config leaf backed by a field, {@linkplain ConfigType}, and deferred leaf. + * + *

It is used to fetch the backing field's values on each {@link #getValue()} call, to make sure a leaf and its corresponding POJO field are always synchronised. + * + * @param + * @param + */ +public class BackedConfigLeaf implements ConfigLeaf { + private final ConfigLeaf backing; + private final ConfigType type; + private final Object pojo; + private final Field backingField; + private R cachedValue = null; + private ConfigBranch parent; + + public BackedConfigLeaf(ConfigLeaf backing, ConfigType type, Object pojo, Field backingField) { + if (!backingField.isAccessible()) throw new RuntimeFiberException("A BackedConfigLeaf may only be made for an accessible field!"); + + this.backing = backing; + this.type = type; + this.pojo = pojo; + this.backingField = backingField; + } + + @Override + public String getName() { + return backing.getName(); + } + + @Override + public Map> getAttributes() { + return backing.getAttributes(); + } + + @Override + public Optional getAttributeValue(FiberId id, ConfigType type) { + return backing.getAttributeValue(id, type); + } + + @Override + public Optional getAttributeValue(FiberId id, SerializableType expectedType) { + return backing.getAttributeValue(id, expectedType); + } + + @Override + public ConfigAttribute getOrCreateAttribute(FiberId id, SerializableType attributeType, @Nullable A defaultValue) { + return backing.getOrCreateAttribute(id, attributeType, defaultValue); + } + + @Nullable + @Override + public ConfigBranch getParent() { + return parent; + } + + @Override + public void attachTo(ConfigBranch parent) throws IllegalTreeStateException { + // Attaching/detaching requires careful inspection of whether or not the parent's items contains `this`. + // In the case of deferred leaves, the leaf in the parent's items is the deferred leaf and not the backing leaf. + // Thus, we can't defer attaching/detaching to the backing leaf, as it will think it's not part of the parent's items. + if (this.parent != null && this.parent != parent) { + throw new IllegalTreeStateException(this + " needs to be detached before changing the parent"); + } + + // this node may have already been added by the collection + if (parent != null && !parent.getItems().contains(this)) { + parent.getItems().add(this); + } + + this.parent = parent; + } + + @Override + public void detach() { + // Note: infinite recursion between ConfigNode#detach() and NodeCollection#remove() could occur here, + // but the latter performs the actual collection removal before detaching + if (this.parent != null) { + // here, we also need to avoid triggering a ConcurrentModificationException + // thankfully, remove does not cause a CME if it's a no-op + this.parent.getItems().remove(this); + } + + this.parent = null; + } + + @Override + public boolean setValue(@Nonnull S value) { + if (this.backing.setValue(value)) { + try { + value = backing.getValue(); // Might've changed after a type check + correction, so we fetch again + this.backingField.set(pojo, type.toRuntimeType(value)); + } catch (IllegalAccessException e) { + throw new RuntimeFiberException("Failed to update field value", e); + } + + return true; + } + + return false; + } + + @Nonnull + @Override + @SuppressWarnings("unchecked") + public S getValue() { + try { + R fieldValue = (R) backingField.get(pojo); + + if (!Objects.equals(fieldValue, cachedValue)) { + this.backing.setValue(type.toSerializedType(fieldValue)); + cachedValue = fieldValue; + } + } catch (IllegalAccessException e) { + // Because this exception might appear to happen 'at random' to the user, we wrap it to at least provide more information about what just happened + throw new RuntimeFiberException("Couldn't fetch setting value from POJO", e); + } + + return backing.getValue(); + } + + @Override + public SerializableType getConfigType() { + return backing.getConfigType(); + } + + @Nonnull + @Override + public BiConsumer getListener() { + return backing.getListener(); + } + + @Override + public void addChangeListener(BiConsumer listener) { + backing.addChangeListener(listener); + } + + @Nullable + @Override + public S getDefaultValue() { + return backing.getDefaultValue(); + } + + @Override + public String getComment() { + return backing.getComment(); + } + + @Override + public boolean accepts(@Nonnull S rawValue) { + return backing.accepts(rawValue); + } +} diff --git a/src/test/java/io/github/fablabsmc/fablabs/api/fiber/v1/annotation/AnnotatedSettingsTest.java b/src/test/java/io/github/fablabsmc/fablabs/api/fiber/v1/annotation/AnnotatedSettingsTest.java index b28bead..9bd23b0 100644 --- a/src/test/java/io/github/fablabsmc/fablabs/api/fiber/v1/annotation/AnnotatedSettingsTest.java +++ b/src/test/java/io/github/fablabsmc/fablabs/api/fiber/v1/annotation/AnnotatedSettingsTest.java @@ -281,6 +281,16 @@ void testSuperPojo() { assertEquals(2, this.node.getItems().size(), "Node has two items"); } + @Test + @DisplayName("Directly mutated POJO") + void lateChangePojo() throws FiberException { + LateChangePojo pojo = new LateChangePojo(); + this.annotatedSettings.applyToNode(this.node, pojo); + ConfigLeaf a = this.node.lookupLeaf("a", ConfigTypes.INTEGER.getSerializedType()); + pojo.a = 10; + assertEquals(10, a.getValue().intValue()); + } + private static class FinalSettingPojo { private final int a = 5; } @@ -422,4 +432,8 @@ private static class SuperPojo { private static class ExtendingPojo extends SuperPojo { private int b = 5; } + + private static class LateChangePojo { + private int a = 5; + } }