diff --git a/lib/src/main/java/org/altbeacon/beacon/BeaconManager.java b/lib/src/main/java/org/altbeacon/beacon/BeaconManager.java index 33d2976bc..3572befdb 100644 --- a/lib/src/main/java/org/altbeacon/beacon/BeaconManager.java +++ b/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 monitorNotifiers = new CopyOnWriteArraySet<>(); @NonNull - private final ArrayList rangedRegions = new ArrayList<>(); + private final Set rangedRegions = new CopyOnWriteArraySet<>(); @NonNull private final List 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 getMonitoredRegions() { } /** - * @return the list of regions currently being ranged + * Read-only access to the {@link Region} instances currently being ranged + *

+ * 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 getRangedRegions() { - synchronized(this.rangedRegions) { - return new ArrayList<>(this.rangedRegions); - } + return Collections.unmodifiableSet(rangedRegions); } /** diff --git a/lib/src/main/java/org/altbeacon/beacon/service/BeaconService.java b/lib/src/main/java/org/altbeacon/beacon/service/BeaconService.java index 6b07817f6..f3e886a71 100644 --- a/lib/src/main/java/org/altbeacon/beacon/service/BeaconService.java +++ b/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()); diff --git a/lib/src/test/java/org/altbeacon/beacon/BeaconManagerTest.java b/lib/src/test/java/org/altbeacon/beacon/BeaconManagerTest.java new file mode 100644 index 000000000..60094940a --- /dev/null +++ b/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 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.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.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.emptyList()); + Region region2 = new Region(id, "00:11:22:33:FF:EE"); + Region region3 = new Region(id + "-other", Collections.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); + } + +}