Skip to content
This repository has been archived by the owner on Apr 30, 2024. It is now read-only.

Fix concurrent modifcation exception when destroying scoops #60

Merged
merged 2 commits into from
Apr 4, 2016
Merged

Fix concurrent modifcation exception when destroying scoops #60

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
20ab1f6
Fix concurrent modifcation exception when destroying scoops
Apr 2, 2016
ca38ac5
Adding unit test
itspbj Apr 4, 2016
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 6 additions & 1 deletion scoop/src/main/java/com/lyft/scoop/Scoop.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -6,8 +6,10 @@
import android.view.View;
import android.view.ViewGroup;
import java.util.HashMap;
import java.util.HashSet;
import java.util.LinkedHashMap;
import java.util.Map;
import java.util.Set;

public final class Scoop {

Expand All @@ -30,7 +32,10 @@ private Scoop(String name, Scoop parent, Map<String, Object> services) {
}

public void destroy() {
for (Map.Entry<String, Scoop> entry : this.children.entrySet()) {
final Set<Map.Entry<String, Scoop>> entries = this.children.entrySet();

final Set<Map.Entry<String, Scoop>> entriesCopy = new HashSet<>(entries);
for (Map.Entry<String, Scoop> entry : entriesCopy) {
Copy link

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

@buildbreaker This could be simplified

for (Scoop value : entriesCopy.values()) {
  value.destroy();
}

Or iterate over a list of scoops:
List<Scoop> valuesCopy = new ArrayList(entries.values())

It's already merged though so it's ok.

entry.getValue().destroy();
}

Expand Down
46 changes: 45 additions & 1 deletion scoop/src/test/java/com/lyft/scoop/ScoopTest.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@
@RunWith(RobolectricTestRunner.class)
@Config(manifest = Config.NONE)
public class ScoopTest {

private static final Scoop TEST_SCOOP = new Scoop.Builder("root").build();

@Test
Expand Down Expand Up @@ -67,7 +68,7 @@ public void findServiceInParent() {
}

@Test
public void destroyRootScoop() {
public void destroyChildScoop() {
FooService service1 = new FooService();

Scoop rootScoop = new Scoop.Builder("root")
Expand Down Expand Up @@ -118,7 +119,50 @@ public void fromViewNoScoop() {
} catch (RuntimeException e) {
//Expected result
}
}

public void destroyRootScoop() {
FooService service1 = new FooService();

Scoop rootScoop = new Scoop.Builder("root")
.service("foo_service_1", service1)
.build();

FooService childService1 = new FooService();

Scoop childScoop1 = new Scoop.Builder("child1", rootScoop)
.service("child_service_1", childService1)
.build();

FooService childService2 = new FooService();

Scoop childScoop2 = new Scoop.Builder("child2", rootScoop)
.service("child_service_2", childService2)
.build();

FooService service3 = new FooService();

Scoop grandChildScoop = new Scoop.Builder("grand_child", childScoop1)
.service("foo_service_3", service3)
.build();

rootScoop.destroy();

Assert.assertTrue(rootScoop.isDestroyed());
Assert.assertNull(rootScoop.findChild("child"));
Assert.assertNotNull(rootScoop.findService("foo_service_1"));

Assert.assertTrue(childScoop1.isDestroyed());
Assert.assertNotNull(childScoop1.getParent());
Assert.assertNull(childScoop1.findChild("grand_child"));
Assert.assertNotNull(childScoop1.findService("foo_service_1"));
Assert.assertNotNull(childScoop1.findService("child_service_1"));

Assert.assertTrue(grandChildScoop.isDestroyed());
Assert.assertNotNull(grandChildScoop.getParent());
Assert.assertNotNull(grandChildScoop.findService("foo_service_1"));
Assert.assertNotNull(grandChildScoop.findService("child_service_1"));
Assert.assertNotNull(grandChildScoop.findService("foo_service_3"));
}

static class FooService {
Expand Down