Skip to content

New query condition to match all strings that starts with some other given strings #673

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

Closed
Show file tree
Hide file tree
Changes from 2 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 9 additions & 1 deletion Parse/src/main/java/com/parse/ParseEncoder.java
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,7 @@
import java.util.Date;
import java.util.List;
import java.util.Map;
import java.util.regex.Pattern;

/**
* A {@code ParseEncoder} can be used to transform objects such as {@link ParseObjects} into JSON
Expand All @@ -40,7 +41,8 @@
|| value instanceof ParseACL
|| value instanceof ParseFile
|| value instanceof ParseGeoPoint
|| value instanceof ParseRelation;
|| value instanceof ParseRelation
|| value instanceof Pattern;
}

public Object encode(Object object) {
Expand Down Expand Up @@ -120,6 +122,12 @@ public Object encode(Object object) {
return ((ParseQuery.RelationConstraint) object).encode(this);
}

if (object instanceof Pattern) {
JSONObject json = new JSONObject();
json.put("$regex", object.toString());
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Would it be possible to avoid Patterns and move this logic to ParseQuery just like all other $ constraints?

I mean using ParseQuery.addCondition(key, “$regex”, string). That way we keep consistencies and leave encoder untouched.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think that could be better to avoid update the ParseEncoder. I didn't researched a lot.

Now, I think could be better to update the constraintsAllStartsWith to:

  public ParseQuery<T> whereContainsAllStartsWith(String key, Collection<String> values) {
    checkIfRunning();

    ArrayList<KeyConstraints> startsWithConstraints = new ArrayList<>();
    for (String value : values) {
      KeyConstraints keyConstraints = new KeyConstraints();
      keyConstraints.put("$regex", buildStartsWithRegex(value));
      startsWithConstraints.add(keyConstraints);
    }

    return whereContainsAll(key, startsWithConstraints);
  }

This might auto compile as expected. If people prefer to revert the ParseEncoder changes, I'll update the PR 👍

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It makes sense. Also since whereContainsAll is documented to work only with ParseObjects, you could call directly builder.addCondition(key, "$all", startsWithConstraints) and return this

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I've updated the function to use KeyConstraints.

I keep using whereContsinaAll because was not restricted to ParseObjects. The docs were wrong.

return json;
}

if (object == null) {
return JSONObject.NULL;
}
Expand Down
40 changes: 39 additions & 1 deletion Parse/src/main/java/com/parse/ParseQuery.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/
package com.parse;

import android.support.annotation.NonNull;

import org.json.JSONException;
import org.json.JSONObject;

Expand Down Expand Up @@ -1694,6 +1696,31 @@ public ParseQuery<T> whereContainsAll(String key, Collection<?> values) {
return this;
}

/**
* Add a constraint to the query that requires a particular key's value match another
* {@code ParseQuery}.
* <p/>
* This only works on keys whose values are {@link ParseObject}s or lists of {@link ParseObject}s.
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think this javadoc is wrong? Add a constraint ... to match another ParseQuery. Same for whereContainsAll

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I thought so also when I copied the comment from containsAll, but was not sure to change it.

Thanks to comment this 👍 Someone could write a good documentation.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yeah, I think you can remove/reword both to their real meaning.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Comments updated.

* Add a constraint to the query that requires a particular key's value to contain each one of
* the provided list of values entirely or just starting with given strings.
*
* @param key
* The key to check. This key's value must be an array.
* @param values
* The values that will match.
* @return this, so you can chain this call.
*/
public ParseQuery<T> whereContainsAllStartsWith(String key, Collection<String> values) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Any suggestions for a different name? This seems obscure at first.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

At the beginning I was thinking about containsAllStartingWith. Maybe this could be a better approach.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It took me a while to understand what this does. Personally I think the cleanest and simplest wording is to keep whereStartsWith:

  • whereStartsWith(key, String prefix): check that value in key (must be String) starts with prefix
  • whereStartsWith(key, Collection<String> prefixes): check that values in key (must be Array) start with prefixes in the given collection (implicit: all of the constraints must be satisfied)

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Could be a good name, but I think it's good to have the All word to know that all strings in collection must match. But I like your option too.

Let's see what other prefer.

checkIfRunning();

ArrayList<Pattern> regexValues = new ArrayList<>();
for (String value : values) {
regexValues.add(Pattern.compile(buildStartsWithRegex(value)));
}

return whereContainsAll(key, regexValues);
}

/**
* Add a constraint to the query that requires a particular key's value match another
* {@code ParseQuery}.
Expand Down Expand Up @@ -1947,7 +1974,7 @@ public ParseQuery<T> whereContains(String key, String substring) {
* @return this, so you can chain this call.
*/
public ParseQuery<T> whereStartsWith(String key, String prefix) {
String regex = "^" + Pattern.quote(prefix);
String regex = buildStartsWithRegex(prefix);
whereMatches(key, regex);
return this;
}
Expand Down Expand Up @@ -2150,4 +2177,15 @@ public ParseQuery<T> setTrace(boolean shouldTrace) {
builder.setTracingEnabled(shouldTrace);
return this;
}

/**
* Helper method to convert a string to regex for start word matching.
*
* @param prefix String to use as prefix in regex.
* @return The string converted as regex for start word matching.
*/
@NonNull
private String buildStartsWithRegex(String prefix) {
return "^" + Pattern.quote(prefix);
}
}
9 changes: 9 additions & 0 deletions Parse/src/test/java/com/parse/ParseEncoderTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -22,6 +22,7 @@
import java.util.ArrayList;
import java.util.Date;
import java.util.HashMap;
import java.util.regex.Pattern;

import static org.junit.Assert.assertEquals;
import static org.junit.Assert.assertNotNull;
Expand Down Expand Up @@ -161,6 +162,14 @@ public void testRelationContraint() throws JSONException {
assertEquals(">", relationConstraintJSON.getString("key"));
}

@Test
public void testPattern() throws JSONException {
Pattern pattern = Pattern.compile("^\\Qvalue\\E");
JSONObject patternJSON = (JSONObject) testClassObject.encode(pattern);
assertNotNull(patternJSON);
assertEquals("^\\Qvalue\\E", patternJSON.getString("$regex"));
}

@Test
public void testNull() throws JSONException {
Object object = testClassObject.encode(null);
Expand Down
37 changes: 36 additions & 1 deletion Parse/src/test/java/com/parse/ParseQueryTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -8,6 +8,8 @@
*/
package com.parse;

import android.support.annotation.NonNull;

import org.junit.After;
import org.junit.Before;
import org.junit.Test;
Expand Down Expand Up @@ -411,6 +413,20 @@ public void testWhereContainsAll() throws Exception {
verifyCondition(query, "key", "$all", values);
}

@Test
public void testWhereContainsAllStartingWith() throws Exception {
ParseQuery<ParseObject> query = new ParseQuery<>("Test");
List<String> values = Arrays.asList("value", "valueAgain");
List<String> valuesConverted = Arrays.asList(
buildStartsWithPattern("value"),
buildStartsWithPattern("valueAgain")
);

query.whereContainsAllStartsWith("key", values);

verifyConditionAsString(query, "key", "$all", valuesConverted);
}

@Test
public void testWhereNotContainedIn() throws Exception {
ParseQuery<ParseObject> query = new ParseQuery<>("Test");
Expand Down Expand Up @@ -447,7 +463,7 @@ public void testWhereStartsWith() throws Exception {
String value = "prefix";
query.whereStartsWith("key", value);

verifyCondition(query, "key", "$regex", "^" + Pattern.quote(value));
verifyCondition(query, "key", "$regex", buildStartsWithPattern(value));
}

@Test
Expand Down Expand Up @@ -760,6 +776,20 @@ private static void verifyCondition(
assertEquals(map.get(constraintKey), values.get(constraintKey));
}
}

private static void verifyConditionAsString(
ParseQuery query, String key, String conditionKey, List values) {
// We generate a state to verify the content of the builder
ParseQuery.State state = query.getBuilder().build();
ParseQuery.QueryConstraints queryConstraints = state.constraints();
ParseQuery.KeyConstraints keyConstraints =
(ParseQuery.KeyConstraints) queryConstraints.get(key);
Collection<String> list = (Collection<String>) keyConstraints.get(conditionKey);
assertEquals(values.size(), list.size());
for (Object listValue : list) {
assertTrue(values.contains(listValue.toString()));
}
}

//endregion

Expand Down Expand Up @@ -846,4 +876,9 @@ public Task<Void> then(Task<Void> task) throws Exception {
})).cast();
}
}

@NonNull
private String buildStartsWithPattern(String value) {
return "^" + Pattern.quote(value);
}
}