From 56e960fce9b8dd7e15745481d5f356443d3e93f5 Mon Sep 17 00:00:00 2001 From: Zach Hein <31706984+Zech-Hein@users.noreply.github.com> Date: Mon, 18 Nov 2024 08:12:04 -0600 Subject: [PATCH] Revert "empty list of entities" --- .../resources/CWWKDMessages.nlsprops | 35 +------- .../data/internal/persistence/QueryInfo.java | 89 +++++-------------- .../internal/persistence/RepositoryImpl.java | 16 +--- .../data/internal/persistence/Util.java | 38 -------- .../jakarta/data/web/DataTestServlet.java | 2 + .../errpaths/web/DataErrPathsTestServlet.java | 69 -------------- .../jakarta/data/errpaths/web/Voters.java | 5 -- .../test/jakarta/data/jpa/DataJPATest.java | 4 +- 8 files changed, 30 insertions(+), 228 deletions(-) diff --git a/dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops b/dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops index b368720c1c94..8dbac31d7401 100644 --- a/dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops +++ b/dev/io.openliberty.data.internal.persistence/resources/io/openliberty/data/internal/persistence/resources/CWWKDMessages.nlsprops @@ -125,9 +125,9 @@ CWWKD1009.lifecycle.param.err.explanation=Lifecycle methods must have a single \ CWWKD1009.lifecycle.param.err.useraction=Update the repository method to have a \ single parameter that is an entity or an array of List of the entity. -CWWKD1010.unknown.entity.prop=CWWKD1010E: An entity property named {0} is \ - not found on the {1} entity class for the {2} method of the {3} repository \ - interface. Valid property names for the entity are: {4}. +CWWKD1010.unknown.entity.prop=CWWKD1010E: The {0} method of the {1} repository \ + interface expects an entity property named {2} that is not found on the \ + {3} entity class. Valid property names for the entity are: {4}. CWWKD1010.unknown.entity.prop.explanation=The repository method is attempting \ to use an entity property that does not exist. CWWKD1010.unknown.entity.prop.useraction=Update the repository method to match \ @@ -908,32 +908,3 @@ CWWKD1090.orderby.conflict.explanation=The OrderBy annotation is not compatible with the OrderBy keyword. CWWKD1090.orderby.conflict.useraction=Use only the OrderBy annotation or the \ OrderBy keyword. - -CWWKD1091.method.name.parse.err=CWWKD1091E: An entity property named {0} is \ - not found on the {1} entity class for the {2} method of the {3} repository \ - interface. Confirm that the repository method either has a name that conforms \ - to the Query by Method Name pattern or is annotated with one of: {4}. \ - Valid property names for the entity are: {5}. -CWWKD1091.method.name.parse.err.explanation=The repository method name was parsed \ - according to the Query by Method Name pattern because the repository method \ - does not have a Jakarta Data annotation that indicates the type of operation. -CWWKD1091.method.name.parse.err.useraction=Correct the repository method name \ - or annotate the repository method to indicate a Jakarta Data operation such as \ - Query, Find, or Save. - -CWWKD1092.lifecycle.arg.empty=CWWKD1092E: The {0} method of the {1} repository \ - does not accept an empty {2} parameter. The parameter value must contain at \ - least one entity. -CWWKD1092.lifecycle.arg.empty.explanation=Insert, Update, Save, and Delete methods - require at least one entity. -CWWKD1092.lifecycle.arg.empty.useraction=Ensure that the array, List, or Stream \ - that is supplied to the method contains at least one entity. - -CWWKD1093.fn.not.applicable=CWWKD1093E: The {0} function cannot be evaluated \ - for the {1} entity that is used by the {2} method of the {3} repository \ - interface. The entity does not have a property that is annotated with {4} \ - or from which {0} can be inferred. -CWWKD1093.fn.not.applicable.explanation=The function cannot be used on the entity \ - because the entity lacks an ID or version property. -CWWKD1093.fn.not.applicable.useraction=Do not use the function in query language \ - or parameters for the repository method. diff --git a/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/QueryInfo.java b/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/QueryInfo.java index cf33d7abc111..e2be4a181217 100644 --- a/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/QueryInfo.java +++ b/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/QueryInfo.java @@ -1163,14 +1163,6 @@ Object findAndUpdate(Object arg, EntityManager em) throws Exception { results.add(findAndUpdateOne(arg, em)); } } - - if (results.isEmpty()) - throw exc(IllegalArgumentException.class, - "CWWKD1092.lifecycle.arg.empty", - method.getName(), - repositoryInterface.getName(), - method.getGenericParameterTypes()[0].getTypeName()); - em.flush(); Class returnType = method.getReturnType(); @@ -2464,23 +2456,15 @@ String getAttributeName(String name, boolean failIfNotFound) { // id(this) attributeName = entityInfo.attributeNames.get(By.ID); if (attributeName == null && failIfNotFound) - throw exc(UnsupportedOperationException.class, - "CWWKD1093.fn.not.applicable", - name, - entityInfo.getType().getName(), - method.getName(), - repositoryInterface.getName(), - "@Id"); + throw new MappingException("Entity class " + entityInfo.getType().getName() + + " does not have a property named " + name + + " or which is designated as the @Id."); // TODO NLS } else if (len == 13 && name.regionMatches(true, 0, "version", 0, 7)) { // version(this) if (entityInfo.versionAttributeName == null && failIfNotFound) - throw exc(UnsupportedOperationException.class, - "CWWKD1093.fn.not.applicable", - name, - entityInfo.getType().getName(), - method.getName(), - repositoryInterface.getName(), - "@Version"); + throw new MappingException("Entity class " + entityInfo.getType().getName() + + " does not have a property named " + name + + " or which is designated as the @Version."); // TODO NLS else attributeName = entityInfo.versionAttributeName; } else { @@ -2495,10 +2479,10 @@ String getAttributeName(String name, boolean failIfNotFound) { else throw exc(MappingException.class, "CWWKD1010.unknown.entity.prop", - name, - entityInfo.getType().getName(), method.getName(), repositoryInterface.getName(), + name, + entityInfo.getType().getName(), entityInfo.attributeTypes.keySet()); } else { String lowerName = name.toLowerCase(); @@ -2520,23 +2504,15 @@ String getAttributeName(String name, boolean failIfNotFound) { lowerName = lowerName.replace("_", ""); attributeName = entityInfo.attributeNames.get(lowerName); if (attributeName == null && failIfNotFound) { - if (Util.hasOperationAnno(method)) - throw exc(MappingException.class, - "CWWKD1010.unknown.entity.prop", - name, - entityInfo.getType().getName(), - method.getName(), - repositoryInterface.getName(), - entityInfo.attributeTypes.keySet()); - else - throw exc(MappingException.class, - "CWWKD1091.method.name.parse.err", - name, - entityInfo.getType().getName(), - method.getName(), - repositoryInterface.getName(), - Util.OP_ANNOS, - entityInfo.attributeTypes.keySet()); + // TODO If attempting to parse Query by Method Name without a By keyword, then the message + // should also include the possibility that repository method is missing an annotation. + throw exc(MappingException.class, + "CWWKD1010.unknown.entity.prop", + method.getName(), + repositoryInterface.getName(), + name, + entityInfo.getType().getName(), + entityInfo.attributeTypes.keySet()); } } } @@ -3525,12 +3501,11 @@ Object insert(Object arg, EntityManager em) throws Exception { ArrayList results; boolean hasSingularEntityParam = false; - int entityCount = 0; if (entityParamType.isArray()) { int length = Array.getLength(arg); results = resultVoid ? null : new ArrayList<>(length); - for (; entityCount < length; entityCount++) { - Object entity = toEntity(Array.get(arg, entityCount)); + for (int i = 0; i < length; i++) { + Object entity = toEntity(Array.get(arg, i)); em.persist(entity); if (results != null) results.add(entity); @@ -3544,7 +3519,6 @@ Object insert(Object arg, EntityManager em) throws Exception { if (arg instanceof Iterable) { results = resultVoid ? null : new ArrayList<>(); for (Object e : ((Iterable) arg)) { - entityCount++; Object entity = toEntity(e); em.persist(entity); if (results != null) @@ -3552,7 +3526,6 @@ Object insert(Object arg, EntityManager em) throws Exception { } em.flush(); } else { - entityCount = 1; hasSingularEntityParam = true; results = resultVoid ? null : new ArrayList<>(1); Object entity = toEntity(arg); @@ -3563,13 +3536,6 @@ Object insert(Object arg, EntityManager em) throws Exception { } } - if (entityCount == 0) - throw exc(IllegalArgumentException.class, - "CWWKD1092.lifecycle.arg.empty", - method.getName(), - repositoryInterface.getName(), - method.getGenericParameterTypes()[0].getTypeName()); - Class returnType = method.getReturnType(); Object returnValue; if (resultVoid) { @@ -4032,12 +3998,11 @@ Object save(Object arg, EntityManager em) throws Exception { List results; boolean hasSingularEntityParam = false; - int entityCount = 0; if (entityParamType.isArray()) { results = new ArrayList<>(); int length = Array.getLength(arg); - for (; entityCount < length; entityCount++) - results.add(em.merge(toEntity(Array.get(arg, entityCount)))); + for (int i = 0; i < length; i++) + results.add(em.merge(toEntity(Array.get(arg, i)))); em.flush(); } else { arg = arg instanceof Stream // @@ -4046,13 +4011,10 @@ Object save(Object arg, EntityManager em) throws Exception { if (Iterable.class.isAssignableFrom(entityParamType)) { results = new ArrayList<>(); - for (Object e : ((Iterable) arg)) { - entityCount++; + for (Object e : ((Iterable) arg)) results.add(em.merge(toEntity(e))); - } em.flush(); } else { - entityCount = 1; hasSingularEntityParam = true; results = resultVoid ? null : new ArrayList<>(1); Object entity = em.merge(toEntity(arg)); @@ -4062,13 +4024,6 @@ Object save(Object arg, EntityManager em) throws Exception { } } - if (entityCount == 0) - throw exc(IllegalArgumentException.class, - "CWWKD1092.lifecycle.arg.empty", - method.getName(), - repositoryInterface.getName(), - method.getGenericParameterTypes()[0].getTypeName()); - Class returnType = method.getReturnType(); Object returnValue; if (resultVoid) { diff --git a/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/RepositoryImpl.java b/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/RepositoryImpl.java index c19b9d585af4..b41f05a04914 100644 --- a/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/RepositoryImpl.java +++ b/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/RepositoryImpl.java @@ -847,7 +847,7 @@ else if (DoubleStream.class.equals(multiType)) queryInfo.returnArrayType.isInstance(firstNonNullResult) || queryInfo.returnArrayType.isPrimitive() && Util.isWrapperClassFor(queryInfo.returnArrayType, - firstNonNullResult.getClass())) { + firstNonNullResult.getClass())) { returnValue = Array.newInstance(queryInfo.returnArrayType, size); int i = 0; for (Object result : results) @@ -972,13 +972,6 @@ else if (DoubleStream.class.equals(multiType)) updateCount = queryInfo.remove(arg, em); } - if (numExpected == 0) - throw exc(IllegalArgumentException.class, - "CWWKD1091.lifecycle.arg.empty", - method.getName(), - repositoryInterface.getName(), - method.getGenericParameterTypes()[0].getTypeName()); - if (updateCount < numExpected) if (numExpected == 1) throw exc(OptimisticLockingFailureException.class, @@ -1023,13 +1016,6 @@ else if (DoubleStream.class.equals(multiType)) updateCount = queryInfo.update(arg, em); } - if (numExpected == 0) - throw exc(IllegalArgumentException.class, - "CWWKD1092.lifecycle.arg.empty", - method.getName(), - repositoryInterface.getName(), - method.getGenericParameterTypes()[0].getTypeName()); - if (updateCount < numExpected) if (numExpected == 1) throw exc(OptimisticLockingFailureException.class, diff --git a/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/Util.java b/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/Util.java index b052a9a1c2cf..55a7b56b1945 100644 --- a/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/Util.java +++ b/dev/io.openliberty.data.internal.persistence/src/io/openliberty/data/internal/persistence/Util.java @@ -9,8 +9,6 @@ *******************************************************************************/ package io.openliberty.data.internal.persistence; -import java.lang.annotation.Annotation; -import java.lang.reflect.Method; import java.lang.reflect.Modifier; import java.math.BigDecimal; import java.math.BigInteger; @@ -29,12 +27,6 @@ import jakarta.data.Order; import jakarta.data.Sort; import jakarta.data.page.PageRequest; -import jakarta.data.repository.Delete; -import jakarta.data.repository.Find; -import jakarta.data.repository.Insert; -import jakarta.data.repository.Query; -import jakarta.data.repository.Save; -import jakarta.data.repository.Update; /** * A location for helper methods that do not require any state. @@ -52,11 +44,6 @@ public class Util { Set.of(long.class, int.class, short.class, byte.class, double.class, float.class); - /** - * List of Jakarta Data operation annotations for use in NLS messages. - */ - static final String OP_ANNOS = "Delete, Find, Insert, Query, Save, Update"; - /** * Return types for deleteBy that distinguish delete-only from find-and-delete. */ @@ -133,31 +120,6 @@ public static boolean cannotBeEntity(Class c) { Modifier.isAbstract(modifiers); } - /** - * Identifies whether a method is annotated with a Jakarta Data annotation - * that performs and operation, such as Query, Find, or Save. This method is - * for use by error reporting only, so it does not need to be very efficient. - * - * @param method repository method. - * @return if the repository method has an annotation indicating an operation. - */ - @Trivial - static final boolean hasOperationAnno(Method method) { - Set> operationAnnos = // - Set.of(Delete.class, - Insert.class, - Find.class, - Query.class, - Save.class, - Update.class); - - for (Annotation anno : method.getAnnotations()) - if (operationAnnos.contains(anno.annotationType())) - return true; - - return false; - } - /** * Indicates if the specified class is a wrapper for the primitive class. * diff --git a/dev/io.openliberty.data.internal_fat/test-applications/DataTestApp/src/test/jakarta/data/web/DataTestServlet.java b/dev/io.openliberty.data.internal_fat/test-applications/DataTestApp/src/test/jakarta/data/web/DataTestServlet.java index 402484a6cc10..e55fb5407b51 100644 --- a/dev/io.openliberty.data.internal_fat/test-applications/DataTestApp/src/test/jakarta/data/web/DataTestServlet.java +++ b/dev/io.openliberty.data.internal_fat/test-applications/DataTestApp/src/test/jakarta/data/web/DataTestServlet.java @@ -5940,6 +5940,8 @@ public void testUpdateWithEntityParameter() { assertEquals(3, persons.updateSome(ulysses, ursula, uriah)); + assertEquals(0, persons.updateSome()); + assertEquals(4, people.deleteBySSN_IdBetween(0L, 999999999L)); } diff --git a/dev/io.openliberty.data.internal_fat_errorpaths/test-applications/DataErrPathsTestApp/src/test/jakarta/data/errpaths/web/DataErrPathsTestServlet.java b/dev/io.openliberty.data.internal_fat_errorpaths/test-applications/DataErrPathsTestApp/src/test/jakarta/data/errpaths/web/DataErrPathsTestServlet.java index a418454e180b..3aaa01f56f9b 100644 --- a/dev/io.openliberty.data.internal_fat_errorpaths/test-applications/DataErrPathsTestApp/src/test/jakarta/data/errpaths/web/DataErrPathsTestServlet.java +++ b/dev/io.openliberty.data.internal_fat_errorpaths/test-applications/DataErrPathsTestApp/src/test/jakarta/data/errpaths/web/DataErrPathsTestServlet.java @@ -19,7 +19,6 @@ import java.util.List; import java.util.Optional; import java.util.concurrent.CompletionException; -import java.util.stream.Stream; import jakarta.annotation.Resource; import jakarta.annotation.sql.DataSourceDefinition; @@ -178,74 +177,6 @@ public void testDuplicateNamedParam() { } } - /** - * Verify IllegalArgumentException is raised if you attempt to delete an - * empty list of entities. - */ - @Test - public void testEmptyDelete() { - try { - voters.deleteAll(List.of()); - fail("Deleted an empty list of entities."); - } catch (IllegalArgumentException x) { - if (x.getMessage() == null || - !x.getMessage().startsWith("CWWKD1092E:") || - !x.getMessage().contains("deleteAll")) - throw x; - } - } - - /** - * Verify IllegalArgumentException is raised if you attempt to delete an - * empty array of entities. - */ - @Test - public void testEmptyInsert() { - try { - voters.register(new Voter[] {}); - fail("Inserted an empty array of entities."); - } catch (IllegalArgumentException x) { - if (x.getMessage() == null || - !x.getMessage().startsWith("CWWKD1092E:") || - !x.getMessage().contains("register")) - throw x; - } - } - - /** - * Verify IllegalArgumentException is raised if you attempt to save an - * empty list of entities. - */ - @Test - public void testEmptySave() { - try { - List saved = voters.saveAll(List.of()); - fail("Saved an empty list of entities. Result: " + saved); - } catch (IllegalArgumentException x) { - if (x.getMessage() == null || - !x.getMessage().startsWith("CWWKD1092E:") || - !x.getMessage().contains("saveAll")) - throw x; - } - } - - /** - * Verify IllegalArgumentException is raised if you attempt to update an - * empty stream of entities. - */ - @Test - public void testEmptyUpdate() { - try { - List updated = voters.changeAll(Stream.of()); - fail("Updated an empty stream of entities. Result: " + updated); - } catch (IllegalArgumentException x) { - if (x.getMessage() == null || - !x.getMessage().startsWith("CWWKD1092E:") || - !x.getMessage().contains("changeAll")) - throw x; - } - } - /** * Verify an error is raised for a repository method that has extra Param * annotations that do not correspond to any named parameters in the query. diff --git a/dev/io.openliberty.data.internal_fat_errorpaths/test-applications/DataErrPathsTestApp/src/test/jakarta/data/errpaths/web/Voters.java b/dev/io.openliberty.data.internal_fat_errorpaths/test-applications/DataErrPathsTestApp/src/test/jakarta/data/errpaths/web/Voters.java index b25c4e0bd934..9e656840e5e7 100644 --- a/dev/io.openliberty.data.internal_fat_errorpaths/test-applications/DataErrPathsTestApp/src/test/jakarta/data/errpaths/web/Voters.java +++ b/dev/io.openliberty.data.internal_fat_errorpaths/test-applications/DataErrPathsTestApp/src/test/jakarta/data/errpaths/web/Voters.java @@ -14,7 +14,6 @@ import java.time.Month; import java.util.List; -import java.util.stream.Stream; import jakarta.data.Sort; import jakarta.data.repository.BasicRepository; @@ -23,7 +22,6 @@ import jakarta.data.repository.Param; import jakarta.data.repository.Query; import jakarta.data.repository.Repository; -import jakarta.data.repository.Update; /** * Repository with a valid entity. @@ -62,9 +60,6 @@ List bornOn(@Param("year") int yearBorn, @Param("month") int monthNum, // duplicate parameter name @Param("day") int dayBorn); - @Update - List changeAll(Stream v); - /** * This invalid method has a conflict between its OrderBy annotation and * method name keyword. diff --git a/dev/io.openliberty.data.internal_fat_jpa/fat/src/test/jakarta/data/jpa/DataJPATest.java b/dev/io.openliberty.data.internal_fat_jpa/fat/src/test/jakarta/data/jpa/DataJPATest.java index 860443566a4e..644962365692 100644 --- a/dev/io.openliberty.data.internal_fat_jpa/fat/src/test/jakarta/data/jpa/DataJPATest.java +++ b/dev/io.openliberty.data.internal_fat_jpa/fat/src/test/jakarta/data/jpa/DataJPATest.java @@ -42,6 +42,7 @@ public class DataJPATest extends FATServletClient { */ static final String[] EXPECTED_ERROR_MESSAGES = // new String[] { + "CWWKD1010E.*countBySurgePriceGreaterThanEqual", "CWWKD1026E.*allSorted", "CWWKD1046E.*publicDebtAsByte", "CWWKD1046E.*publicDebtAsDouble", @@ -54,8 +55,7 @@ public class DataJPATest extends FATServletClient { "CWWKD1046E.*numFullTimeWorkersAsShort", "CWWKD1054E.*findByIsControlTrueAndNumericValueBetween", "CWWKD1075E.*Apartment2", - "CWWKD1075E.*Apartment3", - "CWWKD1091E.*countBySurgePriceGreaterThanEqual" + "CWWKD1075E.*Apartment3" }; @ClassRule