fix: stale QuadItem removal after ClusterItem position updates

[Aparnamohan1312]

Summary
Fixes an issue where updateItem() may fail to remove the previously indexed QuadItem when a mutable ClusterItem changes position before being updated.

Reproduction
The issue occurs when the same mutable ClusterItem instance is reused:

1. Add a ClusterItem to NonHierarchicalDistanceBasedAlgorithm.
2. Update the item's position.
3. Call updateItem(item).
   Because the item's position has already changed, removeItem() reconstructs a new QuadItem using the updated coordinates instead of the coordinates used during insertion.

PointQuadTree.remove() then traverses the tree using the updated location, while the original QuadItem is still indexed under its previous location.

Root Cause
QuadItem caches the projected point at construction time.
removeItem() currently creates a new QuadItem from the current state of the ClusterItem. If the position has changed, the reconstructed QuadItem no longer represents the object that was originally inserted into the PointQuadTree, causing removal to search the wrong branch.

Fix
Store the original QuadItem created during addItem() in an internal lookup map keyed by the corresponding ClusterItem.
removeItem() and removeItems() now retrieve and remove the original QuadItem instance rather than constructing a new wrapper. This ensures removal uses the same cached coordinates that were used during insertion while preserving the existing public API and behavior.

Validation

• Existing add, remove, update, and bulk operations continue to behave as before.
• Added coverage for mutable ClusterItem position updates to exercise the affected update path.
• The change is internal, constant-time (O(1) lookup), and does not affect the library's public API.

---

Before submitting your PR, there are a few things you can do to make sure it goes smoothly:

• Make sure to open a GitHub issue as a bug/feature request before writing your code! That way we can discuss the change, evaluate designs, and agree on the general idea
• Edit the title of this pull request with a semantic commit prefix (e.g. "fix: "), which is necessary for automated release workflows to decide whether to generate a new release and what type it should be.
• Will this cause breaking changes to existing Java or Kotlin integrations? If so, ensure the commit has a BREAKING CHANGE footer so when this change is integrated a major version update is triggered. See: https://www.conventionalcommits.org/en/v1.0.0/
• Ensure the tests and linter pass
• Code coverage does not decrease (if any source code was changed)
• Appropriate docs were updated (if necessary)

Fixes #< #1729> 🦕

[dkhawk]

Thank you @Aparnamohan1312 for fixing #1729.

Review Summary

• Root Cause & Core Fix: When a mutable ClusterItem changes coordinates after insertion, constructing a new QuadItem(item) generates updated coordinates. PointQuadTree.remove(quadItem) then traverses down the tree using the new coordinates rather than where the item was originally indexed, leaving a stale entry trapped in mQuadTree.
• Solution Assessment: Storing the original QuadItem created in addItem() inside mItemMap (HashMap<T, QuadItem<T>>) allows O(1) retrieval of the original wrapper during removeItem() and removeItems(). This guarantees that PointQuadTree.remove() uses the original cached coordinates where the item is stored.
• Memory & Lifecycle Safety: Verified that clearItems(), removeItem(), and removeItems() all properly purge entries from mItemMap, preventing memory leaks.
• Subclass Safety Enhancement: Added a safe fallback (?: QuadItem(item)) in removeItem() and removeItems() so that any subclasses bypassing addItem() will fall back safely to standard wrapper construction.

Test Coverage & On-Device Demo Verification

• Unit Tests: Expanded QuadItemTest.java with 5 regression tests (testUpdateItemAfterPositionChange, testRemoveItemAfterPositionChange, testUpdateItemPreventsStaleQuadTreeEntries, testRemoveItemsAfterPositionChange, testClearItemsAfterPositionChange).
  • Verified tests fail against unpatched code (AssertionError: expected <0> but was <1> due to stale entries remaining in mQuadTree) and pass cleanly with the fix.
• Demo App Verification: Enhanced ClusteringDiffDemoActivity and Person to mutate ClusterItem coordinates in-place (setPosition()) and included 60 filler markers to trigger PointQuadTree quadrant splitting (MAX_ELEMENTS = 50). Verified on-device that rotating marker locations cleanly updates without stale clusters.

LGTM.

[dkhawk]

Hi @Aparnamohan1312,

Thank you again for the contribution. Notice that the cla/google check is currently failing on this PR.

Why the CLA check is failing

The cla/google bot verified that your GitHub account (@Aparnamohan1312, the PR opener) has signed the CLA. However, commit 50c12dc6 was authored using the email address aparna.mohan@walmart.com, which does not appear to be linked to your GitHub account or registered on your CLA signature.

The following contributors were found for this pull request:
✅ PR Opener: @Aparnamohan1312
❌ Author: <aparna.mohan@walmart.com>

How to fix this

You can resolve this using any of the following methods:

1. Link the email to your GitHub account: Add aparna.mohan@walmart.com to your GitHub email settings (https://github.com/settings/emails). Once verified, click the "rescan" link on the cla/google check details page.
2. Register the corporate email on your CLA: If aparna.mohan@walmart.com is covered under a corporate CLA, ensure it is added to your authorized contributor list at https://cla.developers.google.com/.
3. Amend the commit author email: If you prefer to use your personal or GitHub-associated email, you can amend the author email on your commit locally and force-push to your branch:

   git commit --amend --author="Aparna Mohan <YOUR_CLA_EMAIL@example.com>" --no-edit
   git push --force-with-lease

Please let us know once you have updated the CLA check so we can proceed with merging this PR.

[Aparnamohan1312]

Thank you @Aparnamohan1312 for fixing #1729.

Review Summary

• Root Cause & Core Fix: When a mutable ClusterItem changes coordinates after insertion, constructing a new QuadItem(item) generates updated coordinates. PointQuadTree.remove(quadItem) then traverses down the tree using the new coordinates rather than where the item was originally indexed, leaving a stale entry trapped in mQuadTree.
• Solution Assessment: Storing the original QuadItem created in addItem() inside mItemMap (HashMap<T, QuadItem<T>>) allows O(1) retrieval of the original wrapper during removeItem() and removeItems(). This guarantees that PointQuadTree.remove() uses the original cached coordinates where the item is stored.
• Memory & Lifecycle Safety: Verified that clearItems(), removeItem(), and removeItems() all properly purge entries from mItemMap, preventing memory leaks.
• Subclass Safety Enhancement: Added a safe fallback (?: QuadItem(item)) in removeItem() and removeItems() so that any subclasses bypassing addItem() will fall back safely to standard wrapper construction.

Test Coverage & On-Device Demo Verification

• Unit Tests: Expanded QuadItemTest.java with 5 regression tests (testUpdateItemAfterPositionChange, testRemoveItemAfterPositionChange, testUpdateItemPreventsStaleQuadTreeEntries, testRemoveItemsAfterPositionChange, testClearItemsAfterPositionChange).

  • Verified tests fail against unpatched code (AssertionError: expected <0> but was <1> due to stale entries remaining in mQuadTree) and pass cleanly with the fix.

• Demo App Verification: Enhanced ClusteringDiffDemoActivity and Person to mutate ClusterItem coordinates in-place (setPosition()) and included 60 filler markers to trigger PointQuadTree quadrant splitting (MAX_ELEMENTS = 50). Verified on-device that rotating marker locations cleanly updates without stale clusters.

LGTM.

Hi @dkhawk, thank you for reviewing. I've amended my email as suggested for CLA check.