diff --git a/ebean-api/src/main/java/io/ebean/common/BeanMap.java b/ebean-api/src/main/java/io/ebean/common/BeanMap.java index 2b9ef350d..643febc14 100644 --- a/ebean-api/src/main/java/io/ebean/common/BeanMap.java +++ b/ebean-api/src/main/java/io/ebean/common/BeanMap.java @@ -175,10 +175,6 @@ public final class BeanMap extends AbstractBeanCollection implements Ma /** * Returns the map entrySet. - *

- * This is because the key values may need to be set against the details (so - * they don't need to be set twice). - *

*/ @Override public Collection getActualEntries() { @@ -242,17 +238,12 @@ public final class BeanMap extends AbstractBeanCollection implements Ma } @Override - @SuppressWarnings({"unchecked"}) public Set> entrySet() { init(); if (isReadOnly()) { return Collections.unmodifiableSet(map.entrySet()); } - if (modifyListening) { - Set> s = map.entrySet(); - return new ModifySet(this, s); - } - return map.entrySet(); + return modifyListening ? new ModifyEntrySet<>(this, map.entrySet()) : map.entrySet(); } @Override @@ -273,8 +264,7 @@ public final class BeanMap extends AbstractBeanCollection implements Ma if (isReadOnly()) { return Collections.unmodifiableSet(map.keySet()); } - // we don't really care about modifications to the ketSet? - return map.keySet(); + return modifyListening ? new ModifyKeySet<>(this, map.keySet()) : map.keySet(); } @Override @@ -345,11 +335,7 @@ public final class BeanMap extends AbstractBeanCollection implements Ma if (isReadOnly()) { return Collections.unmodifiableCollection(map.values()); } - if (modifyListening) { - Collection c = map.values(); - return new ModifyCollection<>(this, c); - } - return map.values(); + return modifyListening ? new ModifyCollection<>(this, map.values()) : map.values(); } @Override diff --git a/ebean-api/src/main/java/io/ebean/common/ModifyCollection.java b/ebean-api/src/main/java/io/ebean/common/ModifyCollection.java index 05b203a2f..48f703ba8 100644 --- a/ebean-api/src/main/java/io/ebean/common/ModifyCollection.java +++ b/ebean-api/src/main/java/io/ebean/common/ModifyCollection.java @@ -25,7 +25,7 @@ class ModifyCollection implements Collection { * The owner is notified of the additions and removals. *

*/ - public ModifyCollection(BeanCollection owner, Collection c) { + ModifyCollection(BeanCollection owner, Collection c) { this.owner = owner; this.c = c; } diff --git a/ebean-api/src/main/java/io/ebean/common/ModifyEntrySet.java b/ebean-api/src/main/java/io/ebean/common/ModifyEntrySet.java new file mode 100644 index 000000000..174863a74 --- /dev/null +++ b/ebean-api/src/main/java/io/ebean/common/ModifyEntrySet.java @@ -0,0 +1,131 @@ +package io.ebean.common; + +import java.util.*; + +/** + * Handles the Entry Set for BeanMap. + */ +class ModifyEntrySet implements Set> { + + private final BeanMap owner; + private final Set> entrySet; + + ModifyEntrySet(BeanMap owner, Set> entrySet) { + this.owner = owner; + this.entrySet = entrySet; + } + + @Override + public int size() { + return entrySet.size(); + } + + @Override + public boolean isEmpty() { + return entrySet.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return entrySet.contains(o); + } + + @Override + public Object[] toArray() { + return entrySet.toArray(); + } + + @Override + public T[] toArray(T[] a) { + return entrySet.toArray(a); + } + + @Override + public boolean containsAll(Collection entries) { + return entrySet.containsAll(entries); + } + + @Override + public void clear() { + owner.clear(); + } + + @Override + public boolean add(Map.Entry entry) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean addAll(Collection> c) { + throw new UnsupportedOperationException(); + } + + @SuppressWarnings("rawtypes") + @Override + public boolean remove(Object o) { + if (o instanceof Map.Entry) { + Map.Entry entry = (Map.Entry) o; + final E val = owner.get(entry.getKey()); + if (Objects.equals(val, entry.getValue())) { + owner.remove(entry.getKey()); + return true; + } + } + return false; + } + + @Override + public boolean retainAll(Collection entries) { + boolean modified = false; + final Iterator> it = iterator(); + while (it.hasNext()) { + if (!entries.contains(it.next())) { + it.remove(); + modified = true; + } + } + return modified; + } + + @Override + public boolean removeAll(Collection entries) { + boolean modified = false; + for (Object entry : entries) { + modified |= remove(entry); + } + return modified; + } + + @Override + public Iterator> iterator() { + return new EntrySetIterator(new ArrayList<>(entrySet).iterator()); + } + + class EntrySetIterator implements Iterator> { + + private final Iterator> iterator; + private Map.Entry entry; + + EntrySetIterator(Iterator> iterator) { + this.iterator = iterator; + } + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public Map.Entry next() { + entry = iterator.next(); + return entry; + } + + @Override + public void remove() { + owner.remove(entry.getKey()); + iterator.remove(); + } + } + +} diff --git a/ebean-api/src/main/java/io/ebean/common/ModifyKeySet.java b/ebean-api/src/main/java/io/ebean/common/ModifyKeySet.java new file mode 100644 index 000000000..e9f829b9b --- /dev/null +++ b/ebean-api/src/main/java/io/ebean/common/ModifyKeySet.java @@ -0,0 +1,126 @@ +package io.ebean.common; + +import java.util.ArrayList; +import java.util.Collection; +import java.util.Iterator; +import java.util.Set; + +/** + * Handle the Key Set for BeanMap. + */ +class ModifyKeySet implements Set { + + private final Set keySet; + private final BeanMap owner; + + ModifyKeySet(BeanMap owner, Set keySet) { + this.owner = owner; + this.keySet = keySet; + } + + @Override + public int size() { + return keySet.size(); + } + + @Override + public boolean isEmpty() { + return keySet.isEmpty(); + } + + @Override + public boolean contains(Object o) { + return keySet.contains(o); + } + + @Override + public Object[] toArray() { + return keySet.toArray(); + } + + @Override + public T[] toArray(T[] a) { + return keySet.toArray(a); + } + + @Override + public boolean add(E key) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean addAll(Collection keys) { + throw new UnsupportedOperationException(); + } + + @Override + public boolean remove(Object o) { + return owner.remove(o) != null; + } + + @Override + public boolean containsAll(Collection keys) { + return keySet.containsAll(keys); + } + + @Override + public void clear() { + owner.clear(); + } + + @Override + public Iterator iterator() { + return new KeySetIterator<>(new ArrayList<>(keySet).iterator()); + } + + @Override + public boolean retainAll(Collection keys) { + return keysMatch(keys, false); + } + + @Override + public boolean removeAll(Collection keys) { + return keysMatch(keys, true); + } + + private boolean keysMatch(Collection keys, boolean containsMatch) { + boolean changed = false; + final Iterator iterator = iterator(); + while (iterator.hasNext()) { + final E key = iterator.next(); + if (keys.contains(key) == containsMatch) { + iterator.remove(); + changed = true; + } + } + return changed; + } + + + class KeySetIterator implements Iterator { + + private final Iterator iterator; + private K key; + + KeySetIterator(Iterator iterator) { + this.iterator = iterator; + } + + @Override + public boolean hasNext() { + return iterator.hasNext(); + } + + @Override + public K next() { + key = iterator.next(); + return key; + } + + @Override + public void remove() { + owner.remove(key); + iterator.remove(); + } + } +} diff --git a/ebean-api/src/main/java/io/ebean/common/ModifySet.java b/ebean-api/src/main/java/io/ebean/common/ModifySet.java deleted file mode 100644 index 1a0a2e1bf..000000000 --- a/ebean-api/src/main/java/io/ebean/common/ModifySet.java +++ /dev/null @@ -1,24 +0,0 @@ -package io.ebean.common; - -import io.ebean.bean.BeanCollection; - -import java.util.Set; - -/** - * Wraps a Set for the purposes of notifying removals and additions to the - * BeanCollection owner. - *

- * This is required for persisting ManyToMany objects. Additions and removals - * become inserts and deletes to the intersection table. - *

- */ -class ModifySet extends ModifyCollection implements Set { - - /** - * Create with an Owner that is notified of any additions or deletions. - */ - public ModifySet(BeanCollection owner, Set s) { - super(owner, s); - } - -} diff --git a/ebean-core/src/test/java/io/ebean/common/BeanMapTest.java b/ebean-core/src/test/java/io/ebean/common/BeanMapTest.java index 08a57e05a..003539a72 100644 --- a/ebean-core/src/test/java/io/ebean/common/BeanMapTest.java +++ b/ebean-core/src/test/java/io/ebean/common/BeanMapTest.java @@ -2,18 +2,21 @@ package io.ebean.common; import io.ebean.bean.BeanCollection; import org.junit.Test; +import org.tests.model.basic.EBasic; -import java.util.LinkedHashMap; -import java.util.Map; +import java.util.*; +import static java.util.Arrays.asList; import static org.assertj.core.api.Assertions.assertThat; public class BeanMapTest { - Object object1 = new Object(); - Object object2 = new Object(); - Object object3 = new Object(); + private final EBasic object1 = new EBasic("o1"); + private final EBasic object2 = new EBasic("o2"); + private final EBasic object3 = new EBasic("o3"); + private final EBasic object4 = new EBasic("o4"); + private final EBasic object5 = new EBasic("o5"); private Map all() { Map all = new LinkedHashMap<>(); @@ -174,9 +177,7 @@ public class BeanMapTest { @Test public void testClear_given_someBeansInAdditions() throws Exception { - BeanMap map = new BeanMap<>(); - map.put("1", object1); - map.setModifyListening(BeanCollection.ModifyListenMode.ALL); + BeanMap map = newModifyListeningMap(); map.put("2", object2); map.put("3", object3); @@ -188,4 +189,252 @@ public class BeanMapTest { assertThat(map.getModifyAdditions()).isEmpty(); } + @Test(expected = UnsupportedOperationException.class) + public void keySet_add_whenModifyListening() { + BeanMap map = newModifyListeningMap(); + map.keySet().add("3"); + } + + @Test(expected = UnsupportedOperationException.class) + public void keySet_add() { + BeanMap map = new BeanMap<>(); + map.keySet().add("3"); + } + + @Test(expected = UnsupportedOperationException.class) + public void keySet_addAll_whenModifyListening() { + BeanMap map = newModifyListeningMap(); + map.keySet().addAll(asList("3", "4")); + } + + @Test(expected = UnsupportedOperationException.class) + public void keySet_addAll() { + BeanMap map = new BeanMap<>(); + map.keySet().addAll(asList("3", "4")); + } + + @Test + public void keySet_remove() { + + BeanMap map = new BeanMap<>(); + map.put("1", object1); + map.put("2", object2); + map.setModifyListening(BeanCollection.ModifyListenMode.ALL); + + final Set keySet = map.keySet(); + keySet.remove("1"); + + assertThat(keySet.contains("1")).isFalse(); + assertThat(map).doesNotContainKeys("1"); + assertThat(map.get("1")).isNull(); + + assertThat(map.getModifyRemovals()).containsExactly(object1); + } + + @Test + public void keySet_clear() { + + BeanMap map = new BeanMap<>(); + map.put("1", object1); + map.put("2", object2); + map.setModifyListening(BeanCollection.ModifyListenMode.ALL); + + final Set keySet = map.keySet(); + keySet.clear(); + + assertThat(map).isEmpty(); + assertThat(keySet).isEmpty(); + + assertThat(map.getModifyRemovals()).containsExactly(object1, object2); + } + + @Test + public void keySet_iterator_remove() { + + BeanMap map = new BeanMap<>(); + map.put("1", object1); + map.put("2", object2); + map.put("3", object3); + map.setModifyListening(BeanCollection.ModifyListenMode.ALL); + + final Set keySet = map.keySet(); + final Iterator iterator = keySet.iterator(); + while (iterator.hasNext()) { + final String key = iterator.next(); + if (key.equals("2")) { + iterator.remove(); + } + } + + assertThat(map).hasSize(2); + assertThat(keySet).hasSize(2); + assertThat(keySet).containsExactly("1", "3"); + assertThat(map).containsKeys("1", "3"); + + assertThat(map.getModifyRemovals()).containsExactly(object2); + } + + @Test + public void keySet_removeAll() { + + BeanMap map = new BeanMap<>(); + map.put("1", object1); + map.put("2", object2); + map.put("3", object3); + map.put("4", object4); + map.put("5", object5); + map.setModifyListening(BeanCollection.ModifyListenMode.ALL); + + final Set keySet = map.keySet(); + final boolean changed = keySet.removeAll(asList("2", "3", "5")); + + assertThat(changed).isTrue(); + assertThat(map).hasSize(2); + assertThat(keySet).hasSize(2); + assertThat(keySet).containsExactly("1", "4"); + assertThat(map).containsKeys("1", "4"); + + assertThat(map.getModifyRemovals()).containsExactly(object2, object3, object5); + } + + + @Test + public void keySet_retainAll() { + + BeanMap map = new BeanMap<>(); + map.put("1", object1); + map.put("2", object2); + map.put("3", object3); + map.put("4", object4); + map.put("5", object5); + map.setModifyListening(BeanCollection.ModifyListenMode.ALL); + + final Set keySet = map.keySet(); + final boolean changed = keySet.retainAll(asList("2", "3", "5")); + + assertThat(changed).isTrue(); + assertThat(map).hasSize(3); + assertThat(keySet).hasSize(3); + assertThat(keySet).containsExactly("2", "3", "5"); + assertThat(map).containsKeys("2", "3", "5"); + + assertThat(map.getModifyRemovals()).containsExactly(object1, object4); + } + + @Test(expected = UnsupportedOperationException.class) + public void values_add() { + BeanMap map = new BeanMap<>(); + map.values().add(object3); + } + + @Test(expected = UnsupportedOperationException.class) + public void values_addAll() { + BeanMap map = new BeanMap<>(); + map.values().addAll(asList(object3, object5)); + } + + @Test(expected = UnsupportedOperationException.class) + public void entrySet_add() { + newModifyListeningMap() + .entrySet() + .add(new AbstractMap.SimpleEntry<>("3", object3)); + } + + @Test + public void entrySet_clear() { + final BeanMap map = newModifyListeningMap(); + final Set> entries = map.entrySet(); + entries.clear(); + + assertThat(entries).isEmpty(); + assertThat(map).isEmpty(); + assertThat(map.getModifyRemovals()).containsExactly(object1); + } + + @Test + public void entrySet_remove() { + final BeanMap map = newModifyListeningMap5(); + final Set> entries = map.entrySet(); + + assertThat(map).hasSize(5); + + final boolean existed1 = entries.remove(new AbstractMap.SimpleEntry<>("1", object1)); + assertThat(existed1).isTrue(); + + final boolean existed22 = entries.remove(new AbstractMap.SimpleEntry<>("22", object1)); + assertThat(existed22).isFalse(); + + assertThat(map).hasSize(4); + assertThat(map.getModifyRemovals()).containsExactly(object1); + } + + @Test + public void entrySet_remove_whenNotEqualValue() { + final BeanMap map = newModifyListeningMap5(); + final Set> entries = map.entrySet(); + + assertThat(map).hasSize(5); + + final boolean modified = entries.remove(new AbstractMap.SimpleEntry<>("1", object2)); + assertThat(modified).isFalse(); + + assertThat(map).hasSize(5); + assertThat(map.getModifyRemovals()).isNull(); + } + + @Test + public void entrySet_iterator_remove() { + final BeanMap map = newModifyListeningMap5(); + final Set> entries = map.entrySet(); + final Iterator> iterator = entries.iterator(); + while (iterator.hasNext()) { + final Map.Entry entry = iterator.next(); + if (entry.getKey().equals("2") || entry.getKey().equals("5")) { + iterator.remove(); + } + } + assertThat(map).hasSize(3); + assertThat(entries).hasSize(3); + assertThat(map.getModifyRemovals()).containsExactly(object2, object5); + } + + @Test + public void entrySet_removeAll() { + final BeanMap map = newModifyListeningMap5(); + final Set> entries = map.entrySet(); + + entries.removeAll(asList(new AbstractMap.SimpleEntry<>("1", object1), new AbstractMap.SimpleEntry<>("3", object4), new AbstractMap.SimpleEntry<>("4", object4))); + assertThat(map).hasSize(3); + assertThat(entries).hasSize(3); + assertThat(map.getModifyRemovals()).containsExactly(object1, object4); + } + + @Test + public void entrySet_retainAll() { + final BeanMap map = newModifyListeningMap5(); + final Set> entries = map.entrySet(); + + entries.retainAll(asList(new AbstractMap.SimpleEntry<>("1", object1), new AbstractMap.SimpleEntry<>("3", object4), new AbstractMap.SimpleEntry<>("4", object4))); + assertThat(map).hasSize(2); + assertThat(entries).hasSize(2); + assertThat(map.getModifyRemovals()).containsExactly(object2, object3, object5); + } + + private BeanMap newModifyListeningMap() { + BeanMap map = new BeanMap<>(); + map.put("1", object1); + map.setModifyListening(BeanCollection.ModifyListenMode.ALL); + return map; + } + + private BeanMap newModifyListeningMap5() { + BeanMap map = new BeanMap<>(); + map.put("1", object1); + map.put("2", object2); + map.put("3", object3); + map.put("4", object4); + map.put("5", object5); + map.setModifyListening(BeanCollection.ModifyListenMode.ALL); + return map; + } } diff --git a/ebean-core/src/test/java/org/tests/model/map/BeanMapOrphanRemovalTest.java b/ebean-core/src/test/java/org/tests/model/map/BeanMapOrphanRemovalTest.java new file mode 100644 index 000000000..48288e7a3 --- /dev/null +++ b/ebean-core/src/test/java/org/tests/model/map/BeanMapOrphanRemovalTest.java @@ -0,0 +1,50 @@ +package org.tests.model.map; + +import io.ebean.DB; +import org.junit.Test; + +import java.util.Arrays; +import java.util.Map; +import java.util.Set; + +import static org.assertj.core.api.Assertions.assertThat; + + +public class BeanMapOrphanRemovalTest { + + @Test + public void keySet_retainAll() { + + MpUser user = new MpUser(); + user.setName("u1"); + addRoles(user, "r1", "r2", "r3", "r4"); + DB.save(user); + + final MpUser user1 = DB.find(MpUser.class, user.getId()); + final Map roles = user1.getRoles(); + assertThat(roles).hasSize(4); + + + final Set keySet = roles.keySet(); + keySet.retainAll(Arrays.asList("r2", "r3")); + + DB.save(user1); + + final MpUser user2 = DB.find(MpUser.class, user.getId()); + final Map roles2 = user2.getRoles(); + assertThat(roles2).hasSize(2); + } + + private void addRoles(MpUser user, String... roles){ + for (String code : roles) { + MpRole role = newRole(code); + user.getRoles().put(role.getCode(), role); + } + } + + private MpRole newRole(String code) { + MpRole role = new MpRole(); + role.setCode(code); + return role; + } +} diff --git a/ebean-core/src/test/java/org/tests/model/map/MpUser.java b/ebean-core/src/test/java/org/tests/model/map/MpUser.java index 100e1e6a7..94edddb19 100644 --- a/ebean-core/src/test/java/org/tests/model/map/MpUser.java +++ b/ebean-core/src/test/java/org/tests/model/map/MpUser.java @@ -1,11 +1,7 @@ package org.tests.model.map; -import javax.persistence.CascadeType; -import javax.persistence.Entity; -import javax.persistence.Id; -import javax.persistence.MapKey; -import javax.persistence.OneToMany; -import java.util.HashMap; +import javax.persistence.*; +import java.util.LinkedHashMap; import java.util.Map; @Entity @@ -16,9 +12,9 @@ public class MpUser { private String name; - @OneToMany(cascade = CascadeType.ALL) + @OneToMany(cascade = CascadeType.ALL, orphanRemoval = true) @MapKey(name = "code") - public Map roles = new HashMap<>(); + private Map roles = new LinkedHashMap<>(); public Long getId() { return id;