Properly handle multiple metadata entries with same key

FabricMC · Sep 17, 2023 · b65b92f · b65b92f
1 parent f92f89c
commit b65b92f
Show file tree

Hide file tree

Showing 14 changed files with 81 additions and 52 deletions.
diff --git a/src/main/java/net/fabricmc/mappingio/FlatMappingVisitor.java b/src/main/java/net/fabricmc/mappingio/FlatMappingVisitor.java
@@ -45,7 +45,7 @@ default boolean visitHeader() throws IOException {
 
 	void visitNamespaces(String srcNamespace, List<String> dstNamespaces) throws IOException;
 
-	default void visitMetadata(String key, String value) throws IOException { }
+	default void visitMetadata(String key, String value, boolean overrideExisting) throws IOException { }
 
 	/**
 	 * Determine whether the mapping content (classes and anything below, metadata if not part of the header) should be visited.

diff --git a/src/main/java/net/fabricmc/mappingio/MappingVisitor.java b/src/main/java/net/fabricmc/mappingio/MappingVisitor.java
@@ -69,7 +69,7 @@ default boolean visitHeader() throws IOException {
 
 	void visitNamespaces(String srcNamespace, List<String> dstNamespaces) throws IOException;
 
-	default void visitMetadata(String key, String value) throws IOException { }
+	default void visitMetadata(String key, String value, boolean overrideExisting) throws IOException { }
 
 	/**
 	 * Determine whether the mapping content (classes and anything below, metadata if not part of the header) should be visited.

diff --git a/src/main/java/net/fabricmc/mappingio/adapter/FlatAsRegularMappingVisitor.java b/src/main/java/net/fabricmc/mappingio/adapter/FlatAsRegularMappingVisitor.java
@@ -65,8 +65,8 @@ public void visitNamespaces(String srcNamespace, List<String> dstNamespaces) thr
 	}
 
 	@Override
-	public void visitMetadata(String key, String value) throws IOException {
-		next.visitMetadata(key, value);
+	public void visitMetadata(String key, String value, boolean overrideExisting) throws IOException {
+		next.visitMetadata(key, value, overrideExisting);
 	}
 
 	@Override

diff --git a/src/main/java/net/fabricmc/mappingio/adapter/ForwardingMappingVisitor.java b/src/main/java/net/fabricmc/mappingio/adapter/ForwardingMappingVisitor.java
@@ -53,8 +53,8 @@ public void visitNamespaces(String srcNamespace, List<String> dstNamespaces) thr
 	}
 
 	@Override
-	public void visitMetadata(String key, String value) throws IOException {
-		next.visitMetadata(key, value);
+	public void visitMetadata(String key, String value, boolean overrideExisting) throws IOException {
+		next.visitMetadata(key, value, overrideExisting);
 	}
 
 	@Override

diff --git a/src/main/java/net/fabricmc/mappingio/adapter/MappingNsCompleter.java b/src/main/java/net/fabricmc/mappingio/adapter/MappingNsCompleter.java
@@ -87,8 +87,8 @@ public void visitNamespaces(String srcNamespace, List<String> dstNamespaces) thr
 	}
 
 	@Override
-	public void visitMetadata(String key, String value) throws IOException {
-		if (relayHeaderOrMetadata) next.visitMetadata(key, value);
+	public void visitMetadata(String key, String value, boolean overrideExisting) throws IOException {
+		if (relayHeaderOrMetadata) next.visitMetadata(key, value, overrideExisting);
 	}
 
 	@Override

diff --git a/src/main/java/net/fabricmc/mappingio/adapter/MappingSourceNsSwitch.java b/src/main/java/net/fabricmc/mappingio/adapter/MappingSourceNsSwitch.java
@@ -127,8 +127,8 @@ public void visitNamespaces(String srcNamespace, List<String> dstNamespaces) thr
 	}
 
 	@Override
-	public void visitMetadata(String key, String value) throws IOException {
-		if (classMapReady && relayHeaderOrMetadata) next.visitMetadata(key, value);
+	public void visitMetadata(String key, String value, boolean overrideExisting) throws IOException {
+		if (classMapReady && relayHeaderOrMetadata) next.visitMetadata(key, value, overrideExisting);
 	}
 
 	@Override

diff --git a/src/main/java/net/fabricmc/mappingio/adapter/RegularAsFlatMappingVisitor.java b/src/main/java/net/fabricmc/mappingio/adapter/RegularAsFlatMappingVisitor.java
@@ -59,8 +59,8 @@ public void visitNamespaces(String srcNamespace, List<String> dstNamespaces) thr
 	}
 
 	@Override
-	public void visitMetadata(String key, String value) throws IOException {
-		next.visitMetadata(key, value);
+	public void visitMetadata(String key, String value, boolean overrideExisting) throws IOException {
+		next.visitMetadata(key, value, overrideExisting);
 	}
 
 	@Override

diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileReader.java
@@ -154,7 +154,7 @@ private static void read(ColumnFileReader reader, MappingVisitor visitor) throws
 							}
 
 							if (property != null) {
-								visitor.visitMetadata(property, parts[1]);
+								visitor.visitMetadata(property, parts[1], true);
 							}
 						}
 					}

diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileWriter.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny1FileWriter.java
@@ -58,7 +58,7 @@ public void visitNamespaces(String srcNamespace, List<String> dstNamespaces) thr
 	}
 
 	@Override
-	public void visitMetadata(String key, String value) throws IOException {
+	public void visitMetadata(String key, String value, boolean overrideExisting) throws IOException {
 		switch (key) {
 		case Tiny1FileReader.nextIntermediaryClassProperty:
 		case Tiny1FileReader.nextIntermediaryFieldProperty:

diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileReader.java
@@ -101,7 +101,7 @@ private static void read(ColumnFileReader reader, MappingVisitor visitor) throws
 							escapeNames = true;
 						}
 
-						visitor.visitMetadata(key, value);
+						visitor.visitMetadata(key, value, true);
 					}
 				}
 			}

diff --git a/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileWriter.java b/src/main/java/net/fabricmc/mappingio/format/tiny/Tiny2FileWriter.java
@@ -59,7 +59,7 @@ public void visitNamespaces(String srcNamespace, List<String> dstNamespaces) thr
 	}
 
 	@Override
-	public void visitMetadata(String key, String value) throws IOException {
+	public void visitMetadata(String key, String value, boolean overrideExisting) throws IOException {
 		if (key.equals(Tiny2Util.escapedNamesProperty)) {
 			escapeNames = true;
 			wroteEscapedNamesProperty = true;

diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingTree.java b/src/main/java/net/fabricmc/mappingio/tree/MappingTree.java
@@ -23,8 +23,8 @@ public interface MappingTree extends MappingTreeView {
 	String setSrcNamespace(String namespace);
 	List<String> setDstNamespaces(List<String> namespaces);
 
-	void addMetadata(String key, String value);
-	String removeMetadata(String key);
+	void addMetadata(String key, String value, boolean overrideExisting);
+	boolean removeMetadata(String key);
 
 	@Override
 	Collection<? extends ClassMapping> getClasses();

diff --git a/src/main/java/net/fabricmc/mappingio/tree/MappingTreeView.java b/src/main/java/net/fabricmc/mappingio/tree/MappingTreeView.java
@@ -58,7 +58,7 @@ default String getNamespaceName(int id) {
 	}
 
 	Collection<Entry<String, String>> getMetadata();
-	String getMetadata(String key);
+	Collection<String> getMetadata(String key);
 
 	Collection<? extends ClassMappingView> getClasses();
 	ClassMappingView getClass(String srcName);

diff --git a/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java b/src/main/java/net/fabricmc/mappingio/tree/MemoryMappingTree.java
@@ -30,6 +30,7 @@
 import java.util.Map;
 import java.util.Objects;
 import java.util.Set;
+import java.util.stream.Collectors;
 
 import net.fabricmc.mappingio.MappedElementKind;
 import net.fabricmc.mappingio.MappingFlag;
@@ -44,21 +45,12 @@ public MemoryMappingTree(boolean indexByDstNames) {
 		this.indexByDstNames = indexByDstNames;
 	}
 
-	public MemoryMappingTree(MappingTree src) {
+	public MemoryMappingTree(MappingTree src) throws IOException {
 		if (src instanceof MemoryMappingTree) {
 			indexByDstNames = ((MemoryMappingTree) src).indexByDstNames;
 		}
 
-		setSrcNamespace(src.getSrcNamespace());
-		setDstNamespaces(src.getDstNamespaces());
-
-		for (Map.Entry<String, String> entry : src.getMetadata()) {
-			addMetadata(entry.getKey(), entry.getValue());
-		}
-
-		for (ClassMapping cls : src.getClasses()) {
-			addClass(cls);
-		}
+		src.accept(this);
 	}
 
 	public void setIndexByDstNames(boolean indexByDstNames) {
@@ -205,37 +197,45 @@ private void updateDstNames(int[] nameMap) {
 	}
 
 	@Override
-	public Collection<Map.Entry<String, String>> getMetadata() {
-		return metadata;
+	public List<Map.Entry<String, String>> getMetadata() {
+		return Collections.unmodifiableList(metadata.stream()
+				.map(entry -> new AbstractMap.SimpleEntry<>(entry.key, entry.value))
+				.collect(Collectors.toList()));
 	}
 
 	@Override
-	public String getMetadata(String key) {
-		for (Map.Entry<String, String> entry : metadata) {
-			if (entry.getKey().equals(key)) return entry.getValue();
-		}
-
-		return null;
+	public List<String> getMetadata(String key) {
+		return Collections.unmodifiableList(metadata.stream()
+				.filter(entry -> entry.key.equals(key))
+				.map(entry -> entry.value)
+				.collect(Collectors.toList()));
 	}
 
 	@Override
-	public void addMetadata(String key, String value) {
-		metadata.add(new AbstractMap.SimpleEntry<>(key, value));
+	public void addMetadata(String key, String value, boolean overrideExisting) {
+		MetadataEntry entry = new MetadataEntry(key, value, overrideExisting);
+
+		if (overrideExisting) {
+			removeMetadata(key);
+		}
+
+		metadata.add(entry);
 	}
 
 	@Override
-	public String removeMetadata(String key) {
-		for (Iterator<Map.Entry<String, String>> it = metadata.iterator(); it.hasNext(); ) {
-			Map.Entry<String, String> entry = it.next();
+	public boolean removeMetadata(String key) {
+		boolean removedAny = false;
 
-			if (entry.getKey().equals(key)) {
-				it.remove();
+		for (Iterator<MetadataEntry> it = metadata.iterator(); it.hasNext(); ) {
+			MetadataEntry entry = it.next();
 
-				return entry.getValue();
+			if (entry.key.equals(key)) {
+				it.remove();
+				removedAny = true;
 			}
 		}
 
-		return null;
+		return removedAny;
 	}
 
 	@Override
@@ -304,8 +304,8 @@ public void accept(MappingVisitor visitor, VisitOrder order) throws IOException
 			if (visitor.visitHeader()) {
 				visitor.visitNamespaces(srcNamespace, dstNamespaces);
 
-				for (Map.Entry<String, String> entry : metadata) {
-					visitor.visitMetadata(entry.getKey(), entry.getValue());
+				for (MetadataEntry entry : metadata) {
+					visitor.visitMetadata(entry.key, entry.value, entry.overrideExisting);
 				}
 			}
 
@@ -390,8 +390,14 @@ public void visitNamespaces(String srcNamespace, List<String> dstNamespaces) {
 	}
 
 	@Override
-	public void visitMetadata(String key, String value) {
-		this.metadata.add(new AbstractMap.SimpleEntry<>(key, value));
+	public void visitMetadata(String key, String value, boolean overrideExisting) {
+		MetadataEntry entry = new MetadataEntry(key, value, overrideExisting);
+
+		if (overrideExisting) {
+			removeMetadata(key);
+		}
+
+		metadata.add(entry);
 	}
 
 	@Override
@@ -1699,6 +1705,29 @@ public String toString() {
 		private final int hash;
 	}
 
+	static final class MetadataEntry {
+		MetadataEntry(String key, String value, boolean enforceUniqueness) {
+			this.key = key;
+			this.value = value;
+			this.overrideExisting = enforceUniqueness;
+		}
+
+		@Override
+		public boolean equals(Object other) {
+			if (!(other instanceof MetadataEntry)) {
+				return false;
+			}
+
+			MetadataEntry entry = (MetadataEntry) other;
+
+			return this.key.equals(entry.key) && this.value.equals(entry.value);
+		}
+
+		final String key;
+		final String value;
+		final boolean overrideExisting;
+	}
+
 	static final class GlobalMemberKey {
 		GlobalMemberKey(ClassEntry owner, String name, String desc, boolean isField) {
 			this.owner = owner;
@@ -1739,7 +1768,7 @@ public String toString() {
 	private boolean indexByDstNames;
 	private String srcNamespace;
 	private List<String> dstNamespaces = Collections.emptyList();
-	private final List<Map.Entry<String, String>> metadata = new ArrayList<>();
+	private final List<MetadataEntry> metadata = new ArrayList<>();
 	private final Map<String, ClassEntry> classesBySrcName = new LinkedHashMap<>();
 	private Map<String, ClassEntry>[] classesByDstNames;