-
Notifications
You must be signed in to change notification settings - Fork 2.8k
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Fix "surprising queries" samples. #242
Changes from 1 commit
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -694,15 +694,15 @@ public void queryRestrictions_surprisingMultipleValuesAllMustMatch_returnsNoEnti | |
// [START surprising_behavior_example_1] | ||
Query q = | ||
new Query("Widget") | ||
.setFilter(new FilterPredicate("x", FilterOperator.GREATER_THAN, 1)) | ||
.setFilter(new FilterPredicate("x", FilterOperator.LESS_THAN, 2)); | ||
.setFilter( | ||
CompositeFilterOperator.and( | ||
new FilterPredicate("x", FilterOperator.GREATER_THAN, 1), | ||
new FilterPredicate("x", FilterOperator.LESS_THAN, 2))); | ||
// [END surprising_behavior_example_1] | ||
|
||
// Note: The documentation describes that the entity "a" will not match | ||
// because no value matches all filters. When run with the local test | ||
// runner, the entity "a" *is* matched. This may be a difference in | ||
// behavior between the local devserver and Cloud Datastore, so there | ||
// aren't any assertions we can make in this test. | ||
// Entity "a" will not match because no individual value matches all filters. | ||
List<Entity> results = datastore.prepare(q).asList(FetchOptions.Builder.withDefaults()); | ||
assertThat(results).named("query results").isEmpty(); | ||
} | ||
|
||
@Test | ||
|
@@ -716,21 +716,21 @@ public void queryRestrictions_surprisingMultipleValuesEquals_returnsMatchedEntit | |
c.setProperty("x", ImmutableList.<Long>of(-6L, 2L)); | ||
Entity d = new Entity("Widget", "d"); | ||
d.setProperty("x", ImmutableList.<Long>of(-6L, 4L)); | ||
datastore.put(ImmutableList.<Entity>of(a, b, c, d)); | ||
Entity e = new Entity("Widget", "e"); | ||
e.setProperty("x", ImmutableList.<Long>of(1L, 2L, 3L)); | ||
datastore.put(ImmutableList.<Entity>of(a, b, c, d, e)); | ||
|
||
// [START surprising_behavior_example_2] | ||
Query q = | ||
new Query("Widget") | ||
.setFilter(new FilterPredicate("x", FilterOperator.EQUAL, 1)) | ||
.setFilter(new FilterPredicate("x", FilterOperator.EQUAL, 2)); | ||
.setFilter( | ||
CompositeFilterOperator.and( | ||
new FilterPredicate("x", FilterOperator.EQUAL, 1), | ||
new FilterPredicate("x", FilterOperator.EQUAL, 2))); | ||
// [END surprising_behavior_example_2] | ||
|
||
List<Entity> results = datastore.prepare(q).asList(FetchOptions.Builder.withDefaults()); | ||
assertThat(getKeys(results)).named("query result keys").contains(a.getKey()); | ||
|
||
// Note: When run in the test server, this matches "c" as expected and does | ||
// not match "d" as expected. For some reason it does *not* match "b". | ||
// The behavior of queries on repeated values is definitely surprising. | ||
assertThat(getKeys(results)).named("query result keys").containsExactly(a.getKey(), e.getKey()); | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Any reason you are not using the Datastore There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I think the results will still be a list of Entities in that case. I want to do an equality check on the keys rather than the entity. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why/how a key-only query return you Entities and not Keys? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. PreparedQuery always returns Entity. https://cloud.google.com/appengine/docs/java/javadoc/com/google/appengine/api/datastore/PreparedQuery#asList-com.google.appengine.api.datastore.FetchOptions- I don't see any method in the PreparedQuery class that returns a iterable of Key. There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yes, but if you set the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Correct. I still need the There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Actually because you are not using Because this is part of user interfacing samples (I assume that even the tests are meant to be used as reference) I would highly recommend using the ideal code for accomplishing what is needed (regardless if There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Ah, you're right. |
||
} | ||
|
||
@Test | ||
|
@@ -770,17 +770,14 @@ public void queryRestrictions_surprisingMultipleValuesTwoNotEquals_returnsMatche | |
// [START surprising_behavior_example_4] | ||
Query q = | ||
new Query("Widget") | ||
.setFilter(new FilterPredicate("x", FilterOperator.NOT_EQUAL, 1)) | ||
.setFilter(new FilterPredicate("x", FilterOperator.NOT_EQUAL, 2)); | ||
.setFilter( | ||
CompositeFilterOperator.and( | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. That is a strange one that seems to ignore the comment in the Datastore queries doc saying that "A query can have no more than one NOT_EQUAL filter, and a query that has one cannot have any other inequality filters". @pcostell suggests that this is because the AE Datastore client breaks it There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I added a link to the docs. That link describes that the query acts like
|
||
new FilterPredicate("x", FilterOperator.NOT_EQUAL, 1), | ||
new FilterPredicate("x", FilterOperator.NOT_EQUAL, 2))); | ||
// [END surprising_behavior_example_4] | ||
|
||
List<Entity> results = datastore.prepare(q).asList(FetchOptions.Builder.withDefaults()); | ||
assertThat(getKeys(results)).named("query result keys").contains(b.getKey()); | ||
|
||
// Note: The documentation describes that the entity "a" will not match. | ||
// When run with the local test runner, the entity "a" *is* matched. This | ||
// may be a difference in behavior between the local devserver and Cloud | ||
// Datastore. | ||
assertThat(getKeys(results)).named("query result keys").containsExactly(b.getKey()); | ||
} | ||
|
||
private Entity retrievePersonWithLastName(String targetLastName) { | ||
|
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Fine as is, though a shorter alternative could be:
List<Long> xs = Arrays.asList(1L, 2L);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Done.