Skip to content

Commit f4b6e03

Browse files
stariy95stariy95
committed
CAY-2916 SelectById: unify logic for null/empty values
1 parent ae0f310 commit f4b6e03

4 files changed

Lines changed: 166 additions & 16 deletions

File tree

‎RELEASE-NOTES.txt‎

Lines changed: 1 addition & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,7 @@ CAY-2893 Update velocity-engine-core dependency
2020
CAY-2897 Add no-op default implementations to the GraphChangeHandler interface
2121
CAY-2905 Upgrade Gradle to 8.14
2222
CAY-2908 Review and upgrade dependencies
23+
CAY-2916 SelectById: unify logic for null/empty values
2324

2425
Bug Fixes:
2526

‎cayenne/src/main/java/org/apache/cayenne/query/SelectById.java‎

Lines changed: 64 additions & 13 deletions
Original file line numberDiff line numberDiff line change
@@ -72,7 +72,7 @@ public static <T> SelectById<T> queryId(Class<T> entityType, Object id) {
7272
*/
7373
public static <T> SelectById<T> queryMap(Class<T> entityType, Map<String, ?> id) {
7474
QueryRoot root = new ByEntityTypeResolver(entityType);
75-
IdSpec idSpec = new SingleMapIdSpec(id);
75+
IdSpec idSpec = new SingleMapIdSpec(checkIdMap(id));
7676
return new SelectById<>(root, idSpec);
7777
}
7878

@@ -90,6 +90,9 @@ public static <T> SelectById<T> queryObjectId(Class<T> entityType, ObjectId id)
9090
* @since 5.0
9191
*/
9292
public static <T> SelectById<T> queryIds(Class<T> entityType, Object... ids) {
93+
if(ids == null) {
94+
throw new CayenneRuntimeException("Null ids");
95+
}
9396
QueryRoot root = new ByEntityTypeResolver(entityType);
9497
IdSpec idSpec = new MultiScalarIdSpec(Arrays.asList(ids));
9598
return new SelectById<>(root, idSpec);
@@ -99,6 +102,9 @@ public static <T> SelectById<T> queryIds(Class<T> entityType, Object... ids) {
99102
* @since 5.0
100103
*/
101104
public static <T> SelectById<T> queryIdsCollection(Class<T> entityType, Collection<Object> ids) {
105+
if(ids == null) {
106+
throw new CayenneRuntimeException("Null ids");
107+
}
102108
QueryRoot root = new ByEntityTypeResolver(entityType);
103109
IdSpec idSpec = new MultiScalarIdSpec(ids);
104110
return new SelectById<>(root, idSpec);
@@ -109,6 +115,14 @@ public static <T> SelectById<T> queryIdsCollection(Class<T> entityType, Collecti
109115
*/
110116
@SafeVarargs
111117
public static <T> SelectById<T> queryMaps(Class<T> entityType, Map<String, ?>... ids) {
118+
if(ids == null) {
119+
throw new CayenneRuntimeException("Null ids");
120+
}
121+
122+
for(Map<String, ?> id : ids) {
123+
checkIdMap(id);
124+
}
125+
112126
QueryRoot root = new ByEntityTypeResolver(entityType);
113127
IdSpec idSpec = MultiMapIdSpec.ofMap(ids);
114128
return new SelectById<>(root, idSpec);
@@ -118,6 +132,11 @@ public static <T> SelectById<T> queryMaps(Class<T> entityType, Map<String, ?>...
118132
* @since 5.0
119133
*/
120134
public static <T> SelectById<T> queryMapsCollection(Class<T> entityType, Collection<Map<String, ?>> ids) {
135+
if(ids == null) {
136+
throw new CayenneRuntimeException("Null ids");
137+
}
138+
139+
ids.forEach(SelectById::checkIdMap);
121140
QueryRoot root = new ByEntityTypeResolver(entityType);
122141
IdSpec idSpec = MultiMapIdSpec.ofMapCollection(ids);
123142
return new SelectById<>(root, idSpec);
@@ -127,8 +146,8 @@ public static <T> SelectById<T> queryMapsCollection(Class<T> entityType, Collect
127146
* @since 5.0
128147
*/
129148
public static <T> SelectById<T> queryObjectIds(Class<T> entityType, ObjectId... ids) {
130-
if(ids == null || ids.length == 0) {
131-
throw new CayenneRuntimeException("Null or empty ids");
149+
if(ids == null) {
150+
throw new CayenneRuntimeException("Null ids");
132151
}
133152
String entityName = ids[0].getEntityName();
134153
for(ObjectId id : ids) {
@@ -144,8 +163,8 @@ public static <T> SelectById<T> queryObjectIds(Class<T> entityType, ObjectId...
144163
* @since 5.0
145164
*/
146165
public static <T> SelectById<T> queryObjectIdsCollection(Class<T> entityType, Collection<ObjectId> ids) {
147-
if(ids == null || ids.isEmpty()) {
148-
throw new CayenneRuntimeException("Null or empty ids");
166+
if(ids == null) {
167+
throw new CayenneRuntimeException("Null ids");
149168
}
150169
String entityName = ids.iterator().next().getEntityName();
151170
for(ObjectId id : ids) {
@@ -253,7 +272,7 @@ public static SelectById<DataRow> dataRowQueryId(Class<?> entityType, Object id)
253272
*/
254273
public static SelectById<DataRow> dataRowQueryMap(Class<?> entityType, Map<String, ?> id) {
255274
QueryRoot root = new ByEntityTypeResolver(entityType);
256-
IdSpec idSpec = new SingleMapIdSpec(id);
275+
IdSpec idSpec = new SingleMapIdSpec(checkIdMap(id));
257276
return new SelectById<>(root, idSpec, true);
258277
}
259278

@@ -271,6 +290,9 @@ public static SelectById<DataRow> dataRowQueryObjectId(ObjectId id) {
271290
* @since 5.0
272291
*/
273292
public static SelectById<DataRow> dataRowQueryIds(Class<?> entityType, Object... ids) {
293+
if(ids == null) {
294+
throw new CayenneRuntimeException("Null ids");
295+
}
274296
QueryRoot root = new ByEntityTypeResolver(entityType);
275297
IdSpec idSpec = new MultiScalarIdSpec(Arrays.asList(ids));
276298
return new SelectById<>(root, idSpec, true);
@@ -280,6 +302,9 @@ public static SelectById<DataRow> dataRowQueryIds(Class<?> entityType, Object...
280302
* @since 5.0
281303
*/
282304
public static SelectById<DataRow> dataRowQueryIdsCollection(Class<?> entityType, Collection<Object> ids) {
305+
if(ids == null) {
306+
throw new CayenneRuntimeException("Null ids");
307+
}
283308
QueryRoot root = new ByEntityTypeResolver(entityType);
284309
IdSpec idSpec = new MultiScalarIdSpec(ids);
285310
return new SelectById<>(root, idSpec, true);
@@ -290,6 +315,14 @@ public static SelectById<DataRow> dataRowQueryIdsCollection(Class<?> entityType,
290315
*/
291316
@SafeVarargs
292317
public static SelectById<DataRow> dataRowQueryMaps(Class<?> entityType, Map<String, ?>... ids) {
318+
if(ids == null) {
319+
throw new CayenneRuntimeException("Null ids");
320+
}
321+
322+
for(Map<String, ?> id : ids) {
323+
checkIdMap(id);
324+
}
325+
293326
QueryRoot root = new ByEntityTypeResolver(entityType);
294327
IdSpec idSpec = MultiMapIdSpec.ofMap(ids);
295328
return new SelectById<>(root, idSpec, true);
@@ -299,6 +332,12 @@ public static SelectById<DataRow> dataRowQueryMaps(Class<?> entityType, Map<Stri
299332
* @since 5.0
300333
*/
301334
public static SelectById<DataRow> dataRowQueryMapsCollection(Class<?> entityType, Collection<Map<String, ?>> ids) {
335+
if(ids == null) {
336+
throw new CayenneRuntimeException("Null ids");
337+
}
338+
339+
ids.forEach(SelectById::checkIdMap);
340+
302341
QueryRoot root = new ByEntityTypeResolver(entityType);
303342
IdSpec idSpec = MultiMapIdSpec.ofMapCollection(ids);
304343
return new SelectById<>(root, idSpec, true);
@@ -308,8 +347,8 @@ public static SelectById<DataRow> dataRowQueryMapsCollection(Class<?> entityType
308347
* @since 5.0
309348
*/
310349
public static SelectById<DataRow> dataRowQueryObjectIds(ObjectId... ids) {
311-
if(ids == null || ids.length == 0) {
312-
throw new CayenneRuntimeException("Null or empty ids");
350+
if(ids == null) {
351+
throw new CayenneRuntimeException("Null ids");
313352
}
314353
String entityName = ids[0].getEntityName();
315354
for(ObjectId id : ids) {
@@ -325,8 +364,8 @@ public static SelectById<DataRow> dataRowQueryObjectIds(ObjectId... ids) {
325364
* @since 5.0
326365
*/
327366
public static SelectById<DataRow> dataRowQueryObjectIdsCollection(Collection<ObjectId> ids) {
328-
if(ids == null || ids.isEmpty()) {
329-
throw new CayenneRuntimeException("Null or empty ids");
367+
if(ids == null) {
368+
throw new CayenneRuntimeException("Null ids");
330369
}
331370
String entityName = ids.iterator().next().getEntityName();
332371
for(ObjectId id : ids) {
@@ -603,6 +642,14 @@ private static void checkObjectId(ObjectId id, String entityName) {
603642
}
604643
}
605644

645+
private static Map<String, ?> checkIdMap(Map<String, ?> id) {
646+
if(id == null || id.isEmpty()) {
647+
throw new CayenneRuntimeException("Null or empty id map");
648+
}
649+
650+
return id;
651+
}
652+
606653
@SafeVarargs
607654
private static <E, R> Collection<R> foldArguments(Function<E, R> mapper, E first, E... other) {
608655
List<R> result = new ArrayList<>(1 + other.length);
@@ -680,7 +727,8 @@ protected SingleMapIdSpec(Map<String, ?> id) {
680727

681728
@Override
682729
public Expression getQualifier(ObjEntity entity) {
683-
return matchAllDbExp(id, Expression.EQUAL_TO);
730+
Expression expression = matchAllDbExp(id, Expression.EQUAL_TO);
731+
return expression == null ? expFalse() : expression;
684732
}
685733
}
686734

@@ -722,10 +770,13 @@ protected MultiMapIdSpec(Collection<Map<String, ?>> ids) {
722770
public Expression getQualifier(ObjEntity entity) {
723771
List<Expression> expressions = new ArrayList<>();
724772
for(Map<String, ?> id : ids) {
725-
expressions.add(matchAllDbExp(id, Expression.EQUAL_TO));
773+
Expression expression = matchAllDbExp(id, Expression.EQUAL_TO);
774+
if(expression != null) {
775+
expressions.add(expression);
776+
}
726777
}
727778

728-
return or(expressions);
779+
return expressions.isEmpty() ? expFalse() : or(expressions);
729780
}
730781
}
731782

‎cayenne/src/test/java/org/apache/cayenne/query/SelectByIdTest.java‎

Lines changed: 41 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -20,6 +20,9 @@
2020

2121
import static org.junit.Assert.assertNotNull;
2222

23+
import java.util.Collections;
24+
25+
import org.apache.cayenne.CayenneRuntimeException;
2326
import org.apache.cayenne.testdo.testmap.Artist;
2427
import org.junit.Test;
2528

@@ -30,7 +33,7 @@ public void testPrefetch() {
3033

3134
PrefetchTreeNode root = PrefetchTreeNode.withPath("a.b", PrefetchTreeNode.JOINT_PREFETCH_SEMANTICS);
3235

33-
SelectById<Artist> q = SelectById.query(Artist.class, 6);
36+
SelectById<Artist> q = SelectById.queryId(Artist.class, 6);
3437
q.prefetch(root);
3538

3639
PrefetchTreeNode prefetch = q.getPrefetches();
@@ -42,7 +45,7 @@ public void testPrefetch() {
4245
@Test
4346
public void testPrefetch_Path() {
4447

45-
SelectById<Artist> q = SelectById.query(Artist.class, 7);
48+
SelectById<Artist> q = SelectById.queryId(Artist.class, 7);
4649
q.prefetch("a.b", PrefetchTreeNode.DISJOINT_PREFETCH_SEMANTICS);
4750
PrefetchTreeNode prefetch = q.getPrefetches();
4851

@@ -62,7 +65,7 @@ public void testPrefetch_Subroot() {
6265

6366
PrefetchTreeNode root = PrefetchTreeNode.withPath("a.b", PrefetchTreeNode.JOINT_PREFETCH_SEMANTICS);
6467

65-
SelectById<Artist> q = SelectById.query(Artist.class, 8);
68+
SelectById<Artist> q = SelectById.queryId(Artist.class, 8);
6669
q.prefetch(root);
6770

6871
PrefetchTreeNode prefetch = q.getPrefetches();
@@ -77,4 +80,39 @@ public void testPrefetch_Subroot() {
7780
assertNotNull(prefetch.getNode("a.b"));
7881
assertNotNull(prefetch.getNode("a.b.c"));
7982
}
83+
84+
@Test
85+
public void testQueryId_NullId() {
86+
assertNotNull(SelectById.queryId(Artist.class, null));
87+
}
88+
89+
@Test
90+
public void testDataRowQueryId_NullId() {
91+
assertNotNull(SelectById.dataRowQueryId(Artist.class, null));
92+
}
93+
94+
@Test
95+
public void testDataRowQueryIds_EmptyVarargs() {
96+
assertNotNull(SelectById.dataRowQueryIds(Artist.class));
97+
}
98+
99+
@Test
100+
public void testDataRowQueryIdsCollection_EmptyCollection() {
101+
assertNotNull(SelectById.dataRowQueryIdsCollection(Artist.class, Collections.emptyList()));
102+
}
103+
104+
@Test(expected = CayenneRuntimeException.class)
105+
public void testDataRowQueryMap_EmptyMap() {
106+
SelectById.dataRowQueryMap(Artist.class, Collections.emptyMap());
107+
}
108+
109+
@Test
110+
public void testDataRowQueryMaps_EmptyVarargs() {
111+
assertNotNull(SelectById.dataRowQueryMaps(Artist.class));
112+
}
113+
114+
@Test
115+
public void testDataRowQueryMapsCollection_EmptyCollection() {
116+
assertNotNull(SelectById.dataRowQueryMapsCollection(Artist.class, Collections.emptyList()));
117+
}
80118
}

‎cayenne/src/test/java/org/apache/cayenne/query/SelectById_RunIT.java‎

Lines changed: 60 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -88,6 +88,66 @@ public void testIntPk() throws Exception {
8888
assertEquals("artist2", a2.getArtistName());
8989
}
9090

91+
@Test
92+
public void testNullPk() {
93+
List<Artist> artists = SelectById.queryId(Artist.class, null).select(context);
94+
assertEquals(0, artists.size());
95+
}
96+
97+
@Test
98+
public void testDataRowNullPk() {
99+
List<DataRow> artists = SelectById.dataRowQueryId(Artist.class, null).select(context);
100+
assertEquals(0, artists.size());
101+
}
102+
103+
@Test
104+
public void testEmptyPkMulti() {
105+
List<Artist> artists = SelectById.queryIds(Artist.class).select(context);
106+
assertEquals(0, artists.size());
107+
}
108+
109+
@Test
110+
public void testEmptyPkCollection() {
111+
List<Artist> artists = SelectById.queryIdsCollection(Artist.class, Collections.emptyList()).select(context);
112+
assertEquals(0, artists.size());
113+
}
114+
115+
@Test
116+
public void testEmptyMapPkMulti() {
117+
List<Artist> artists = SelectById.queryMaps(Artist.class).select(context);
118+
assertEquals(0, artists.size());
119+
}
120+
121+
@Test
122+
public void testEmptyMapPkCollection() {
123+
List<Artist> artists = SelectById.queryMapsCollection(Artist.class, Collections.emptyList()).select(context);
124+
assertEquals(0, artists.size());
125+
}
126+
127+
@Test
128+
public void testDataRowEmptyPkMulti() {
129+
List<DataRow> artists = SelectById.dataRowQueryIds(Artist.class).select(context);
130+
assertEquals(0, artists.size());
131+
}
132+
133+
@Test
134+
public void testDataRowEmptyPkCollection() {
135+
List<DataRow> artists = SelectById.dataRowQueryIdsCollection(Artist.class, Collections.emptyList()).select(context);
136+
assertEquals(0, artists.size());
137+
}
138+
139+
@Test
140+
public void testDataRowEmptyMapPkMulti() {
141+
List<DataRow> artists = SelectById.dataRowQueryMaps(Artist.class).select(context);
142+
assertEquals(0, artists.size());
143+
}
144+
145+
@Test
146+
public void testDataRowEmptyMapPkCollection() {
147+
List<DataRow> artists = SelectById.dataRowQueryMapsCollection(Artist.class, Collections.emptyList()).select(context);
148+
assertEquals(0, artists.size());
149+
}
150+
91151
@Test
92152
public void testIntPkMulti() throws Exception {
93153
createTwoArtists();

0 commit comments

Comments
 (0)