Replace equals Regions on ranging beacons requests

lib/src/main/java/org/altbeacon/beacon/BeaconManager.java

@@ -56,7 +56,6 @@
 import org.altbeacon.beacon.simulator.BeaconSimulator;
 import org.altbeacon.beacon.utils.ProcessUtils;
 
-import java.util.ArrayList;
 import java.util.Collection;
 import java.util.Collections;
 import java.util.Iterator;
@@ -141,7 +140,7 @@ public class BeaconManager {
     protected final Set<MonitorNotifier> monitorNotifiers = new CopyOnWriteArraySet<>();
 
     @NonNull
-    private final ArrayList<Region> rangedRegions = new ArrayList<>();
+    private final Set<Region> rangedRegions = new CopyOnWriteArraySet<>();
 
     @NonNull
     private final List<BeaconParser> beaconParsers = new CopyOnWriteArrayList<>();
@@ -838,9 +837,8 @@ public void startRangingBeaconsInRegion(@NonNull Region region) throws RemoteExc
         if (determineIfCalledFromSeparateScannerProcess()) {
             return;
         }
-        synchronized (rangedRegions) {
-            rangedRegions.add(region);
-        }
+        rangedRegions.remove(region);
+        rangedRegions.add(region);
         applyChangesToServices(BeaconService.MSG_START_RANGING, region);
     }
 
@@ -863,15 +861,7 @@ public void stopRangingBeaconsInRegion(@NonNull Region region) throws RemoteExce
         if (determineIfCalledFromSeparateScannerProcess()) {
             return;
         }
-        synchronized (rangedRegions) {
-            Region regionToRemove = null;
-            for (Region rangedRegion : rangedRegions) {
-                if (region.getUniqueId().equals(rangedRegion.getUniqueId())) {
-                    regionToRemove = rangedRegion;
-                }
-            }
-            rangedRegions.remove(regionToRemove);
-        }
+        rangedRegions.remove(region);
         applyChangesToServices(BeaconService.MSG_STOP_RANGING, region);
     }
 
@@ -1096,13 +1086,23 @@ public Collection<Region> getMonitoredRegions() {
     }
 
     /**
-     * @return the list of regions currently being ranged
+     * Read-only access to the {@link Region} instances currently being ranged
+     * <p>
+     * This provides a thread-safe "read-only" view.
+     * Attempts to modify the returned set, or its iterator, will throw an
+     * {@link UnsupportedOperationException}. Modifications to the underlying set should be made
+     * through {@link #startRangingBeaconsInRegion(Region)} and
+     * {@link #stopRangingBeaconsInRegion(Region)}.
+     *
+     * @return a thread-safe {@linkplain Collections#unmodifiableSet(Set) unmodifiable view}
+     * providing "read-only" access to the registered {@link Region} instances
+     * @see #startRangingBeaconsInRegion(Region)
+     * @see #stopRangingBeaconsInRegion(Region)
+     * @see Collections#unmodifiableSet(Set)
      */
     @NonNull
     public Collection<Region> getRangedRegions() {
-        synchronized(this.rangedRegions) {
-            return new ArrayList<>(this.rangedRegions);
-        }
+        return Collections.unmodifiableSet(rangedRegions);
     }
 
     /**

lib/src/main/java/org/altbeacon/beacon/service/BeaconService.java

@@ -369,7 +369,8 @@ public void startRangingBeaconsInRegion(Region region, Callback callback) {
         synchronized (mScanHelper.getRangedRegionState()) {
             if (mScanHelper.getRangedRegionState().containsKey(region)) {
                 LogManager.i(TAG, "Already ranging that region -- will replace existing region.");
-                mScanHelper.getRangedRegionState().remove(region); // need to remove it, otherwise the old object will be retained because they are .equal //FIXME That is not true
+                // Need to explicitly remove because only value is updated for equals keys.
+                mScanHelper.getRangedRegionState().remove(region);
             }
             mScanHelper.getRangedRegionState().put(region, new RangeState(callback));
             LogManager.d(TAG, "Currently ranging %s regions.", mScanHelper.getRangedRegionState().size());

lib/src/test/java/org/altbeacon/beacon/BeaconManagerTest.java

@@ -0,0 +1,82 @@
+package org.altbeacon.beacon;
+
+import org.altbeacon.beacon.logging.LogManager;
+import org.altbeacon.beacon.logging.Loggers;
+import org.altbeacon.beacon.simulator.BeaconSimulator;
+import org.junit.Before;
+import org.junit.Test;
+import org.junit.runner.RunWith;
+import org.robolectric.RobolectricTestRunner;
+import org.robolectric.RuntimeEnvironment;
+import org.robolectric.annotation.Config;
+
+import java.util.Collections;
+import java.util.List;
+
+import static org.junit.Assert.*;
+
+@RunWith(RobolectricTestRunner.class)
+@Config(sdk = 28)
+public class BeaconManagerTest {
+
+  @Before
+  public void before() {
+    org.robolectric.shadows.ShadowLog.stream = System.err;
+    LogManager.setLogger(Loggers.verboseLogger());
+    LogManager.setVerboseLoggingEnabled(true);
+    BeaconManager.setsManifestCheckingDisabled(true);
+    BeaconManager.setBeaconSimulator(new BeaconSimulator() {
+      @Override
+      public List<Beacon> getBeacons() {
+        return Collections.emptyList();
+      }
+    });
+  }
+
+  @Test
+  public void startRangingBeaconsInRegionMultipleInvocationsTest() throws Exception {
+    BeaconManager beaconManager = BeaconManager
+        .getInstanceForApplication(RuntimeEnvironment.application);
+
+    String id = "id";
+    Region region1 = new Region(id, Collections.<Identifier>emptyList());
+    Region region2 = new Region(id, "00:11:22:33:FF:EE");
+
+    beaconManager.startRangingBeaconsInRegion(region1);
+    assertEquals(beaconManager.getRangedRegions().size(), 1);
+    assertSame(beaconManager.getRangedRegions().iterator().next(), region1);
+    assertNotSame(beaconManager.getRangedRegions().iterator().next(), region2);
+
+    beaconManager.startRangingBeaconsInRegion(region2);
+    assertEquals(beaconManager.getRangedRegions().size(), 1);
+    assertNotSame(beaconManager.getRangedRegions().iterator().next(), region1);
+    assertSame(beaconManager.getRangedRegions().iterator().next(), region2);
+
+    Region region3 = new Region(id + "-other", Collections.<Identifier>emptyList());
+    beaconManager.startRangingBeaconsInRegion(region3);
+    assertEquals(beaconManager.getRangedRegions().size(), 2);
+  }
+
+  @Test
+  public void stopRangingBeaconsInRegionTest() throws Exception {
+    BeaconManager beaconManager = BeaconManager
+        .getInstanceForApplication(RuntimeEnvironment.application);
+
+    String id = "id";
+    Region region1 = new Region(id, Collections.<Identifier>emptyList());
+    Region region2 = new Region(id, "00:11:22:33:FF:EE");
+    Region region3 = new Region(id + "-other", Collections.<Identifier>emptyList());
+
+    beaconManager.startRangingBeaconsInRegion(region1);
+    beaconManager.startRangingBeaconsInRegion(region2);
+    beaconManager.startRangingBeaconsInRegion(region3);
+    assertEquals(beaconManager.getRangedRegions().size(), 2);
+
+    beaconManager.stopRangingBeaconsInRegion(region1);
+    assertEquals(beaconManager.getRangedRegions().size(), 1);
+
+    beaconManager.stopRangingBeaconsInRegion(region3);
+    assertEquals(beaconManager.getRangedRegions().size(), 0);
+  }
+
+}