Say why a sign-in failed, and add the seam a map test needs

app/build.gradle.kts

@@ -287,7 +287,7 @@ chaquopy {
             // wheel for desktop platforms and a pure-Python `py3-none-any` one as well.
             // There is no Android wheel, so pip falls back to the pure-Python build - which
             // is correct but markedly slower. The messages here are small enough not to care.
-            install("git+https://github.com/parawanderer/FindMy.py@102dd8ea14767d2a2aa745186ac23276f32689f1")
+            install("git+https://github.com/parawanderer/FindMy.py@23a9b8d7109b405f8362ea1e69ebe51f9ca82fca")
 
             install("NSKeyedUnArchiver==1.5")
         }

app/src/androidTest/java/dev/wander/android/opentagviewer/AppleLoginFlowTest.java

@@ -12,6 +12,7 @@
 import static androidx.test.espresso.matcher.ViewMatchers.withId;
 import static androidx.test.espresso.matcher.ViewMatchers.withText;
 import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
+import static org.hamcrest.Matchers.containsString;
 import static org.junit.Assert.assertEquals;
 import static org.junit.Assert.assertFalse;
 import static org.junit.Assert.assertNotNull;
@@ -264,6 +265,50 @@ public void awrongPasswordLeavesThemAbleToTryAgain() {
         onView(withId(R.id.login_button_main)).check(matches(isDisplayed()));
     }
 
+    /**
+     * A sign-in that could not reach Apple says so, in words.
+     *
+     * <p><b>The screen used to show "Login failed:" and nothing else.</b> The most common real
+     * failure is a connection timeout, and `str(TimeoutError())` is the empty string, so the
+     * message it echoed was empty - leaving somebody unable to tell a wrong password from a dead
+     * network from a broken app.
+     *
+     * <p>Asserted on the text, not on "an error appeared". The previous version showed an error
+     * too; it just did not say anything.
+     */
+    @Test
+    public void afailureToReachAppleSaysSoRatherThanShowingAnEmptyError() {
+        this.apple = FakeAppleAuthService.cannotReachApple();
+        AppDependencies.replaceAuthService(this.apple);
+
+        launch();
+        signIn();
+
+        final String expected = getInstrumentation().getTargetContext()
+                .getString(R.string.login_failed_network);
+
+        Eventually.check(() -> onView(withId(R.id.login_error_message_text))
+                .check(matches(withText(expected))));
+    }
+
+    /**
+     * And an unrecognised failure still names something.
+     *
+     * <p>The fallback matters as much as the classified case: it is what anything unexpected
+     * lands in, and it must not be able to render as a bare colon again.
+     */
+    @Test
+    public void anunrecognisedFailureStillShowsItsDetail() {
+        this.apple = FakeAppleAuthService.rejectsTheSignIn("Bad password");
+        AppDependencies.replaceAuthService(this.apple);
+
+        launch();
+        signIn();
+
+        Eventually.check(() -> onView(withId(R.id.login_error_message_text))
+                .check(matches(withText(containsString("Bad password")))));
+    }
+
     /** A rejected code says so, and gives the boxes back rather than stranding them. */
     @Test
     public void aWrongCodeIsReportedAndTheBoxesComeBack() {

app/src/androidTest/java/dev/wander/android/opentagviewer/anisette/AdiDeviceIdentityTest.java

@@ -142,10 +142,21 @@ public void everyProfileSuppliesTheSixFieldsFindMyExpects() throws Exception {
         }
     }
 
-    /** A fresh install is the new profile. The old one is only ever recovered, never chosen. */
+    /**
+     * A fresh install claims the Mac.
+     *
+     * <p>It claimed the iPhone until Apple began answering 401 to {@code get_2fa_methods} for
+     * clients presenting that profile - provisioning and password auth both succeeded, and the
+     * very next request did not. The desktop exporter makes the same call against the same
+     * account and is answered, and this profile is byte-identical to what it provisions with.
+     *
+     * <p>So this asserts a decision taken from evidence, not a preference. If it ever changes
+     * back, that has to be because the 2FA question was answered - not because an iPhone icon
+     * looks better in a device list.
+     */
     @Test
-    public void afreshIdentityIsAnIphone() {
-        assertEquals(Hardware.IPHONE, AdiDeviceIdentity.generate().hardware());
+    public void afreshIdentityIsTheMacTheExporterAlsoUses() {
+        assertEquals(Hardware.LEGACY_MAC, AdiDeviceIdentity.generate().hardware());
     }
 
     /**
@@ -167,15 +178,15 @@ public void afreshIdentityHasTheShapesAdiAccepts() {
     }
 
     /**
-     * A fresh install's local user id is a UUID, because FindMy.py's is.
+     * The iPhone profile's local user id is a UUID, because FindMy.py's is.
      *
      * <p>Not cosmetic. The value Java provisions ADI with is handed to FindMy.py verbatim and
      * encoded there, so it has to be a string both sides can carry and that Apple has seen in
      * this shape before - which is the UUID every FindMy.py client already sends.
      */
     @Test
-    public void afreshInstallsLocalUserIdIsAUuid() {
-        final String id = AdiDeviceIdentity.generate().localUserUuid();
+    public void theiphoneProfilesLocalUserIdIsAUuid() {
+        final String id = Hardware.IPHONE.newLocalUserId(new java.security.SecureRandom());
 
         assertEquals(36, id.length());
         assertEquals(id.toUpperCase(java.util.Locale.ROOT), id);
@@ -191,12 +202,11 @@ public void afreshInstallsLocalUserIdIsAUuid() {
      * itself to Apple as two.
      */
     @Test
-    public void afreshInstallProvisionsUnderWhatFindMyWillSend() {
-        final AdiDeviceIdentity fresh = AdiDeviceIdentity.generate();
-        final String whatJavaSends =
-                fresh.hardware().localUserHeader(fresh.localUserUuid());
+    public void theiphoneProfileProvisionsUnderWhatFindMyWouldSend() {
+        final String id = Hardware.IPHONE.newLocalUserId(new java.security.SecureRandom());
+        final String whatJavaSends = Hardware.IPHONE.localUserHeader(id);
         final String whatFindMyWillSend = Base64.encodeToString(
-                fresh.localUserUuid().getBytes(StandardCharsets.UTF_8), Base64.NO_WRAP);
+                id.getBytes(StandardCharsets.UTF_8), Base64.NO_WRAP);
 
         assertEquals(whatFindMyWillSend, whatJavaSends);
     }

app/src/androidTest/java/dev/wander/android/opentagviewer/anisette/LocalAnisetteIdentityTest.java

@@ -102,11 +102,17 @@ public void anIdentityWrittenBeforeProfilesExistedIsStillTheMac() throws Excepti
         assertEquals("MacBookPro13,2", modelOf(subject().hardwareProfileJson()));
     }
 
-    /** Nothing stored at all is a genuinely new install, and gets the profile worth having. */
+    /**
+     * Nothing stored at all is a genuinely new install - and it claims the Mac.
+     *
+     * <p>It claimed an iPhone until Apple answered 401 to {@code get_2fa_methods} for clients
+     * presenting that profile. See {@code AdiDeviceIdentity#generate()}.
+     */
     @Test
-    public void afreshInstallIsAnIphone() throws Exception {
-        assertEquals(AdiDeviceIdentity.Hardware.IPHONE.toJson(), subject().hardwareProfileJson());
-        assertEquals("iPhone15,2", modelOf(subject().hardwareProfileJson()));
+    public void afreshInstallIsTheMac() throws Exception {
+        assertEquals(AdiDeviceIdentity.Hardware.LEGACY_MAC.toJson(),
+                subject().hardwareProfileJson());
+        assertEquals("MacBookPro13,2", modelOf(subject().hardwareProfileJson()));
     }
 
     /**
@@ -140,7 +146,7 @@ public void readingALegacyProfileDoesNotWriteOneOverTheTopOfIt() {
     public void afreshInstallRecordsWhatItDecided() {
         subject().hardwareProfileJson();
 
-        assertEquals(AdiDeviceIdentity.Hardware.IPHONE.name(),
+        assertEquals(AdiDeviceIdentity.Hardware.LEGACY_MAC.name(),
                 this.preferences.getString(LocalAnisette.KEY_HARDWARE, null));
         assertNotNull(this.preferences.getString(LocalAnisette.KEY_DEVICE_ID, null));
         assertNotNull(this.preferences.getString(LocalAnisette.KEY_ADI_ID, null));
@@ -267,6 +273,11 @@ public void ahalfWrittenIdentityIsTreatedAsAbsent() {
                 .putString(LocalAnisette.KEY_DEVICE_ID, OLD_DEVICE_ID)
                 .commit());
 
-        assertEquals(AdiDeviceIdentity.Hardware.IPHONE.toJson(), subject().hardwareProfileJson());
+        assertEquals(AdiDeviceIdentity.Hardware.LEGACY_MAC.toJson(),
+                subject().hardwareProfileJson());
+        // Not the point of this test, but worth pinning: it wrote a *new* identity rather than
+        // adopting the orphaned device id.
+        assertNotEquals(OLD_DEVICE_ID,
+                this.preferences.getString(LocalAnisette.KEY_DEVICE_ID, null));
     }
 }

app/src/androidTest/java/dev/wander/android/opentagviewer/db/repo/BeaconRepositoryBackfillTest.java

@@ -143,6 +143,37 @@ public void amixOfBothKindsRefreshesTogether() {
         assertEquals("only the paired one needs converting", 1, converter.calls.size());
     }
 
+    /**
+     * The crash a real import produced.
+     *
+     * <p>The import path built this map with {@code Collectors.toMap}, which throws on a null
+     * value - so importing a self-generated tag ended in a {@link NullPointerException} deep in
+     * the stream machinery, with nothing naming the tag or the import. It is the only kind of
+     * tag that arrives here with no plist, so it was also the only way to find it.
+     */
+    @Test
+    public void amixOfBothKindsSurvivesBeingCollected() {
+        final Map<String, String> fallbacks = BeaconRepository.plistFallbacks(List.of(
+                OwnedBeacon.builder().id("paired-1").content(PLIST).build(),
+                OwnedBeacon.builder().id("oh-1").content(null).accessoryJson(CUSTOM_JSON).build()));
+
+        assertEquals(2, fallbacks.size());
+        assertEquals(PLIST, fallbacks.get("paired-1"));
+        assertNull("a tag with no plist keeps its key and a null value", fallbacks.get("oh-1"));
+        assertTrue("the key must be there, or that tag is simply never fetched",
+                fallbacks.containsKey("oh-1"));
+    }
+
+    /** And an import of nothing but self-generated tags is still a map of tags. */
+    @Test
+    public void anImportOfOnlySelfGeneratedTagsCollectsFine() {
+        final Map<String, String> fallbacks = BeaconRepository.plistFallbacks(List.of(
+                OwnedBeacon.builder().id("oh-1").content(null).accessoryJson(CUSTOM_JSON).build()));
+
+        assertEquals(1, fallbacks.size());
+        assertTrue(fallbacks.containsKey("oh-1"));
+    }
+
     /**
      * The alignment record is what stops the first fetch searching the tag's whole
      * history, so the backfill has to hand it to the converter rather than dropping it.

app/src/androidTest/java/dev/wander/android/opentagviewer/python/ChaquopyHardwareDescriberTest.java

@@ -0,0 +1,105 @@
+package dev.wander.android.opentagviewer.python;
+
+import static androidx.test.platform.app.InstrumentationRegistry.getInstrumentation;
+import static org.junit.Assert.assertEquals;
+import static org.junit.Assert.assertNotNull;
+import static org.junit.Assert.assertNull;
+
+import com.chaquo.python.Python;
+import com.chaquo.python.android.AndroidPlatform;
+
+import androidx.test.ext.junit.runners.AndroidJUnit4;
+
+import org.junit.BeforeClass;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+
+/**
+ * Java asking the shared heuristic what an accessory is.
+ *
+ * <p>The heuristic itself is tested in {@code python/opentagviewer_export/tests/} and reachable
+ * from the APK per {@code PythonPackagingTest}. What is tested here is the <b>Java side of the
+ * bridge</b>: that the wrapper hands over what the function expects, returns the string rather
+ * than a {@code PyObject}'s {@code toString} of something else, and - the part that matters most
+ * - never throws, because it is called from a screen that must render either way.
+ */
+@RunWith(AndroidJUnit4.class)
+public class ChaquopyHardwareDescriberTest {
+
+    private static final String AIRTAG_PLIST =
+            "<?xml version=\"1.0\" encoding=\"UTF-8\"?>\n"
+            + "<!DOCTYPE plist PUBLIC \"-//Apple//DTD PLIST 1.0//EN\""
+            + " \"http://www.apple.com/DTDs/PropertyList-1.0.dtd\">\n"
+            + "<plist version=\"1.0\">\n"
+            + "<dict>\n"
+            + "    <key>identifier</key><string>725A989D-D871-49A7-B2FE-948C24F356AB</string>\n"
+            + "    <key>model</key><string></string>\n"
+            + "    <key>productId</key><integer>21760</integer>\n"
+            + "    <key>vendorId</key><integer>76</integer>\n"
+            + "    <key>stableIdentifier</key><array>"
+            + "<string>2001~#001234a12345aaac~#A02BCDEFG1AB</string></array>\n"
+            + "</dict>\n"
+            + "</plist>\n";
+
+    private final HardwareDescriber describer = new ChaquopyHardwareDescriber();
+
+    @BeforeClass
+    public static void startPython() {
+        if (!Python.isStarted()) {
+            Python.start(new AndroidPlatform(
+                    getInstrumentation().getTargetContext().getApplicationContext()));
+        }
+    }
+
+    /** The whole point: a real answer, crossing the bridge, from the shared module. */
+    @Test
+    public void anairTagIsNamedRatherThanNumbered() {
+        assertEquals("AirTag", this.describer.describe(AIRTAG_PLIST));
+    }
+
+    /**
+     * A tag with no plist is a null answer rather than a crash.
+     *
+     * <p>A self-generated tag has none, and this is the screen's most common non-Apple case.
+     *
+     * <p><b>Deliberately not named for the short-circuit.</b> The implementation returns before
+     * crossing the bridge, which saves starting an interpreter - but nothing here can observe
+     * that, and Python would answer None for a null plist anyway. Asserting the contract this
+     * test can actually see beats a name implying one it cannot.
+     */
+    @Test
+    public void atagWithoutAPlistIsANullAnswer() {
+        assertNull(this.describer.describe(null));
+        assertNull(this.describer.describe(""));
+        assertNull(this.describer.whereToLookUp(null));
+    }
+
+    /**
+     * Nonsense is a null answer, not an exception.
+     *
+     * <p>The screen calls this after it has already drawn a label. Throwing would replace a
+     * correct-if-vague answer with a crash, which is a strictly worse trade - so the failure
+     * mode has to be "no improvement", and that is worth pinning rather than trusting.
+     */
+    @Test
+    public void garbageIsRefusedQuietly() {
+        assertNull(this.describer.describe("not a plist at all"));
+        assertNull(this.describer.whereToLookUp("not a plist at all"));
+    }
+
+    /**
+     * And the second question answers too, so the wrapper is not accidentally one function.
+     *
+     * <p>Only asserts that the call completes and is consistent with itself: what it says for an
+     * AirTag - a name it recognises - is the shared module's business, and pinning the sentence
+     * here would be the copy this design exists to avoid.
+     */
+    @Test
+    public void thelookupHintIsReachableToo() {
+        // A recognised accessory needs no lookup hint; the contract is that asking is safe.
+        this.describer.whereToLookUp(AIRTAG_PLIST);
+
+        assertNotNull("a recognised accessory should still describe",
+                this.describer.describe(AIRTAG_PLIST));
+    }
+}

app/src/androidTest/java/dev/wander/android/opentagviewer/python/FakeAppleAuthService.java

@@ -85,6 +85,22 @@ public static FakeAppleAuthService rejectsTheSignIn(final String message) {
         return fake;
     }
 
+    /**
+     * Apple could not be reached at all - the failure that produced an empty error message.
+     *
+     * <p>Worth its own named state rather than {@code rejectsTheSignIn("")}, because the shape
+     * is what matters: a timeout carries <b>no message</b>, so the screen has to build the
+     * sentence from the reason instead of echoing what it was handed.
+     */
+    public static FakeAppleAuthService cannotReachApple() {
+        final FakeAppleAuthService fake =
+                new FakeAppleAuthService(LOGIN_STATE.LOGGED_OUT, null);
+        // Empty, exactly as str(TimeoutError()) arrives from Python.
+        fake.loginFailsWith = new PythonAccountLoginException(
+                "", PythonAccountLoginException.REASON_NETWORK);
+        return fake;
+    }
+
     /** Signing in works, but the code that gets typed is refused. */
     public FakeAppleAuthService thatRejectsTheCode(final String message) {
         this.codeFailsWith = new PythonAccountLoginException(message);