Merge pull request #1307 from petebankhead/0.4.4-fixes

Potential v0.4.4 release

CHANGELOG.md

@@ -1,3 +1,13 @@
+## Version 0.4.4
+
+This is a *minor release* that aims to be fully compatible with previous v0.4.x releases, while fixing one bug.
+
+> A bigger update (v0.5.0) is planned for release before the [QuPath Training Course in San Diego (October 9-11, 2023)](https://forum.image.sc/t/upcoming-qupath-training-course-october-9-11-san-diego/83751).
+
+### Bug fixed
+* Derived classifications can be read incorrectly from data files and json (https://github.com/qupath/qupath/issues/1306)
+
+
 ## Version 0.4.3
 
 This is a *minor release* that aims to be fully compatible with previous v0.4.x releases, while fixing bugs.

qupath-app/src/main/resources/VERSION

@@ -1 +1 @@
-0.4.3
+0.4.4

qupath-core-processing/src/main/resources/VERSION

@@ -1 +1 @@
-0.4.3
+0.4.4

qupath-core/src/main/java/qupath/lib/objects/classes/PathClass.java

@@ -23,6 +23,7 @@
 
 package qupath.lib.objects.classes;
 
+import java.io.ObjectStreamException;
 import java.io.Serializable;
 import java.util.ArrayList;
 import java.util.Arrays;
@@ -161,7 +162,17 @@ private static final UUID getSecret() {
 	 */
 	private static final Map<String, PathClass> existingClasses = new ConcurrentHashMap<>();
 
-
+	/**
+	 * Reset the cache of existing PathClass objects.
+	 * This should <b>only</b> be used for testing, to enable tests to be free of influence from previously-generated
+	 * classes.
+	 */
+	static synchronized void resetCaches() {
+		logger.warn("Resetting PathClass caches");
+		existingClasses.clear();
+	}
+
+
 	/**
 	 * Cache the set representation
 	 */
@@ -825,6 +836,12 @@ public static synchronized PathClass getSingleton(PathClass pathClass) {
 		// This can occur during deserialization
 		if (!pathClass.isDerivedClass() && pathClass.getName() == null)
 			return NULL_CLASS;
+		// Addition thanks to @carlocastoldi
+		// addresses https://github.com/qupath/qupath/issues/1306
+		if (pathClass.isDerivedClass()) {
+			var parent = getSingleton(pathClass.getParentClass());
+			return getInstance(parent, pathClass.getName(), pathClass.getColor());
+		}
 		var previous = existingClasses.putIfAbsent(createCacheString(pathClass), pathClass);
 		return previous == null ? pathClass : previous;
 	}
@@ -940,6 +957,17 @@ public static class StandardPathClasses {
 
 
 	}
-
+
+	/**
+	 * Called during deserialization to ensure that only a single instance of each PathClass exists.
+	 * Partly addresses https://github.com/qupath/qupath/issues/1306 (during standard deserialation,
+	 * but not from JSON)
+	 * @return
+	 * @throws ObjectStreamException
+	 */
+	protected Object readResolve() throws ObjectStreamException {
+		return getSingleton(this);
+	}
+
 
 }

qupath-core/src/main/resources/VERSION

@@ -1 +1 @@
-0.4.3
+0.4.4

qupath-core/src/test/java/qupath/lib/objects/classes/TestPathClass.java

@@ -23,6 +23,20 @@
 
 package qupath.lib.objects.classes;
 
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.Isolated;
+import qupath.lib.common.ColorTools;
+import qupath.lib.io.GsonTools;
+import qupath.lib.io.PathIO;
+import qupath.lib.objects.PathObject;
+
+import java.io.IOException;
+import java.io.InputStreamReader;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.List;
+
 import static org.junit.jupiter.api.Assertions.assertEquals;
 import static org.junit.jupiter.api.Assertions.assertNotEquals;
 import static org.junit.jupiter.api.Assertions.assertNotNull;
@@ -31,16 +45,14 @@
 import static org.junit.jupiter.api.Assertions.assertThrows;
 import static org.junit.jupiter.api.Assertions.assertTrue;
 
-import java.util.ArrayList;
-import java.util.Arrays;
-import java.util.List;
-
-import org.junit.jupiter.api.Test;
-
-import qupath.lib.common.ColorTools;
-
 @SuppressWarnings("javadoc")
+@Isolated
 public class TestPathClass {
+
+	@BeforeEach
+	void resetClasses() {
+		PathClass.resetCaches();
+	}
 
 	private static List<Integer> colors = Arrays.asList(
 			ColorTools.BLACK,
@@ -137,7 +149,76 @@ public void test_classes() {
 		assertEquals("Else", PathClass.getInstance(null, "Else   ", null).toString());
 
 	}
-
+
+	/**
+	 * Check deserializing using the v0.4 JSON representation.
+	 * @throws IOException
+	 */
+	@Test
+	public void test_pathClassJsonDeserialization() throws IOException {
+		try (var reader = new InputStreamReader(TestPathClass.class.getResourceAsStream("derived-class.json"))) {
+			var pathClass = GsonTools.getInstance().fromJson(reader, PathClass.class);
+			checkPathClassIsSingleton(pathClass);
+		}
+	}
+
+	/**
+	 * Check deserializing using the v0.3 JSON representation.
+	 * @throws IOException
+	 */
+	@Test
+	public void test_pathClassJsonLegacyDeserialization() throws IOException {
+		try (var reader = new InputStreamReader(TestPathClass.class.getResourceAsStream("derived-class-legacy.json"))) {
+			var pathClass = GsonTools.getInstance().fromJson(reader, PathClass.class);
+			checkPathClassIsSingleton(pathClass);
+		}
+	}
+
+	/**
+	 * Check deserializing a PathClass directly from an object output stream.
+	 * @throws IOException
+	 * @throws ClassNotFoundException
+	 */
+	@Test
+	public void test_pathClassJavaDeserialization() throws IOException, ClassNotFoundException {
+		try (var stream = PathIO.createObjectInputStream(TestPathClass.class.getResourceAsStream("derived-class.serialized"))) {
+			var pathClass = (PathClass)stream.readObject();
+			checkPathClassIsSingleton(pathClass);
+		}
+	}
+
+	/**
+	 * Check deserializing directly from an object output stream, extracting PathClass from a PathObject.
+	 * @throws IOException
+	 * @throws ClassNotFoundException
+	 */
+	@Test
+	public void test_pathObjectJavaDeserialization() throws IOException, ClassNotFoundException {
+		try (var stream = PathIO.createObjectInputStream(TestPathClass.class.getResourceAsStream("derived-class-object.serialized"))) {
+			var pathObject = (PathObject)stream.readObject();
+			var pathClass = pathObject.getPathClass();
+			checkPathClassIsSingleton(pathClass);
+		}
+	}
+
+	/**
+	 * Perform checks that a specified PathClass is a singleton, and that its base/parent classes also match those
+	 * that can be obtained separately.
+	 * <p>
+	 * Introduced because of https://github.com/qupath/qupath/issues/1306
+	 * @param pathClass
+	 */
+	private void checkPathClassIsSingleton(PathClass pathClass) {
+		assertEquals(pathClass, PathClass.getSingleton(pathClass));
+		assertEquals(pathClass, PathClass.fromString(pathClass.toString()));
+
+		var list = pathClass.toList();
+		assertEquals(pathClass.getBaseClass(), PathClass.fromString(list.get(0)));
+		assertEquals(pathClass, PathClass.fromCollection(list));
+		if (pathClass.isDerivedClass())
+			assertEquals(pathClass.getParentClass(), PathClass.fromCollection(list.subList(0, list.size()-1)));
+	}
+
 	private static int countColons(String s) {
 		return s.length() - s.replaceAll(":", "").length();
 	}

qupath-core/src/test/java/qupath/lib/objects/classes/TestPathClassFactory.java

@@ -23,10 +23,11 @@
 
 package qupath.lib.objects.classes;
 
-import static org.junit.jupiter.api.Assertions.assertEquals;
-import static org.junit.jupiter.api.Assertions.assertSame;
-import static org.junit.jupiter.api.Assertions.assertThrows;
-import static org.junit.jupiter.api.Assertions.assertTrue;
+import org.junit.jupiter.api.Assertions;
+import org.junit.jupiter.api.BeforeEach;
+import org.junit.jupiter.api.Test;
+import org.junit.jupiter.api.parallel.Isolated;
+import qupath.lib.common.ColorTools;
 
 import java.util.Arrays;
 import java.util.HashSet;
@@ -35,13 +36,19 @@
 import java.util.stream.Collectors;
 import java.util.stream.IntStream;
 
-import org.junit.jupiter.api.Assertions;
-import org.junit.jupiter.api.Test;
-
-import qupath.lib.common.ColorTools;
+import static org.junit.jupiter.api.Assertions.assertEquals;
+import static org.junit.jupiter.api.Assertions.assertSame;
+import static org.junit.jupiter.api.Assertions.assertThrows;
+import static org.junit.jupiter.api.Assertions.assertTrue;
 
 @SuppressWarnings("javadoc")
+@Isolated
 public class TestPathClassFactory {
+
+	@BeforeEach
+	void resetClasses() {
+		PathClass.resetCaches();
+	}
 
 	@Test
 	public void test_getPathClass1() {

qupath-core/src/test/resources/qupath/lib/objects/classes/derived-class-legacy.json

@@ -0,0 +1,16 @@
+{
+  "parentClass": {
+    "parentClass": {
+      "parentClass": {
+        "name": "First-legacy",
+        "colorRGB": -16711165
+      },
+      "name": "Second-legacy",
+      "colorRGB": -16711165
+    },
+    "name": "Third-legacy",
+    "colorRGB": -16711165
+  },
+  "name": "Fourth-legacy",
+  "colorRGB": -16711165
+}

qupath-core/src/test/resources/qupath/lib/objects/classes/derived-class.json

@@ -0,0 +1,9 @@
+{
+  "names": [
+    "First",
+    "Second",
+    "Third",
+    "Fourth"
+  ],
+  "color": [10, 20, 30]
+}

qupath-extension-bioformats/src/main/resources/VERSION

@@ -1 +1 @@
-0.4.3
+0.4.4

qupath-extension-openslide/src/main/resources/VERSION

@@ -1 +1 @@
-0.4.3
+0.4.4

qupath-extension-processing/src/main/resources/VERSION

@@ -1 +1 @@
-0.4.3
+0.4.4

qupath-extension-script-editor/src/main/resources/VERSION

@@ -1 +1 @@
-0.4.3
+0.4.4

qupath-extension-svg/src/main/resources/VERSION

@@ -1 +1 @@
-0.4.3
+0.4.4

qupath-gui-fx/src/main/resources/VERSION

@@ -1 +1 @@
-0.4.3
+0.4.4

settings.gradle

@@ -1,4 +1,4 @@
-gradle.ext.qupathVersion = "0.4.3"
+gradle.ext.qupathVersion = "0.4.4"
 
 rootProject.name = 'qupath'