Merge branch 'fix215' into fix191_cat_keywords

.github/workflows/svg.yml

@@ -3,6 +3,9 @@ name: SVG
 on:
   workflow_dispatch:
   push:
+    paths:
+      - 'android/data/src/main/res/raw/*.svg'
+      - 'android/data/svg/**'
     branches:
       - main
 

android/backup/src/main/java/net/twisterrob/inventory/android/activity/BackupActivity.java

@@ -113,34 +113,40 @@ private boolean shouldDisplay(@Nullable Progress progress) {
 		if (progress == null) {
 			return false;
 		}
-		// this should ignore any incoming finish progress while the dialog is up
-		// and hopefully that only happens when Google Drive reads the second time
-		// lack of initiating component (see doExportExternal), try the best heuristic to filter Drive peeking for size:
-		if (unhandled != null) {
-			// while another finish is already displayed (because the BackupService is sequential)
-			LOG.warn("Double-finish\n{}\n{}", unhandled, progress);
-			boolean cancelled = false, pipe = false;
-			if (progress.failure instanceof CancellationException) {
-				cancelled = true;
-				Throwable cause = progress.failure.getCause();
-				if (IOTools.isEPIPE(cause)) {
-					pipe = true;
-					// EPIPE error (this just means that external party closed the PIPE)
-					for (StackTraceElement st : cause.getStackTrace()) {
-						// stack trace failed in ZippedXMLExporter.copyXSLT
-						if (ZippedXMLExporter.class.getName().equals(st.getClassName())
-								&& "copyXSLT".equals(st.getMethodName())) {
-							// very likely the external app closed the stream
-							// or the user has 100kb free space, but then... good for them
-							LOG.warn("Ignoring Google Drive's weirdness of peeking for size.", progress.failure);
-							return false;
-						}
+
+		// If another finish is not yet displayed, display this progress immediately.
+		// This is ok, because the BackupService is sequential.
+		if (unhandled == null) {
+			return true;
+		}
+
+		// The following should ignore any incoming finish progress while the dialog is up.
+		// Hopefully that only happens when Google Drive reads the second time.
+		// Due to a lack of initiating component (see BackupControllerFragment.doExport),
+		// try the best heuristic to filter Drive peeking for size.
+		LOG.warn("Double-finish\n{}\n{}", unhandled, progress);
+		boolean cancelled = false, pipe = false;
+		if (progress.failure instanceof CancellationException) {
+			cancelled = true;
+			Throwable cause = progress.failure.getCause();
+			if (IOTools.isEPIPE(cause)) {
+				pipe = true;
+				// EPIPE error (this just means that external party closed the PIPE)
+				for (StackTraceElement st : cause.getStackTrace()) {
+					// Stack trace failed in ZippedXMLExporter.copyEntry
+					if (ZippedXMLExporter.class.getName().equals(st.getClassName())
+							&& "copyEntry".equals(st.getMethodName())) {
+						// very likely the external app closed the stream
+						// or the user has 100kb free space, but then... good for them
+						LOG.warn("Ignoring Google Drive's weirdness of peeking for size.", progress.failure);
+						return false;
 					}
 				}
 			}
-			LOG.warn("Letting double-dialog display: cancelled={}, pipe={}, stack={}", cancelled, pipe, false);
-			// anything that doesn't match the above: it's better to display double dialog
 		}
+
+		// Anything that doesn't match the above: it's better to display double dialog.
+		LOG.warn("Letting double-dialog display: cancelled={}, pipe={}, stack={}", cancelled, pipe, false);
 		return true;
 	}
 

android/backup/src/main/java/net/twisterrob/inventory/android/backup/concurrent/BackupService.java

@@ -186,6 +186,7 @@ public static void enqueueWork(@NonNull Context context, @NonNull Intent work) {
 						.get(exportFactory.get().progress(dispatcher))
 						.parcelExporter();
 				ParcelFileDescriptor file = queue.remove();
+				LOG.trace("Dequeued export to {}", file);
 				finish(exporter.exportTo(file));
 			} else if (ACTION_EXPORT.equals(intent.getAction())) {
 				dispatcher.setCancellable(false);
@@ -242,6 +243,7 @@ private Progress importFrom(Uri input) {
 
 	public class LocalBinder extends Binder {
 		public void export(@NonNull ParcelFileDescriptor pfd) {
+			LOG.trace("Enqueueing export to {}", pfd);
 			queue.add(ObjectTools.checkNotNull(pfd));
 			BackupService context = BackupService.this;
 			Intent intent = new Intent(BackupService.ACTION_EXPORT_PFD_WORKAROUND, null, context, BackupService.class);

android/backup/src/main/java/net/twisterrob/inventory/android/backup/xml/ZippedXMLExporter.java

@@ -80,6 +80,10 @@ private void validateXml(InputStream xml) throws IOException {
 		}
 	}
 
+	/**
+	 * @noinspection JavadocReference
+	 * @see net.twisterrob.inventory.android.activity.BackupActivity#shouldDisplay where this method is reflected upon.
+	 */
 	private void copyEntry(String zipFileName, InputStream stream) throws IOException {
 		LOG.trace("Copying into {}", zipFileName);
 		try {

android/data/svg/src/main/AndroidManifest.xml

@@ -1,14 +1,10 @@
 <manifest
 	xmlns:android="http://schemas.android.com/apk/res/android"
-	xmlns:tools="http://schemas.android.com/tools"
 	>
 
 	<application
 		android:icon="@android:drawable/sym_def_app_icon"
-		android:allowBackup="false"
-		tools:ignore="DataExtractionRules"
 		>
-		<!-- lint:DataExtractionRules not necessary for this test app. -->
 	</application>
 
 </manifest>

android/src/androidTest/java/net/twisterrob/inventory/android/InventoryJUnitRunner.java

@@ -1,7 +1,13 @@
 package net.twisterrob.inventory.android;
 
+import android.os.Bundle;
+
 import net.twisterrob.android.test.junit.AndroidJUnitRunner;
+import net.twisterrob.inventory.android.hacks.ViewCompatHacks;
 
 public class InventoryJUnitRunner extends AndroidJUnitRunner {
-	// Empty for now, useful for quick debugging.
+	@Override public void onCreate(Bundle arguments) {
+		super.onCreate(arguments);
+		ViewCompatHacks.patchFor293190504();
+	}
 }

android/src/androidTest/java/net/twisterrob/inventory/android/activity/ImageLoadingTest.java

@@ -27,7 +27,8 @@ public class ImageLoadingTest {
 	private final MainActivityActor main = new MainActivityActor();
 
 	@Category({UseCase.Complex.class, On.Main.class, On.Item.class})
-	@Test public void test() {
+	@Test(timeout = 30 * 1000)
+	public void test() {
 		MainActivityActor.Navigator home = main.assertHomeScreen();
 		HomeRoomsActor rooms = home.rooms();
 		rooms.assertExists("!All Categories");

android/src/androidTest/java/net/twisterrob/inventory/android/activity/ItemActivityTest_Image.java

@@ -40,7 +40,8 @@ public class ItemActivityTest_Image {
 
 	@LargeTest
 	@Category({UseCase.Complex.class, On.Item.class, Op.DeletesBelonging.class})
-	@Test public void testImageDeletedWithItem() throws IOException {
+	@Test(timeout = 30 * 1000)
+	public void testImageDeletedWithItem() throws IOException {
 		ItemEditActivityActor newItem = roomView.addItem();
 		newItem.setName(TEST_ITEM);
 		newItem.save();

android/src/androidTest/java/net/twisterrob/inventory/android/activity/ItemViewActivityTest_Selection.java

@@ -140,7 +140,8 @@ public class ItemViewActivityTest_Selection {
 	 * This test is a bit jumpy because hasSelection jumps to the item being verified.
 	 */
 	@Category({Op.Cancels.class, UseCase.Complex.class})
-	@Test public void testExitsRandomSelectionAll() {
+	@Test(timeout = 2 * 60 * 1000)
+	public void testExitsRandomSelectionAll() {
 		final int count = 9;
 		createItems(count);
 		SelectionActor selection = itemView.select(subItem(1));

android/src/androidTest/java/net/twisterrob/inventory/android/backup/xml/BackupDataXmlExportImportIntgTest.java

@@ -3,12 +3,14 @@
 import java.io.*;
 
 import org.junit.*;
+import org.junit.runner.RunWith;
 
 import android.content.Context;
 import android.database.Cursor;
 
 import androidx.annotation.NonNull;
 import androidx.test.core.app.ApplicationProvider;
+import androidx.test.ext.junit.runners.AndroidJUnit4;
 
 import net.twisterrob.inventory.android.backup.Importer;
 import net.twisterrob.inventory.android.content.*;
@@ -17,6 +19,7 @@
 import net.twisterrob.inventory.android.test.InventoryActivityRule;
 import net.twisterrob.inventory.android.test.activity.TestActivity;
 
+@RunWith(AndroidJUnit4.class)
 public class BackupDataXmlExportImportIntgTest {
 
 	private static final String BAD_XML = "<!-- --> -- & &amp; &lt; &gt; <?xml?> <![CDATA[ ]]>";

android/src/androidTest/java/net/twisterrob/inventory/android/backup/xml/XMLImporterTest.java

@@ -1,5 +1,7 @@
 package net.twisterrob.inventory.android.backup.xml;
 
+import java.util.concurrent.TimeUnit;
+
 import org.junit.*;
 import org.junit.function.ThrowingRunnable;
 import org.mockito.*;
@@ -20,13 +22,15 @@
 import net.twisterrob.inventory.android.content.Database;
 import net.twisterrob.inventory.android.content.contract.PropertyType;
 import net.twisterrob.inventory.android.content.model.Types;
+import net.twisterrob.inventory.android.test.TimeoutRule;
 import net.twisterrob.java.io.IOTools;
 
 import static net.twisterrob.test.hamcrest.Matchers.*;
 
 public class XMLImporterTest {
 
-	@Rule public final MockitoRule mockito = MockitoJUnit.rule();
+	@Rule(order = 1) public final TimeoutRule timeout = new TimeoutRule(5, TimeUnit.SECONDS);
+	@Rule(order = 2) public final MockitoRule mockito = MockitoJUnit.rule();
 
 	@Mock Database mockDatabase;
 	@Mock Types mockTypes;

android/src/androidTest/java/net/twisterrob/inventory/android/backup/xml/ZippedXMLExporterTest.java

@@ -1,6 +1,7 @@
 package net.twisterrob.inventory.android.backup.xml;
 
 import java.io.*;
+import java.util.concurrent.TimeUnit;
 import java.util.zip.ZipFile;
 
 import static java.util.Collections.*;
@@ -30,6 +31,7 @@
 import net.twisterrob.android.utils.tools.IOTools;
 import net.twisterrob.inventory.android.backup.Exporter;
 import net.twisterrob.inventory.android.content.Database;
+import net.twisterrob.inventory.android.test.TimeoutRule;
 
 import static net.twisterrob.test.hamcrest.Matchers.hasEntry;
 import static net.twisterrob.test.hamcrest.Matchers.*;
@@ -38,8 +40,9 @@ public class ZippedXMLExporterTest {
 
 	private static final byte[] BOM = {(byte)0xEF, (byte)0xBB, (byte)0xBF};
 
-	@Rule public final TemporaryFolder temp = new TemporaryFolder();
-	@Rule public final MockitoRule mockito = MockitoJUnit.rule();
+	@Rule(order = 1) public final TimeoutRule timeout = new TimeoutRule(5, TimeUnit.SECONDS);
+	@Rule(order = 2) public final TemporaryFolder temp = new TemporaryFolder();
+	@Rule(order = 3) public final MockitoRule mockito = MockitoJUnit.rule();
 
 	@Mock Database mockDatabase;
 	@Mock CursorExporter mockExporter;

android/src/androidTest/java/net/twisterrob/inventory/android/hacks/DeferredWeakHashMapFor293190504.kt

@@ -0,0 +1,73 @@
+package net.twisterrob.inventory.android.hacks
+
+import java.util.LinkedList
+import java.util.WeakHashMap
+import kotlin.collections.MutableMap.MutableEntry
+
+private typealias Change<K, V> = DeferredWeakHashMapFor293190504<K, V>.() -> Unit
+
+/**
+ * See https://issuetracker.google.com/issues/293190504
+ *
+ * This is a workaround for not allowing concurrent modifications
+ * to [androidx.core.view.ViewCompat.AccessibilityPaneVisibilityManager]'s internal state.
+ *
+ * Any structure-modifying changes made to the map while iterating
+ * is deferred until the iteration is over.
+ *
+ * This prevents the nested `put` call in `checkPaneVisibility`
+ * from cleaning up the weak queue mid-iteration.
+ */
+internal class DeferredWeakHashMapFor293190504<K, V> : WeakHashMap<K, V>() {
+	@Volatile
+	private var locked = false
+	private val changes: MutableList<Change<K, V>> = LinkedList()
+
+	override fun put(key: K, value: V): V? {
+		return if (!locked) {
+			super.put(key, value)
+		} else {
+			changes.add { put(key, value) }
+			// Should be super.get(key), but reads also mutate the map because it's weak.
+			null
+		}
+	}
+
+	override fun remove(key: K?): V? {
+		return if (!locked) {
+			super.remove(key)
+		} else {
+			changes.add { remove(key) }
+			// Should be super.get(key), but reads also mutate the map because it's weak.
+			null
+		}
+	}
+
+	override val entries: MutableSet<MutableEntry<K, V>>
+		get() = DeferringEntrySet(super.entries)
+
+	private inner class DeferringEntrySet(
+		private val backingSet: MutableSet<MutableEntry<K, V>>
+	) : MutableSet<MutableEntry<K, V>> by backingSet {
+
+		override fun iterator(): MutableIterator<MutableEntry<K, V>> {
+			check(!locked) { "Already iterating, only one supported." }
+			locked = true
+			val iterator = backingSet.iterator()
+			return object : MutableIterator<MutableEntry<K, V>> by iterator {
+				override fun hasNext(): Boolean {
+					val hasNext = iterator.hasNext()
+					// Assuming a for(:) loop will not do anything after this.
+					if (!hasNext) {
+						locked = false
+						changes.removeAll {
+							this@DeferredWeakHashMapFor293190504.it()
+							return@removeAll true
+						}
+					}
+					return hasNext
+				}
+			}
+		}
+	}
+}

android/src/androidTest/java/net/twisterrob/inventory/android/hacks/ViewCompatHacks.java

@@ -0,0 +1,30 @@
+package net.twisterrob.inventory.android.hacks;
+
+import java.lang.reflect.Field;
+
+public class ViewCompatHacks {
+	/**
+	 * Replace the {@link java.util.WeakHashMap} in {@link androidx.core.view.ViewCompat} with a
+	 * {@link DeferredWeakHashMapFor293190504} to prevent concurrent modification.
+	 *
+	 * @see <a href="https://issuetracker.google.com/issues/293190504">Issue</a>
+	 * @see <a href="https://github.com/TWiStErRob/net.twisterrob.inventory/issues/302">Issue</a>
+	 */
+	public static void patchFor293190504() {
+		try {
+			Field sAccessibilityPaneVisibilityManager = Class
+					.forName("androidx.core.view.ViewCompat")
+					.getDeclaredField("sAccessibilityPaneVisibilityManager");
+			sAccessibilityPaneVisibilityManager.setAccessible(true);
+			Field mPanesToVisible = Class
+					.forName("androidx.core.view.ViewCompat$AccessibilityPaneVisibilityManager")
+					.getDeclaredField("mPanesToVisible");
+			mPanesToVisible.setAccessible(true);
+
+			Object target = sAccessibilityPaneVisibilityManager.get(null);
+			mPanesToVisible.set(target, new DeferredWeakHashMapFor293190504<>());
+		} catch (NoSuchFieldException | ClassNotFoundException | IllegalAccessException ex) {
+			throw new RuntimeException(ex);
+		}
+	}
+}

android/src/androidTest/java/net/twisterrob/inventory/android/test/InventoryActivityRule.java

@@ -1,6 +1,7 @@
 package net.twisterrob.inventory.android.test;
 
 import java.io.File;
+import java.util.concurrent.TimeUnit;
 
 import org.junit.runner.Description;
 import org.junit.runners.model.Statement;
@@ -55,6 +56,7 @@ public InventoryActivityRule<T> dontClearWelcomeFlag() {
 		base = new GlideIdlingResourceRule().apply(base, description);
 		base = new InventoryGlideResetRule().apply(base, description);
 		base = new ExternalAppKiller().apply(base, description);
+		base = new TimeoutRule(20, TimeUnit.SECONDS).apply(base, description);
 		return base;
 	}