fix(attributes-extension): Correctly skip non-matching compound entries for attribute filters

tweed5/attributes-extension/src/main/java/de/siphalor/tweed5/attributesextension/impl/serde/filter/AttributesFilteredCompoundEntry.java

@@ -0,0 +1,70 @@
+package de.siphalor.tweed5.attributesextension.impl.serde.filter;
+
+import de.siphalor.tweed5.core.api.container.ConfigContainer;
+import de.siphalor.tweed5.core.api.entry.CompoundConfigEntry;
+import de.siphalor.tweed5.core.api.entry.ConfigEntry;
+import de.siphalor.tweed5.core.api.entry.ConfigEntryValueVisitor;
+import de.siphalor.tweed5.core.api.entry.ConfigEntryVisitor;
+import de.siphalor.tweed5.patchwork.api.Patchwork;
+import lombok.RequiredArgsConstructor;
+import org.jspecify.annotations.Nullable;
+
+import java.util.Map;
+
+@RequiredArgsConstructor
+public class AttributesFilteredCompoundEntry<T extends @Nullable Object> implements CompoundConfigEntry<T> {
+	private final CompoundConfigEntry<T> delegate;
+
+	@Override
+	public <V> void set(T compoundValue, String key, V value) {
+		if (value == AttributesReadWriteFilterExtensionImpl.NOOP_MARKER) {
+			return;
+		}
+		delegate.set(compoundValue, key, value);
+	}
+
+	@Override
+	public <V> V get(T compoundValue, String key) {
+		return delegate.get(compoundValue, key);
+	}
+
+	@Override
+	public T instantiateCompoundValue() {
+		return delegate.instantiateCompoundValue();
+	}
+
+	@Override
+	public Map<String, ConfigEntry<?>> subEntries() {
+		return delegate.subEntries();
+	}
+
+	@Override
+	public ConfigContainer<?> container() {
+		return delegate.container();
+	}
+
+	@Override
+	public Class<T> valueClass() {
+		return delegate.valueClass();
+	}
+
+	@Override
+	public Patchwork extensionsData() {
+		return delegate.extensionsData();
+	}
+
+	@Override
+	public void visitInOrder(ConfigEntryValueVisitor visitor, T value) {
+		delegate.visitInOrder(visitor, value);
+	}
+
+	@Override
+	public void visitInOrder(ConfigEntryVisitor visitor) {
+		delegate.visitInOrder(visitor);
+	}
+
+	@Override
+	public T deepCopy(T value) {
+		return delegate.deepCopy(value);
+	}
+}

tweed5/attributes-extension/src/main/java/de/siphalor/tweed5/attributesextension/impl/serde/filter/AttributesReadWriteFilterExtensionImpl.java

@@ -5,6 +5,7 @@ import de.siphalor.tweed5.attributesextension.api.AttributesRelatedExtension;
 import de.siphalor.tweed5.attributesextension.api.serde.filter.AttributesReadWriteFilterExtension;
 import de.siphalor.tweed5.core.api.container.ConfigContainer;
 import de.siphalor.tweed5.core.api.container.ConfigContainerSetupPhase;
+import de.siphalor.tweed5.core.api.entry.CompoundConfigEntry;
 import de.siphalor.tweed5.core.api.entry.ConfigEntry;
 import de.siphalor.tweed5.core.api.entry.ConfigEntryVisitor;
 import de.siphalor.tweed5.core.api.extension.TweedExtensionSetupContext;
@@ -38,6 +39,8 @@ public class AttributesReadWriteFilterExtensionImpl
 	private static final Set<String> MIDDLEWARES_MUST_COME_AFTER = Collections.emptySet();
 	private static final UniqueSymbol TWEED_DATA_NOTHING_VALUE = new UniqueSymbol("nothing (skip value)");
 
+	public static final Object NOOP_MARKER = new Object();
+
 	private final ConfigContainer<?> configContainer;
 	private @Nullable AttributesExtension attributesExtension;
 	private final Set<String> filterableAttributes = new HashSet<>();
@@ -186,12 +189,24 @@ public class AttributesReadWriteFilterExtensionImpl
 						TweedReadContext context
 				) throws TweedEntryReadException {
 					ReadWriteContextCustomData contextData = context.extensionsData().get(readWriteContextDataAccess);
-					if (contextData == null || doFiltersMatch(entry, contextData)) {
+					if (contextData == null) {
-						return innerCasted.read(reader, entry, context);
+						contextData = new ReadWriteContextCustomData();
+						context.extensionsData().set(readWriteContextDataAccess, contextData);
 					}
-					TweedEntryReaderWriterImpls.NOOP_READER_WRITER.read(reader, entry, context);
+					if (!doFiltersMatch(entry, contextData)) {
-					// TODO: this should result in a noop instead of a null value
+						TweedEntryReaderWriterImpls.NOOP_READER_WRITER.read(reader, entry, context);
-					return null;
+						return contextData.noopHandlerInstalled() ? NOOP_MARKER : null;
+					}
+					if (entry instanceof CompoundConfigEntry) {
+						CompoundConfigEntry<Object> delegated =
+								new AttributesFilteredCompoundEntry<>(((CompoundConfigEntry<Object>) entry));
+						boolean oldNoopHandlerInstalled = contextData.noopHandlerInstalled();
+						contextData.noopHandlerInstalled(true);
+						Object value = innerCasted.read(reader, delegated, context);
+						contextData.noopHandlerInstalled(oldNoopHandlerInstalled);
+						return value;
+					}
+					return innerCasted.read(reader, entry, context);
 				}
 			};
 		}
@@ -380,5 +395,6 @@ public class AttributesReadWriteFilterExtensionImpl
 	private static class ReadWriteContextCustomData {
 		private final Map<String, Set<String>> attributeFilters = new HashMap<>();
 		private boolean writerInstalled;
+		private boolean noopHandlerInstalled;
 	}
 }

tweed5/attributes-extension/src/test/java/de/siphalor/tweed5/attributesextension/impl/serde/filter/AttributesReadWriteFilterExtensionImplTest.java

@@ -171,12 +171,12 @@ class AttributesReadWriteFilterExtensionImplTest {
 
 		assertThat(readValue)
 				.containsEntry("first", "1st")
-				.containsEntry("second", null)
+				.doesNotContainKey("second")
 				.hasEntrySatisfying(
 						"nested", nested -> assertThat(nested)
 								.asInstanceOf(map(String.class, Object.class))
 								.containsEntry("first", "n 1st")
-								.containsEntry("second", null)
+								.doesNotContainKey("second")
 				);
 	}
 }

tweed5/attributes-extension/src/test/java/de/siphalor/tweed5/attributesextension/impl/serde/filter/AttributesReadWriteFilterExtensionImplWithPatchInfoTest.java

@@ -0,0 +1,84 @@
+package de.siphalor.tweed5.attributesextension.impl.serde.filter;
+
+import de.siphalor.tweed5.attributesextension.api.serde.filter.AttributesReadWriteFilterExtension;
+import de.siphalor.tweed5.attributesextension.impl.AttributesExtensionImpl;
+import de.siphalor.tweed5.core.api.container.ConfigContainer;
+import de.siphalor.tweed5.core.impl.DefaultConfigContainer;
+import de.siphalor.tweed5.core.impl.entry.SimpleConfigEntryImpl;
+import de.siphalor.tweed5.core.impl.entry.StaticMapCompoundConfigEntryImpl;
+import de.siphalor.tweed5.data.extension.api.ReadWriteExtension;
+import de.siphalor.tweed5.data.hjson.HjsonLexer;
+import de.siphalor.tweed5.data.hjson.HjsonReader;
+import de.siphalor.tweed5.defaultextensions.patch.api.PatchExtension;
+import lombok.SneakyThrows;
+import org.junit.jupiter.api.Test;
+
+import java.io.StringReader;
+import java.util.HashMap;
+import java.util.List;
+import java.util.Map;
+
+import static de.siphalor.tweed5.attributesextension.api.AttributesExtension.attribute;
+import static de.siphalor.tweed5.data.extension.api.ReadWriteExtension.entryReaderWriter;
+import static de.siphalor.tweed5.data.extension.api.readwrite.TweedEntryReaderWriters.compoundReaderWriter;
+import static de.siphalor.tweed5.data.extension.api.readwrite.TweedEntryReaderWriters.stringReaderWriter;
+import static de.siphalor.tweed5.testutils.generic.MapTestUtils.sequencedMap;
+import static java.util.Map.entry;
+import static org.assertj.core.api.Assertions.assertThat;
+
+public class AttributesReadWriteFilterExtensionImplWithPatchInfoTest {
+	@Test
+	@SneakyThrows
+	void testOrder() {
+		ConfigContainer<Map<String, Object>> configContainer = new DefaultConfigContainer<>();
+		configContainer.registerExtension(AttributesReadWriteFilterExtensionImpl.class);
+		configContainer.registerExtension(ReadWriteExtension.class);
+		configContainer.registerExtension(AttributesExtensionImpl.class);
+		configContainer.registerExtension(PatchExtension.class);
+		configContainer.finishExtensionSetup();
+
+		var firstEntry = new SimpleConfigEntryImpl<>(configContainer, String.class)
+				.apply(entryReaderWriter(stringReaderWriter()))
+				.apply(attribute("type", "a"));
+		var secondEntry = new SimpleConfigEntryImpl<>(configContainer, String.class)
+				.apply(entryReaderWriter(stringReaderWriter()));
+		var rootEntry = new StaticMapCompoundConfigEntryImpl<>(
+				configContainer,
+				(Class<Map<String, Object>>) (Class<?>) Map.class,
+				HashMap::new,
+				sequencedMap(List.of(entry("first", firstEntry), entry("second", secondEntry)))
+		)
+				.apply(entryReaderWriter(compoundReaderWriter()));
+
+		configContainer.attachTree(rootEntry);
+
+		var readWriteFilterExtension = configContainer.extension(AttributesReadWriteFilterExtension.class).orElseThrow();
+		readWriteFilterExtension.markAttributeForFiltering("type");
+
+		configContainer.initialize();
+
+		var readWriteExtension = configContainer.extension(ReadWriteExtension.class).orElseThrow();
+		var patchExtension = configContainer.extension(PatchExtension.class).orElseThrow();
+		var filterExtension = configContainer.extension(AttributesReadWriteFilterExtension.class).orElseThrow();
+
+		var readExtData = readWriteExtension.createReadWriteContextExtensionsData();
+		var patchInfo = patchExtension.collectPatchInfo(readExtData);
+		filterExtension.addFilter(readExtData, "type", "a");
+
+		var value = readWriteExtension.read(
+				new HjsonReader(new HjsonLexer(new StringReader("""
+						{
+							"first": "FIRST",
+							"second": "SECOND"
+						}
+						"""))),
+				configContainer.rootEntry(),
+				readExtData
+		);
+
+		assertThat(value).extracting(v -> v.get("first")).isEqualTo("FIRST");
+		assertThat(value).extracting(v -> v.get("second")).isNull();
+		assertThat(patchInfo.containsEntry(firstEntry)).isTrue();
+		assertThat(patchInfo.containsEntry(secondEntry)).isFalse();
+	}
+}