Implement extracting/contains assertions

[xp-vit]

AssertJ has really powerful extracting/contains mechanism. See: http://joel-costigliola.github.io/assertj/assertj-core-features-highlight.html Section: Assertions on extracted properties/fields of iterable/array elements

I think it worth to implement similar/same for fluent part of the library. I think we can just add assertJ core as a dependency and implement extracting method which returns: "AbstractListAssert" and we'll just leverage assertions they have. In the end we'll have something like: JsonFluentAssert.assertThatJson(jsonResponse.getBody().getObject()) .node("data").isPresent().isArray().ofLength(3) .extracting(“name”, “age”, “race.name”) .contains(tuple(“Boromir”, 37, “Man”), tuple(“Sam”, 38, “Hobbit”), tuple(“Legolas”, 1000, “Elf”));

What do you think? If you are agree and like the idea, I can work on it.

[lukas-krecan]

Hi, thanks for feedback. here are some quick thoughts (will have to think about it deeper). It would be great to have it but there are two issues.

1. json-unit works with Java 6 and some people may depend on it
2. Some people may be upset by new dependency on assertj

But we can create a new module jsonunit-assertj which could depend on Java 8 and assertj and which would have more assertj-like API. It would lead to code duplication between jsoununit-fluent and jsonunit-assertj but it should be OK.

[xp-vit]

Valid points. However about point 1 http://joel-costigliola.github.io/assertj/assertj-core.html: AssertJ 3.x requires Java 8 or higher (use the org.assertj.core.api.Java6Assertions entry point on Android) AssertJ 2.x requires Java 7 or higher (use the org.assertj.core.api.Java6Assertions entry point on Android) AssertJ 1.x requires Java 6 or higher

About extra dependency I don't think it's a huge issue, but still valid.

Hoever creating jsonunit-assertj lib will be also a good option. And to eliminate code duplications we can just depend on "jsoununit-fluent".

Let me know what do you think?

[lukas-krecan]

I would prefer to extract common code to some other internal module if possible (and necessary). Overall, it would be a great opportunity to clean-up the fluent API and maybe integrate it more closely with AssertJ. I can imagine methods like isArray returning a subclass of AssertJ array assertion and so on. So if you want to try, go ahead. Please do not hesitate to send an early unfinished PR so we can make sure that we understand each other. Thanks.

[xp-vit]

Agreed, starting.

[lukas-krecan]

Hi, how it goes, do you need any help?

[eximius313]

I think this feature is very important. I have an array of objects: [{key: "bar"}, {key: "bar"}] and I'd like to check if key is equal to bar. Currently I must do it like that:

assertThatJson(json)
      .isArray()
      .ofLength(2)
assertThatJson(json)
      .node("[0].key")
      .isEqualTo("bar")

while this would be much better:

assertThatJson(json)
      .isArray()
      .ofLength(2)
      .extractingNode("[0].key")
      .isEqualTo("bar")

[lukas-krecan]

I have started to play with it see https://github.com/lukas-krecan/JsonUnit/pull/102

[eximius313]

@lukas-krecan I've found very nice article about it: https://blog.tratif.com/2018/01/16/custom-fluent-assertions-with-assertj

[eximius313]

@lukas-krecan I've found another glitch: JsonFluentAssert.assertThatJson("{\"myObj\":{\"inside\":[\"foo\"]}}").node("myObj").node("inside") should return exactly the same result as node("myObj.inside"). Unfortunately, while the later works, the former doesn't.

[lukas-krecan]

You can try https://github.com/lukas-krecan/JsonUnit#assertj form 2.0.0.RC1. Looking forward for your feedback.

[eximius313]

First big problem that I found is that previously sequential node calls were referring to siblings:

myAssertion
.node("name").isEqualTo(name)
.node("street").isEqualTo(street);

while now they're referring to children (which is great!), but now the error appears: Missing node in path "name.street"

Maybe introducing something like and which goes one level up (returns parent assertion) in assertion could be a solution for migration (in order to offer backward compatibility):

myAssertion
.node("name").isEqualTo(name)
.and()
.node("street").isEqualTo(street);

where and is implemented as NestedAssertion?

[eximius313]

@lukas-krecan what do you think about it?

[lukas-krecan]

Sorry, I have been busy. I understand how it would help you with the migration. On the other hand, calling node multiple times behaved surprisingly in the previous version.

Now we have three options: a) Do nothing, let people do

myAssertion.node("name").isEqualTo(name);
myAssertion.node("street").isEqualTo(name);

b) Provide something like your proposition. The question is what is it supposed to do.

myAssertion.node("name").isString().isEqualTo(name).and()....

If we want to simplify migration, we should keep the behavior from jsonunit-fluent which would be to jump to the root. There are two issues with that. Method name and does not convey this intent. We can call it root or reset which does feel awkward. The other issues is implementation .isString() method return vanilla Assertj String assertion which we would need to extend in order to support this. It's easy to do, but it adds too much complexity for a feature I am not sure about.

c) Do something in between

myAssertion.and(
    a -> a.node("name").isString().isEqualTo(name), 
    a -> a.node("street").isString().isEqualTo(street)
);  

[eximius313]

I think that b) which returns back to the root is the best option and I think it's name is rather standardized.

1) Look at section Nested assertions in the article I have sent you (https://blog.tratif.com/2018/01/16/custom-fluent-assertions-with-assertj/) 2) You could also take this as an example: https://docs.spring.io/spring-security/site/docs/5.0.6.RELEASE/reference/htmlsingle/#jc-httpsecurity

[eximius313]

@lukas-krecan Have you thought about returning to parent node? Can I help you somehow?

[lukas-krecan]

Hi, sorry. Frankly, I really do not like option b) and I had other stuff to do. That's why I have been postponing the decision.

[eximius313]

Ok, I've got better solution :)

I've been experimenting with a), b) and c) in my project, but none was good enough. Finally I've found this article and it works great!

My implementation look like this: .node("bar", nodeAssert -> nodeAssert.isEqualTo("foo"));

Yes, it is not backward compatibile but... It should not. Why? Because current implementation of .node() will compile just fine, but the implementation changed, so all tests will fail what is even worse! The new way will fail at compile time (in exactly the same manner as with .ofLength which changed to .hasSize)

And as you are introducing version 2.0.0 -> it's the last occasion to change the API

If you like this solution, I can prepare PR. Please let me know what do you think

[eximius313]

Moreover - I think that with this implementation, inPath can be moved from ConfigurableJsonAssert to JsonAssert

[eximius313]

@lukas-krecan - I've made PR. Tell me what you think about it: https://github.com/lukas-krecan/JsonUnit/pull/133

[lukas-krecan]

I have made my mind, I will go for option c)

myAssertion.and(
    a -> a.node("name").isString().isEqualTo(name), 
    a -> a.node("street").isString().isEqualTo(street)
);

[lukas-krecan]

@eximius313 Can you please check this https://github.com/lukas-krecan/JsonUnit/pull/135

[eximius313]

It works! Although in perfect world this:

myAssertion.and(
    a -> a.node("name").isString().isEqualTo(name), 
    a -> a.node("street").isString().isEqualTo(street)
);

could coexist together with this:

myAssertion.
    .node("name", a -> a.isString().isEqualTo(name))
    .node("street", a -> a.isString().isEqualTo(street))

but having just one is 100 times better than nothing ;)

Thank you!