Hello Taher ! I'll be a little bit more verbose in commit log next times :), and for the tStream, it was a miss, i'll fix it soon.
Thanks as always for your review ! Gil Le samedi 01 sept. 2018 à 21:24:56 (+0300), Taher Alkhateeb a écrit : > 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"> > > > >
