Skip to content
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

Defines the pattern for taking unique items from collection of JSONArray #957

Open
wants to merge 1 commit into
base: master
Choose a base branch
from

Conversation

PavelSakharchuk
Copy link

No description provided.

public class Distinct implements PathFunction {

@Override
public Object invoke(String currentPath, PathRef parent, Object model, EvaluationContext ctx, List<Parameter> parameters) {
Copy link
Contributor

Choose a reason for hiding this comment

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

please make sure this function works with arrays of objects as well

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Added tests on the array of objects and empty array.

Comment on lines 23 to 27
Iterable<?> objects = ctx.configuration().jsonProvider().toIterable(model);
Set<Object> objectSet = new HashSet<>();
objects.forEach(objectSet::add);

return new ArrayList<>(objectSet);
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 Java Stream API can be used

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Updated


@Override
public Object invoke(String currentPath, PathRef parent, Object model, EvaluationContext ctx, List<Parameter> parameters) {
if(ctx.configuration().jsonProvider().isArray(model)){
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
if(ctx.configuration().jsonProvider().isArray(model)){
if (ctx.configuration().jsonProvider().isArray(model)) {

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Updated

public Object invoke(String currentPath, PathRef parent, Object model, EvaluationContext ctx, List<Parameter> parameters) {
if(ctx.configuration().jsonProvider().isArray(model)){
Iterable<?> objects = ctx.configuration().jsonProvider().toIterable(model);
Set<Object> objectSet = new HashSet<>();
Copy link
Contributor

Choose a reason for hiding this comment

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

HashSet may change order

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Updated

@@ -37,7 +37,7 @@ public Object invoke(String currentPath, PathRef parent, Object model, Evaluatio
}
}
}
throw new JsonPathException("Aggregation function attempted to calculate value using empty array");
throw new JsonPathException("Aggregation function attempted to calculate value using non array");
Copy link
Contributor

Choose a reason for hiding this comment

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

I guess it should go as a separate commit or even PR

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Reverted


@Test
public void testDistinctOfObjects() throws Exception {
final Object EXPECTED_VALUE = new ObjectMapper().readValue("[{\"a\":\"a-val\"}, {\"b\":\"b-val\"}]", Object.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final Object EXPECTED_VALUE = new ObjectMapper().readValue("[{\"a\":\"a-val\"}, {\"b\":\"b-val\"}]", Object.class);
final Object expectedArray = new ObjectMapper().readValue("[{\"a\":\"a-val\"}, {\"b\":\"b-val\"}]", Object.class);

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Updated


@Test
public void testDistinctOfEmptyObjects() throws Exception {
final Object EXPECTED_VALUE = new ObjectMapper().readValue("[]", Object.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
final Object EXPECTED_VALUE = new ObjectMapper().readValue("[]", Object.class);
final Object expectedArray = new ObjectMapper().readValue("[]", Object.class);

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Updated

README.md Outdated
| first() | Provides the first item of an array | Depends on the array |
| last() | Provides the last item of an array | Depends on the array |
| index(X) | Provides the item of an array of index: X, if the X is negative, take from backwards | Depends on the array |
| distinct() | Provides the unique items of an array | Depends on the array |
Copy link
Contributor

Choose a reason for hiding this comment

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

Suggested change
| distinct() | Provides the unique items of an array | Depends on the array |
| distinct() | Provides an array containing only unique items from the input array | List<E> |

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Updated

protected static final String TEXT_AND_NUMBER_SERIES = "{\"text\" : [ \"a\", \"b\", \"c\", \"d\", \"e\", \"f\" ], \"numbers\" : [ 1, 2, 3, 4, 5, 6, 7, 8, 9, 10]}";
protected static final String OBJECT_SERIES = "{\"empty\": [], \"objects\" : [ {\"a\": \"a-val\"}, {\"b\": \"b-val\"}, { \"a\": \"a-val\"} ]}";
Copy link
Contributor

Choose a reason for hiding this comment

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

what about array of arrays? does it work?

Copy link
Author

Choose a reason for hiding this comment

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

Yes.
I added a test for it.


@Test
public void testDistinctOfObjects() throws Exception {
final Object expectedValue = new ObjectMapper().readValue("[{\"a\":\"a-val\"}, {\"b\":\"b-val\"}]", Object.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't conf.mappingProvider() be used instead of explicit creation of ObjectMapper?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Updated


@Test
public void testDistinctOfEmptyObjects() throws Exception {
final Object expectedValue = new ObjectMapper().readValue("[]", Object.class);
Copy link
Contributor

Choose a reason for hiding this comment

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

shouldn't conf.mappingProvider() be used instead of explicit creation of ObjectMapper?

Copy link
Author

Choose a reason for hiding this comment

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

Thanks. Updated

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants