Tags implement Set using an internal sorted set

fix and improve testcase to test for consistent sorting of tags Signed-off-by: Christoph Rueger <chrisrueger@gmail.com>
bndtools · pkriens · Jun 21, 2024 · May 8, 2024 · May 8, 2024 · May 8, 2024
commit 7857d51a22937b0a338938715dc826721b437154
diff --git a/biz.aQute.bndlib/src/aQute/bnd/service/Tags.java b/biz.aQute.bndlib/src/aQute/bnd/service/Tags.java
@@ -1,25 +1,32 @@
 package aQute.bnd.service;
 
-import static java.util.stream.Collectors.toCollection;
+import static java.util.Collections.unmodifiableSortedSet;
 
 import java.util.Arrays;
 import java.util.Collection;
+import java.util.Iterator;
 import java.util.LinkedHashSet;
 import java.util.Set;
+import java.util.SortedSet;
+import java.util.TreeSet;
+import java.util.stream.Collectors;
 
 /**
  * A set of tags. A tag is a string-token which can be attached to an entity for
  * categorization and filtering. Typically these entities then implement the
  * {@link Tagged} interface.
  */
-public final class Tags extends LinkedHashSet<String> {
-
-	private static final long serialVersionUID = 1L;
+public final class Tags implements Set<String> {
+	private final SortedSet<String>	internalSet;
 
 	public final static Tags	NO_TAGS				= of();
 
-	private Tags(Collection<String> c) {
-		super(c);
+	private Tags() {
+		this.internalSet = unmodifiableSortedSet(new TreeSet<>());
+	}
+
+	private Tags(Collection<? extends String> c) {
+		this.internalSet = unmodifiableSortedSet(new TreeSet<>(c));
 	}
 
 	static Tags of(String... name) {
@@ -40,7 +47,87 @@ public static Tags parse(String csvTags, Tags defaultTags) {
 
 		return new Tags(Arrays.stream(csvTags.split(","))
 			.map(String::trim)
-			.collect(toCollection(LinkedHashSet::new)));
+			.collect(Collectors.toCollection(LinkedHashSet::new)));
+	}
+
+	@Override
+	public int size() {
+		return internalSet.size();
+	}
+
+	@Override
+	public boolean isEmpty() {
+		return internalSet.isEmpty();
+	}
+
+	@Override
+	public boolean contains(Object o) {
+		return internalSet.contains(o);
+	}
+
+	@Override
+	public Iterator<String> iterator() {
+		return internalSet.iterator();
+	}
+
+	@Override
+	public Object[] toArray() {
+		return internalSet.toArray();
+	}
+
+	@Override
+	public <T> T[] toArray(T[] a) {
+		return internalSet.toArray(a);
+	}
+
+	@Override
+	public boolean add(String s) {
+		return internalSet.add(s);
+	}
+
+	@Override
+	public boolean remove(Object o) {
+		return internalSet.remove(o);
+	}
+
+	@Override
+	public boolean containsAll(Collection<?> c) {
+		return internalSet.containsAll(c);
+	}
+
+	@Override
+	public boolean addAll(Collection<? extends String> c) {
+		return internalSet.addAll(c);
+	}
+
+	@Override
+	public boolean retainAll(Collection<?> c) {
+		return internalSet.retainAll(c);
+	}
+
+	@Override
+	public boolean removeAll(Collection<?> c) {
+		return internalSet.removeAll(c);
+	}
+
+	@Override
+	public void clear() {
+		internalSet.clear();
+	}
+
+	@Override
+	public boolean equals(Object o) {
+		return internalSet.equals(o);
+	}
+
+	@Override
+	public int hashCode() {
+		return internalSet.hashCode();
+	}
+
+	@Override
+	public String toString() {
+		return internalSet.toString();
 	}
 
 

diff --git a/biz.aQute.repository/test/aQute/bnd/repository/maven/provider/WorkspaceTest.java b/biz.aQute.repository/test/aQute/bnd/repository/maven/provider/WorkspaceTest.java
@@ -83,17 +83,21 @@ public void testEnv() throws Exception {
 	public void testRepoWithDifferentTag() throws Exception {
 		// similar as testEnv()
 		// but override the tag with a different one and repeat the tests
-		config(Map.of("tags", "foo"));
+		config(Map.of("tags", " foo,bar , a "));
 
 		List<Repository> resolveRepos = workspace.getPlugins(Repository.class, Constants.REPOTAGS_RESOLVE);
 		assertTrue(resolveRepos.isEmpty());
 
 		List<Repository> repos = workspace.getPlugins(Repository.class);
 		Repository repo = repos.get(0);
 		assertTrue(repo instanceof Tagged);
-		assertEquals(1, ((Tagged) repo).getTags()
+		assertEquals(3, ((Tagged) repo).getTags()
 			.size());
-		assertEquals("foo", new ArrayList<>(((Tagged) repo).getTags()).get(0));
+
+		// make sure tags are sorted consistently (alphabetically)
+		assertEquals("a", new ArrayList<>(((Tagged) repo).getTags()).get(0));
+		assertEquals("bar", new ArrayList<>(((Tagged) repo).getTags()).get(1));
+		assertEquals("foo", new ArrayList<>(((Tagged) repo).getTags()).get(2));
 
 	}
 
@@ -116,7 +120,7 @@ void config(Map<String, String> override) throws Exception {
 
 			String tags = config.get("tags");
 			if (tags != null && !tags.isBlank()) {
-				sb.format("  tags=%s\n", tags);
+				sb.format("  tags=\"%s\"\n", tags);
 			}
 
 			build.getParentFile()