Skip to content

Commit c88c132

Browse files
greenrobot-teamgreenrobot-team
committed
Query: throw helpful error if using closed query (#818)
1 parent 875fcde commit c88c132

File tree

3 files changed

+132
-1
lines changed

3 files changed

+132
-1
lines changed

‎objectbox-java/src/main/java/io/objectbox/query/Query.java‎

Lines changed: 43 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -135,7 +135,12 @@ protected void finalize() throws Throwable {
135135
}
136136

137137
/**
138-
* If possible, try to close the query once you are done with it to reclaim resources immediately.
138+
* Closes this query and frees used resources.
139+
* <p>
140+
* If possible, call this always once done with this. Otherwise, will be called once this is finalized (e.g. garbage
141+
* collected).
142+
* <p>
143+
* Calling any other methods of this afterwards will throw an exception.
139144
*/
140145
public synchronized void close() {
141146
if (handle != 0) {
@@ -256,6 +261,7 @@ public long[] findIds() {
256261
*/
257262
@Nonnull
258263
public long[] findIds(final long offset, final long limit) {
264+
checkOpen();
259265
return box.internalCallWithReaderHandle(cursorHandle -> nativeFindIds(handle, cursorHandle, offset, limit));
260266
}
261267

@@ -294,6 +300,7 @@ public PropertyQuery property(Property<T> property) {
294300
}
295301

296302
<R> R callInReadTx(Callable<R> callable) {
303+
checkOpen();
297304
return store.callInReadTxWithRetry(callable, queryAttempts, INITIAL_RETRY_BACK_OFF_IN_MS, true);
298305
}
299306

@@ -308,6 +315,7 @@ <R> R callInReadTx(Callable<R> callable) {
308315
*/
309316
public void forEach(final QueryConsumer<T> consumer) {
310317
ensureNoComparator();
318+
checkOpen(); // findIds also checks, but throw early outside of transaction.
311319
box.getStore().runInReadTx(() -> {
312320
LazyList<T> lazyList = new LazyList<>(box, findIds(), false);
313321
int size = lazyList.size();
@@ -384,6 +392,7 @@ void resolveEagerRelation(@Nonnull T entity, EagerRelation<T, ?> eagerRelation)
384392

385393
/** Returns the count of Objects matching the query. */
386394
public long count() {
395+
checkOpen();
387396
ensureNoFilter();
388397
return box.internalCallWithReaderHandle(cursorHandle -> nativeCount(handle, cursorHandle));
389398
}
@@ -392,6 +401,7 @@ public long count() {
392401
* Sets a parameter previously given to the {@link QueryBuilder} to a new value.
393402
*/
394403
public Query<T> setParameter(Property<?> property, String value) {
404+
checkOpen();
395405
nativeSetParameter(handle, property.getEntityId(), property.getId(), null, value);
396406
return this;
397407
}
@@ -402,6 +412,7 @@ public Query<T> setParameter(Property<?> property, String value) {
402412
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
403413
*/
404414
public Query<T> setParameter(String alias, String value) {
415+
checkOpen();
405416
nativeSetParameter(handle, 0, 0, alias, value);
406417
return this;
407418
}
@@ -410,6 +421,7 @@ public Query<T> setParameter(String alias, String value) {
410421
* Sets a parameter previously given to the {@link QueryBuilder} to a new value.
411422
*/
412423
public Query<T> setParameter(Property<?> property, long value) {
424+
checkOpen();
413425
nativeSetParameter(handle, property.getEntityId(), property.getId(), null, value);
414426
return this;
415427
}
@@ -420,6 +432,7 @@ public Query<T> setParameter(Property<?> property, long value) {
420432
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
421433
*/
422434
public Query<T> setParameter(String alias, long value) {
435+
checkOpen();
423436
nativeSetParameter(handle, 0, 0, alias, value);
424437
return this;
425438
}
@@ -428,6 +441,7 @@ public Query<T> setParameter(String alias, long value) {
428441
* Sets a parameter previously given to the {@link QueryBuilder} to a new value.
429442
*/
430443
public Query<T> setParameter(Property<?> property, double value) {
444+
checkOpen();
431445
nativeSetParameter(handle, property.getEntityId(), property.getId(), null, value);
432446
return this;
433447
}
@@ -438,6 +452,7 @@ public Query<T> setParameter(Property<?> property, double value) {
438452
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
439453
*/
440454
public Query<T> setParameter(String alias, double value) {
455+
checkOpen();
441456
nativeSetParameter(handle, 0, 0, alias, value);
442457
return this;
443458
}
@@ -481,6 +496,7 @@ public Query<T> setParameter(String alias, boolean value) {
481496
* Sets a parameter previously given to the {@link QueryBuilder} to new values.
482497
*/
483498
public Query<T> setParameters(Property<?> property, long value1, long value2) {
499+
checkOpen();
484500
nativeSetParameters(handle, property.getEntityId(), property.getId(), null, value1, value2);
485501
return this;
486502
}
@@ -491,6 +507,7 @@ public Query<T> setParameters(Property<?> property, long value1, long value2) {
491507
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
492508
*/
493509
public Query<T> setParameters(String alias, long value1, long value2) {
510+
checkOpen();
494511
nativeSetParameters(handle, 0, 0, alias, value1, value2);
495512
return this;
496513
}
@@ -499,6 +516,7 @@ public Query<T> setParameters(String alias, long value1, long value2) {
499516
* Sets a parameter previously given to the {@link QueryBuilder} to new values.
500517
*/
501518
public Query<T> setParameters(Property<?> property, int[] values) {
519+
checkOpen();
502520
nativeSetParameters(handle, property.getEntityId(), property.getId(), null, values);
503521
return this;
504522
}
@@ -509,6 +527,7 @@ public Query<T> setParameters(Property<?> property, int[] values) {
509527
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
510528
*/
511529
public Query<T> setParameters(String alias, int[] values) {
530+
checkOpen();
512531
nativeSetParameters(handle, 0, 0, alias, values);
513532
return this;
514533
}
@@ -517,6 +536,7 @@ public Query<T> setParameters(String alias, int[] values) {
517536
* Sets a parameter previously given to the {@link QueryBuilder} to new values.
518537
*/
519538
public Query<T> setParameters(Property<?> property, long[] values) {
539+
checkOpen();
520540
nativeSetParameters(handle, property.getEntityId(), property.getId(), null, values);
521541
return this;
522542
}
@@ -527,6 +547,7 @@ public Query<T> setParameters(Property<?> property, long[] values) {
527547
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
528548
*/
529549
public Query<T> setParameters(String alias, long[] values) {
550+
checkOpen();
530551
nativeSetParameters(handle, 0, 0, alias, values);
531552
return this;
532553
}
@@ -535,6 +556,7 @@ public Query<T> setParameters(String alias, long[] values) {
535556
* Sets a parameter previously given to the {@link QueryBuilder} to new values.
536557
*/
537558
public Query<T> setParameters(Property<?> property, double value1, double value2) {
559+
checkOpen();
538560
nativeSetParameters(handle, property.getEntityId(), property.getId(), null, value1, value2);
539561
return this;
540562
}
@@ -545,6 +567,7 @@ public Query<T> setParameters(Property<?> property, double value1, double value2
545567
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
546568
*/
547569
public Query<T> setParameters(String alias, double value1, double value2) {
570+
checkOpen();
548571
nativeSetParameters(handle, 0, 0, alias, value1, value2);
549572
return this;
550573
}
@@ -553,6 +576,7 @@ public Query<T> setParameters(String alias, double value1, double value2) {
553576
* Sets a parameter previously given to the {@link QueryBuilder} to new values.
554577
*/
555578
public Query<T> setParameters(Property<?> property, String[] values) {
579+
checkOpen();
556580
nativeSetParameters(handle, property.getEntityId(), property.getId(), null, values);
557581
return this;
558582
}
@@ -563,6 +587,7 @@ public Query<T> setParameters(Property<?> property, String[] values) {
563587
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
564588
*/
565589
public Query<T> setParameters(String alias, String[] values) {
590+
checkOpen();
566591
nativeSetParameters(handle, 0, 0, alias, values);
567592
return this;
568593
}
@@ -571,6 +596,7 @@ public Query<T> setParameters(String alias, String[] values) {
571596
* Sets a parameter previously given to the {@link QueryBuilder} to new values.
572597
*/
573598
public Query<T> setParameters(Property<?> property, String key, String value) {
599+
checkOpen();
574600
nativeSetParameters(handle, property.getEntityId(), property.getId(), null, key, value);
575601
return this;
576602
}
@@ -581,6 +607,7 @@ public Query<T> setParameters(Property<?> property, String key, String value) {
581607
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
582608
*/
583609
public Query<T> setParameters(String alias, String key, String value) {
610+
checkOpen();
584611
nativeSetParameters(handle, 0, 0, alias, key, value);
585612
return this;
586613
}
@@ -589,6 +616,7 @@ public Query<T> setParameters(String alias, String key, String value) {
589616
* Sets a parameter previously given to the {@link QueryBuilder} to new values.
590617
*/
591618
public Query<T> setParameter(Property<?> property, byte[] value) {
619+
checkOpen();
592620
nativeSetParameter(handle, property.getEntityId(), property.getId(), null, value);
593621
return this;
594622
}
@@ -599,6 +627,7 @@ public Query<T> setParameter(Property<?> property, byte[] value) {
599627
* @param alias as defined using {@link QueryBuilder#parameterAlias(String)}.
600628
*/
601629
public Query<T> setParameter(String alias, byte[] value) {
630+
checkOpen();
602631
nativeSetParameter(handle, 0, 0, alias, value);
603632
return this;
604633
}
@@ -609,6 +638,7 @@ public Query<T> setParameter(String alias, byte[] value) {
609638
* @return count of removed Objects
610639
*/
611640
public long remove() {
641+
checkOpen();
612642
ensureNoFilter();
613643
return box.internalCallWithWriterHandle(cursorHandle -> nativeRemove(handle, cursorHandle));
614644
}
@@ -632,6 +662,7 @@ public long remove() {
632662
* it may be GCed and observers may become stale (won't receive anymore data).
633663
*/
634664
public SubscriptionBuilder<List<T>> subscribe() {
665+
checkOpen();
635666
return new SubscriptionBuilder<>(publisher, null);
636667
}
637668

@@ -663,6 +694,7 @@ public void publish() {
663694
* Note: the format of the returned string may change without notice.
664695
*/
665696
public String describe() {
697+
checkOpen();
666698
return nativeToString(handle);
667699
}
668700

@@ -673,7 +705,17 @@ public String describe() {
673705
* Note: the format of the returned string may change without notice.
674706
*/
675707
public String describeParameters() {
708+
checkOpen();
676709
return nativeDescribeParameters(handle);
677710
}
678711

712+
/**
713+
* Throws if {@link #close()} has been called for this.
714+
*/
715+
private void checkOpen() {
716+
if (handle == 0) {
717+
throw new IllegalStateException("This query is closed. Build and use a new one.");
718+
}
719+
}
720+
679721
}

‎tests/objectbox-java-test/src/test/java/io/objectbox/query/PropertyQueryTest.java‎

Lines changed: 38 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -26,6 +26,7 @@
2626
import io.objectbox.exception.DbException;
2727
import io.objectbox.exception.NumericOverflowException;
2828
import io.objectbox.query.QueryBuilder.StringOrder;
29+
import org.junit.function.ThrowingRunnable;
2930

3031

3132
import static io.objectbox.TestEntity_.simpleBoolean;
@@ -76,6 +77,43 @@ private void putTestEntityFloat(float vFloat, double vDouble) {
7677
box.put(entity);
7778
}
7879

80+
@Test
81+
public void useAfterClose_fails() {
82+
Query<TestEntity> query = box.query().build();
83+
query.close();
84+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findStrings());
85+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findLongs());
86+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findInts());
87+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findShorts());
88+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findChars());
89+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findBytes());
90+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findFloats());
91+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findDoubles());
92+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findString());
93+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findLong());
94+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findInt());
95+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findShort());
96+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findChar());
97+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findByte());
98+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findBoolean());
99+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findFloat());
100+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).findDouble());
101+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).sum());
102+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).sumDouble());
103+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).max());
104+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).maxDouble());
105+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).min());
106+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).minDouble());
107+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).avg());
108+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).avgLong());
109+
assertThrowsQueryIsClosed(() -> query.property(simpleInt).count());
110+
}
111+
112+
private void assertThrowsQueryIsClosed(ThrowingRunnable runnable) {
113+
IllegalStateException ex = assertThrows(IllegalStateException.class, runnable);
114+
assertEquals("This query is closed. Build and use a new one.", ex.getMessage());
115+
}
116+
79117
@Test
80118
public void testFindStrings() {
81119
putTestEntity(null, 1000);

‎tests/objectbox-java-test/src/test/java/io/objectbox/query/QueryTest.java‎

Lines changed: 51 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@
3030
import io.objectbox.relation.Order;
3131
import io.objectbox.relation.Order_;
3232
import org.junit.Test;
33+
import org.junit.function.ThrowingRunnable;
3334

3435
import java.util.ArrayList;
3536
import java.util.Arrays;
@@ -86,6 +87,56 @@ public void testBuildTwice() {
8687
}
8788
}
8889

90+
@Test
91+
public void useAfterQueryClose_fails() {
92+
Query<TestEntity> query = box.query().build();
93+
query.close();
94+
95+
assertThrowsQueryIsClosed(query::count);
96+
assertThrowsQueryIsClosed(query::describe);
97+
assertThrowsQueryIsClosed(query::describeParameters);
98+
assertThrowsQueryIsClosed(query::find);
99+
assertThrowsQueryIsClosed(() -> query.find(0, 1));
100+
assertThrowsQueryIsClosed(query::findFirst);
101+
assertThrowsQueryIsClosed(query::findIds);
102+
assertThrowsQueryIsClosed(() -> query.findIds(0, 1));
103+
assertThrowsQueryIsClosed(query::findLazy);
104+
assertThrowsQueryIsClosed(query::findLazyCached);
105+
assertThrowsQueryIsClosed(query::findUnique);
106+
assertThrowsQueryIsClosed(query::remove);
107+
108+
// For setParameter(s) the native method is not actually called, so fine to use incorrect alias and property.
109+
assertThrowsQueryIsClosed(() -> query.setParameter("none", "value"));
110+
assertThrowsQueryIsClosed(() -> query.setParameters("none", "a", "b"));
111+
assertThrowsQueryIsClosed(() -> query.setParameter("none", 1));
112+
assertThrowsQueryIsClosed(() -> query.setParameters("none", new int[]{1, 2}));
113+
assertThrowsQueryIsClosed(() -> query.setParameters("none", new long[]{1, 2}));
114+
assertThrowsQueryIsClosed(() -> query.setParameters("none", 1, 2));
115+
assertThrowsQueryIsClosed(() -> query.setParameter("none", 1.0));
116+
assertThrowsQueryIsClosed(() -> query.setParameters("none", 1.0, 2.0));
117+
assertThrowsQueryIsClosed(() -> query.setParameters("none", new String[]{"a", "b"}));
118+
assertThrowsQueryIsClosed(() -> query.setParameter("none", new byte[]{1, 2}));
119+
assertThrowsQueryIsClosed(() -> query.setParameter(simpleString, "value"));
120+
assertThrowsQueryIsClosed(() -> query.setParameters(simpleString, "a", "b"));
121+
assertThrowsQueryIsClosed(() -> query.setParameter(simpleString, 1));
122+
assertThrowsQueryIsClosed(() -> query.setParameters(simpleString, new int[]{1, 2}));
123+
assertThrowsQueryIsClosed(() -> query.setParameters(simpleString, new long[]{1, 2}));
124+
assertThrowsQueryIsClosed(() -> query.setParameters(simpleString, 1, 2));
125+
assertThrowsQueryIsClosed(() -> query.setParameter(simpleString, 1.0));
126+
assertThrowsQueryIsClosed(() -> query.setParameters(simpleString, 1.0, 2.0));
127+
assertThrowsQueryIsClosed(() -> query.setParameters(simpleString, new String[]{"a", "b"}));
128+
assertThrowsQueryIsClosed(() -> query.setParameter(simpleString, new byte[]{1, 2}));
129+
130+
// find would throw once first results are obtained, but shouldn't allow creating an observer to begin with.
131+
assertThrowsQueryIsClosed(() -> query.subscribe().observer(data -> {
132+
}));
133+
}
134+
135+
private void assertThrowsQueryIsClosed(ThrowingRunnable runnable) {
136+
IllegalStateException ex = assertThrows(IllegalStateException.class, runnable);
137+
assertEquals("This query is closed. Build and use a new one.", ex.getMessage());
138+
}
139+
89140
@Test
90141
public void testNullNotNull() {
91142
List<TestEntity> scalars = putTestEntitiesScalars();

0 commit comments

Comments
 (0)