Hi Gil,
Awesome work :) it took me a while to understand that you also committed tests along with the refactoring. For the future I recommend mentioning this somewhere in the commit log for easy review. Also, perhaps as a further improvement you might want to rename the tStream variable into something more meaningful. The variable name might be a bit vague. On Fri, Aug 31, 2018 at 6:46 PM <p...@apache.org> wrote: > > Author: pgil > Date: Fri Aug 31 15:32:02 2018 > New Revision: 1839763 > > URL: http://svn.apache.org/viewvc?rev=1839763&view=rev > Log: > Implemented: Refactor EntityUtil findBy methods using Stream API > (OFBIZ-10537) > > Thanks Jacques and Mathieu for the review > > Added: > > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/test/EntityUtilTestSuite.java > (with props) > Modified: > > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtil.java > ofbiz/ofbiz-framework/trunk/framework/entity/testdef/entitytests.xml > > Added: > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/test/EntityUtilTestSuite.java > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/test/EntityUtilTestSuite.java?rev=1839763&view=auto > ============================================================================== > --- > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/test/EntityUtilTestSuite.java > (added) > +++ > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/test/EntityUtilTestSuite.java > Fri Aug 31 15:32:02 2018 > @@ -0,0 +1,101 @@ > +/******************************************************************************* > + * Licensed to the Apache Software Foundation (ASF) under one > + * or more contributor license agreements. See the NOTICE file > + * distributed with this work for additional information > + * regarding copyright ownership. The ASF licenses this file > + * to you under the Apache License, Version 2.0 (the > + * "License"); you may not use this file except in compliance > + * with the License. You may obtain a copy of the License at > + * > + * http://www.apache.org/licenses/LICENSE-2.0 > + * > + * Unless required by applicable law or agreed to in writing, > + * software distributed under the License is distributed on an > + * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY > + * KIND, either express or implied. See the License for the > + * specific language governing permissions and limitations > + * under the License. > + > *******************************************************************************/ > +package org.apache.ofbiz.entity.test; > + > +import org.apache.ofbiz.base.util.StringUtil; > +import org.apache.ofbiz.base.util.UtilMisc; > +import org.apache.ofbiz.entity.GenericValue; > +import org.apache.ofbiz.entity.condition.EntityCondition; > +import org.apache.ofbiz.entity.condition.EntityExpr; > +import org.apache.ofbiz.entity.condition.EntityOperator; > +import org.apache.ofbiz.entity.testtools.EntityTestCase; > +import org.apache.ofbiz.entity.util.EntityUtil; > + > +import java.util.ArrayList; > +import java.util.List; > + > +public class EntityUtilTestSuite extends EntityTestCase { > + > + public static final String module = EntityUtilTestSuite.class.getName(); > + > + public static final long TEST_COUNT = 1000; > + > + public EntityUtilTestSuite(String name) { > + super(name); > + } > + > + private List<GenericValue> prepareGenericValueList() { > + List<GenericValue> newValues = new ArrayList<>(); > + for (int i = 0; i < TEST_COUNT; i++) { > + newValues.add(delegator.makeValue("Testing", "testingId", > StringUtil.padNumberString(String.valueOf(i), 5), "description", "Description > " + i % 10)); > + } > + return newValues; > + } > + > + public void testGetFieldListFromEntityList() { > + List<GenericValue> newValues = prepareGenericValueList(); > + List<String> descriptionList = > EntityUtil.getFieldListFromEntityList(newValues, "description", false); > + assertEquals("Get not distinct field list from " + TEST_COUNT + " > entity", TEST_COUNT, descriptionList.size()); > + assertEquals("Get first description value", "Description 0", > descriptionList.get(0)); > + assertEquals("Get tens description value", "Description 0", > descriptionList.get(10)); > + > + descriptionList = EntityUtil.getFieldListFromEntityList(newValues, > "description", true); > + assertEquals("Get distinct field list from " + TEST_COUNT + " > entity, modulo 10 values", 10, descriptionList.size()); > + assertEquals("Get first description value", "Description 0", > descriptionList.get(0)); > + } > + > + public void testFilterByCondition() { > + List<GenericValue> newValues = prepareGenericValueList(); > + EntityExpr condition = EntityCondition.makeCondition("description", > "Description 0"); > + List<GenericValue> filteredValues = > EntityUtil.filterByCondition(newValues, condition); > + assertEquals("Filter on 10% description condition " + TEST_COUNT + " > entity", TEST_COUNT / 10, filteredValues.size()); > + assertEquals("Get first description value", "Description 0", > filteredValues.get(0).get("description")); > + > + filteredValues = EntityUtil.filterOutByCondition(newValues, > condition); > + assertEquals("Filter out on 10% description condition " + TEST_COUNT > + " entity", TEST_COUNT - TEST_COUNT / 10, filteredValues.size()); > + assertEquals("Get first description value", "Description 1", > filteredValues.get(0).get("description")); > + } > + > + public void testFilterByAnd() { > + List<GenericValue> newValues = prepareGenericValueList(); > + List<EntityExpr> condition = > UtilMisc.toList(EntityCondition.makeCondition("description", "Description 0"), > + EntityCondition.makeCondition("testingId", "00010")); > + List<GenericValue> filteredWithMap = > EntityUtil.filterByAnd(newValues, UtilMisc.toMap("description", "Description > 0", "testingId", "00010")); > + List<GenericValue> filteredWithCondition = > EntityUtil.filterByAnd(newValues, condition); > + assertEquals("Filter with same condition using Map and > List<EntityExpr> ", filteredWithCondition, filteredWithMap); > + > + condition = > UtilMisc.toList(EntityCondition.makeCondition("description", "Description 0"), > + EntityCondition.makeCondition("testingId", > EntityOperator.LIKE, "000%")); > + filteredWithCondition = EntityUtil.filterByAnd(newValues, condition); > + assertEquals("Filter condition using List<EntityExpr> must have 10 > results", 10, filteredWithCondition.size()); > + filteredWithCondition.forEach(genericValue -> > + assertEquals("Filter condition using List<EntityExpr> must > get simple description", > + "Description 0", genericValue.get("description"))); > + } > + > + public void testFilterByOr() { > + List<GenericValue> newValues = prepareGenericValueList(); > + List<EntityExpr> condition = > UtilMisc.toList(EntityCondition.makeCondition("description", "Description 0"), > + EntityCondition.makeCondition("testingId", "00001")); > + List<GenericValue> filteredWithCondition = > EntityUtil.filterByOr(newValues, condition); > + > + assertEquals("Filter condition using List<EntityExpr> must have " + > (TEST_COUNT / 10 + 1) + " results", > + TEST_COUNT / 10 + 1, filteredWithCondition.size()); > + } > +} > > Propchange: > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/test/EntityUtilTestSuite.java > ------------------------------------------------------------------------------ > svn:eol-style = native > > Propchange: > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/test/EntityUtilTestSuite.java > ------------------------------------------------------------------------------ > svn:keywords = Date Rev Author URL Id > > Propchange: > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/test/EntityUtilTestSuite.java > ------------------------------------------------------------------------------ > svn:mime-type = text/plain > > Modified: > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtil.java > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtil.java?rev=1839763&r1=1839762&r2=1839763&view=diff > ============================================================================== > --- > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtil.java > (original) > +++ > ofbiz/ofbiz-framework/trunk/framework/entity/src/main/java/org/apache/ofbiz/entity/util/EntityUtil.java > Fri Aug 31 15:32:02 2018 > @@ -32,6 +32,7 @@ import java.util.List; > import java.util.Locale; > import java.util.Map; > import java.util.Set; > +import java.util.stream.Stream; > > import org.apache.ofbiz.base.util.Debug; > import org.apache.ofbiz.base.util.UtilDateTime; > @@ -49,6 +50,8 @@ import org.apache.ofbiz.entity.condition > import org.apache.ofbiz.entity.condition.OrderByList; > import org.apache.ofbiz.entity.model.ModelField; > > +import static java.util.stream.Collectors.toList; > + > /** > * Helper methods when dealing with Entities, especially ones that follow > certain conventions > */ > @@ -265,21 +268,10 @@ public final class EntityUtil { > *@return List of GenericValue's that match the values in fields > */ > public static <T extends GenericEntity> List<T> filterByAnd(List<T> > values, Map<String, ? extends Object> fields) { > - if (values == null) return null; > - > - List<T> result = null; > - if (UtilValidate.isEmpty(fields)) { > - result = new LinkedList<T>(); > - result.addAll(values); > - } else { > - result = new LinkedList<T>(); > - for (T value: values) { > - if (value.matchesFields(fields)) { > - result.add(value); > - }// else did not match > - } > + if (values == null || UtilValidate.isEmpty(fields)) { > + return values; > } > - return result; > + return values.stream().filter(value -> > value.matchesFields(fields)).collect(toList()); > } > > /** > @@ -290,25 +282,13 @@ public final class EntityUtil { > *@return List of GenericValue's that match the values in fields > */ > public static <T extends GenericEntity> List<T> filterByAnd(List<T> > values, List<? extends EntityCondition> exprs) { > - if (values == null) return null; > - if (UtilValidate.isEmpty(exprs)) { > - // no constraints... oh well > + if (values == null || UtilValidate.isEmpty(exprs)) { > return values; > } > > - List<T> result = new LinkedList<T>(); > - for (T value: values) { > - boolean include = true; > - > - for (EntityCondition condition: exprs) { > - include = condition.entityMatches(value); > - if (!include) break; > - } > - if (include) { > - result.add(value); > - } > - } > - return result; > + return values.stream() > + .filter(value -> exprs.stream().allMatch(condition -> > condition.entityMatches(value))) > + .collect(toList()); > } > > /** > @@ -319,24 +299,13 @@ public final class EntityUtil { > *@return List of GenericValue's that match the values in fields > */ > public static <T extends GenericEntity> List<T> filterByOr(List<T> > values, List<? extends EntityCondition> exprs) { > - if (values == null) return null; > - if (UtilValidate.isEmpty(exprs)) { > + if (values == null || UtilValidate.isEmpty(exprs)) { > return values; > } > > - List<T> result = new LinkedList<T>(); > - for (T value: values) { > - boolean include = false; > - > - for (EntityCondition condition: exprs) { > - include = condition.entityMatches(value); > - if (include) break; > - } > - if (include) { > - result.add(value); > - } > - } > - return result; > + return values.stream() > + .filter(value -> exprs.stream().anyMatch(condition -> > condition.entityMatches(value))) > + .collect(toList()); > } > > /** > @@ -415,27 +384,17 @@ public final class EntityUtil { > } > > public static <T extends GenericEntity> List<T> > filterByCondition(List<T> values, EntityCondition condition) { > - if (values == null) return null; > - > - List<T> result = new LinkedList<T>(); > - for (T value: values) { > - if (condition.entityMatches(value)) { > - result.add(value); > - } > + if (values == null || UtilValidate.isEmpty(condition)) { > + return values; > } > - return result; > + return values.stream().filter(value -> > condition.entityMatches(value)).collect(toList()); > } > > public static <T extends GenericEntity> List<T> > filterOutByCondition(List<T> values, EntityCondition condition) { > - if (values == null) return null; > - > - List<T> result = new LinkedList<T>(); > - for (T value: values) { > - if (!condition.entityMatches(value)) { > - result.add(value); > - } > + if (values == null || UtilValidate.isEmpty(condition)) { > + return values; > } > - return result; > + return values.stream().filter(value -> > !condition.entityMatches(value)).collect(toList()); > } > > public static List<GenericValue> findDatedInclusionEntity(Delegator > delegator, String entityName, Map<String, ? extends Object> search) throws > GenericEntityException { > @@ -505,27 +464,13 @@ public final class EntityUtil { > if (genericValueList == null || fieldName == null) { > return null; > } > - List<T> fieldList = new LinkedList<T>(); > - Set<T> distinctSet = null; > - if (distinct) { > - distinctSet = new HashSet<T>(); > - } > > - for (GenericValue value: genericValueList) { > - T fieldValue = UtilGenerics.<T>cast(value.get(fieldName)); > - if (fieldValue != null) { > - if (distinct) { > - if (!distinctSet.contains(fieldValue)) { > - fieldList.add(fieldValue); > - distinctSet.add(fieldValue); > - } > - } else { > - fieldList.add(fieldValue); > - } > - } > + Stream<T> tStream = genericValueList.stream().map(genericValue -> > UtilGenerics.cast(genericValue.get(fieldName))); > + if (distinct) { > + return tStream.distinct().collect(toList()); > + } else { > + return tStream.collect(toList()); > } > - > - return fieldList; > } > > public static <T> List<T> > getFieldListFromEntityListIterator(EntityListIterator genericValueEli, String > fieldName, boolean distinct) { > > Modified: ofbiz/ofbiz-framework/trunk/framework/entity/testdef/entitytests.xml > URL: > http://svn.apache.org/viewvc/ofbiz/ofbiz-framework/trunk/framework/entity/testdef/entitytests.xml?rev=1839763&r1=1839762&r2=1839763&view=diff > ============================================================================== > --- ofbiz/ofbiz-framework/trunk/framework/entity/testdef/entitytests.xml > (original) > +++ ofbiz/ofbiz-framework/trunk/framework/entity/testdef/entitytests.xml Fri > Aug 31 15:32:02 2018 > @@ -22,6 +22,7 @@ under the License. > xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"; > > xsi:noNamespaceSchemaLocation="http://ofbiz.apache.org/dtds/test-suite.xsd";> > <test-case case-name="entity-tests"><junit-test-suite > class-name="org.apache.ofbiz.entity.test.EntityTestSuite"/></test-case> > + <test-case case-name="entity-util-tests"><junit-test-suite > class-name="org.apache.ofbiz.entity.test.EntityUtilTestSuite"/></test-case> > <test-case case-name="entity-crypto-tests"><junit-test-suite > class-name="org.apache.ofbiz.entity.test.EntityCryptoTestSuite"/></test-case> > <test-case case-name="entity-query-tests"><junit-test-suite > class-name="org.apache.ofbiz.entity.test.EntityQueryTestSuite"/></test-case> > <test-case case-name="entity-util-properties-tests"> > >
