Moti Asayag has posted comments on this change. Change subject: core: fix e8993e1ca33b823852e15b4adeb3df293afb5676 ......................................................................
Patch Set 3: Code-Review-1 (3 comments) https://gerrit.ovirt.org/#/c/38757/3/backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResource.java File backend/manager/modules/restapi/jaxrs/src/main/java/org/ovirt/engine/api/restapi/resource/AbstractBackendCollectionResource.java: Line 144: * @param list1 Line 145: * @param list2 Line 146: * @return Line 147: */ Line 148: private List<Q> intersect(List<Q> list1, List<Q> list2) { please use meaningful parameter names. This is a generic method which could be added to the Entities class as a static method and should also be covered by tests. Line 149: Collection<Guid> listId1 = new ArrayList<>(); Line 150: Collection<Guid> listId2 = new ArrayList<>(); Line 151: for (Q be : list1) { Line 152: listId1.add((Guid)((BusinessEntity)be).getId()); Line 152: ) i don't think Guid is correct. The definition of BusiessEntity doesn't enforce the type of the key, so there might be other key types, i.e String for Erratum, DiskLunMapId for DiskLunMap and so on. Line 156: } Line 157: Collection<Guid> result = CollectionUtils.intersection(listId1, listId2); Line 158: for (Q be : list1) { Line 159: if (! result.contains(((BusinessEntity)be).getId())) Line 160: list1.remove(be); this line suppose to produce ConcurrentModificationException. you need to use Iterator.remove(). But this logic is already introduced by: CollectionUtils.subtract(coll1, coll2) Line 161: } Line 162: return list1; Line 163: } Line 164: -- To view, visit https://gerrit.ovirt.org/38757 To unsubscribe, visit https://gerrit.ovirt.org/settings Gerrit-MessageType: comment Gerrit-Change-Id: I23ef5740ea59069b10afc329e5ef75bce3c33b31 Gerrit-PatchSet: 3 Gerrit-Project: ovirt-engine Gerrit-Branch: master Gerrit-Owner: Eli Mesika <[email protected]> Gerrit-Reviewer: Eli Mesika <[email protected]> Gerrit-Reviewer: Juan Hernandez <[email protected]> Gerrit-Reviewer: Moti Asayag <[email protected]> Gerrit-Reviewer: Ori Liel <[email protected]> Gerrit-Reviewer: Oved Ourfali <[email protected]> Gerrit-Reviewer: [email protected] Gerrit-Reviewer: oVirt Jenkins CI Server Gerrit-HasComments: Yes _______________________________________________ Engine-patches mailing list [email protected] http://lists.ovirt.org/mailman/listinfo/engine-patches
