Add new html content matcher for MockMvcResultMatchers [SPR-16097]

[spring-projects-issues]

Leonard Brünings opened SPR-16097 and commented

The current ContentResultMatchers have direct support for plain strings, json, and xml. I would propose to add another that supports the usage of cssQuery and DOM operations.

Using https://jsoup.org/ I've created a small POC and it was rather easy to get this example implemented.

This example checks if the csrf token was correctly injected by spring security.

mockMvc.perform(get("/").with(user("user")))
		.andExpect(status().is2xxSuccessful())
		.andExpect(html(contains("form input[type=hidden][name=_csrf]")));

I don't think that there is a way to do the same with the current available methods, the closest would be a containsString("<input type=\"hidden\" name=\"_csrf\"") matcher on the body content, and this only works if the value attribute comes after the name attribute. And it still doesn't check if it is part of the form.

The challenge that I see is to have an expressive syntax without performing redundant work, e.g., only parse the body content into a Document once.
Furthermore, we'd need expressions for singular and multiple results, since cssQuery can match more than one Element.

This is just an example what could be done, suggestions welcome.

andExpect(
    content().html()
        .toMatch(anElement("form input[type=hidden][name=_csrf]").withAttribute("value", matchesPattern(UUID_PATTERN)))
        .toMatch(elements("form input").count(5))
)

---

0 votes, 5 watchers

[spring-projects-issues]

Rossen Stoyanchev commented

Have you considered the HTMLUnit integration?

As for using jsop as you said it's much better to get the content parsed once. At that point it makes sense to keep it separate, i.e. access the response body content via mockMvc.perform(..).andReturn().getResponse() and then apply assertions.

[spring-projects-issues]

Leonard Brünings commented

I didn't know about the HtmlUnit integration, which is quite nice, however I think this is a bit too much for the simple use-case of checking if some html is present.
There is XmlUnit but the content result matchers still have support for xml (node), the same goes for json (jsonassert/jsonunit).
What I dislike about HtmlUnit in this context is that, it breaks the consistency when used with plain mockmvc tests, by requiring a different request syntax.
IMHO, this is still the realm of a unit test and full integration test.

[spring-projects-issues]

Rossen Stoyanchev commented

IMHO, this is still the realm of a unit test and full integration test.

Did you mean "not a full integration test"? How would you approach writing tests then given a choice between jsoup matchers and the HTMLUnit integration? When would you prefer which and would you use both as part of an overall test suite for an application?

/cc robwinch

[spring-projects-issues]

Leonard Brünings commented

Did you mean "not a full integration test"?
Yes

We use WebDriver tests with Geb to write frontend tests, e.g. going to a site and interacting with it, by clicking links/buttons and input data. Sure you can assert that a _csrf hidden field is there, but that should have been verified beforehand. But the whole test level is on a different level. HTMLUnit goes into the same direction by simulating a browser, and it can also execute JavaScript.
I would use the Jsoup matchers in my normal controller (+view) tests to make sure that the parts of the html I care about in this particular test are present. So I think it is quite valuable to be able to use it like the other matchers in normal MockMvc fashion.

[spring-projects-issues]

Rossen Stoyanchev commented

Okay this could be useful for basic structural assertions on HTML content alongside similar options for XML and JSON. Were you thinking of putting together a PR?

[spring-projects-issues]

Leonard Brünings commented

Yes, I was thinking about creating a PR but wanted to ask first if it would be accepted in the first place. I take your answer as a go ahead, I'll let you know when I've got it ready.

[spring-projects-issues]

Rossen Stoyanchev commented

Great thanks.

[spring-projects-issues]

Leonard Brünings commented

Rossen Stoyanchev I've created a temporary repository where I've put my current implementation https://github.com/leonard84/SPR-16097. It is still a work in progress, but if you already have some feedback on the API let me know.

This test shows how it is intended to be used: MatcherIT

This test shows the error messages that are produced MatcherTest

[spring-projects-issues]

Rossen Stoyanchev commented

I haven't given it a proper look yet, but I wanted to pass on a comment from robwinch:

we might consider trying to use dependencies we already have... rely on whatever htmlunit is using

[spring-projects-issues]

Rob Winch commented

Rossen Stoyanchev I thought I had, but perhaps I didn't pass this along (sorry if that is the case).... I looked into using HtmlUnit instead of jsoup and it really doesn't make sense. The way HtmlUnit is structured, it doesn't lend well for solving this problem. In short, I don't think we can use our existing dependencies and that jsoup is a fine choice.

[spring-projects-issues]

Leonard Brünings commented

Rossen Stoyanchev, Rob Winch I've implemented this feature as far as the API is concerned, it currently supports Hamcrest-style and AssertJ-style assertions. You can see them in the MatcherIT I'd like to get some feedback before I start documenting it all.

• Should we leave both Hamcrest-style and AssertJ-style assertions in the API?
• Is there anything missing?
• Any improvements?

[spring-projects-issues]

Rossen Stoyanchev commented

Leonard Brünings, thanks for putting this together.

1. Why is it necessary to have both of these variants available in the API?

• anElement + document(anElement())
• elements + document(elements())
• eachElement + document(eachElement())

As far as I can see, they're both built on the same Hamcrest Matchers, but the second exposes a more fluent style to chain multiple assertions. Having both overloads the API and creates questions when to use which? The second seems preferable but first can be shorter. It would be better to have one syntax with the benefits of both.

Right now it doesn't read very well when using document repeatedly:

html -> 
    document -> anElement
    document -> eachElement
    ...

Each new line is not a new document, but a new selector, which points to something like:

html ->
    selector -> anElement
    selector -> eachElement
    ...

I didn't experiment in detail with the changes, so I'm only pointing out how it would read better, but the idea is to re-think having two styles in favor of just one.

BTW I did not see any dependency on AssertJ in the main classes. Am I missing anything? My first thought was we can't justify both Hamcrest and AssertJ given that the rest of MockMvc supports only Hamcrest, but after looking closer it seems to require no AssertJ dependencies.

2. Looking at resulting classes, it looks a lot like a Hacmcrest integration for JSoup. Except for HtmlContentMatcher all others are either a Hamcrest Matcher, or expose static factory methods for Hamcrest matchers, or support the fluent API around the Hamcrest matchers.

Have you considered proposing such classes to be included in https://github.com/jhy/jsoup ? Hamcrest is a widely used Java library, so it seems generally useful to have such an integration. It's not hard to maintain but it would make more sense to that in jsoup.

So then the only class we need in MockMvc is the HtmlContentMatcher that would live in the o.s.test.web.servlet.result package, but others don't belong in the same package. The most likely home would be o.s.test.util but again if this Hamcrest integration could live closer to JSoup then we'd simply consume it from there.

3. As a smaller point, there are some classes that probably don't need to be public. I noticed PropertyMatcher, and may be ElementsMatcher and SingleElementMatcher, but I think there are more. It's worth reviewing what is public vs non-public API.

[spring-projects-issues]

Leonard Brünings commented

Hi Rossen Stoyanchev,

yes it is not neccessary to have both APIs, I added the fluent-style because I mentioned it in the description after I implemented the more traditional nested Hamcrest-style. I wanted to try both approaches and see which style would be better. And I agree with reducing it to one style, I lean toward keeping the fluent style as it makes it easier and less awkward to assert multiple things.

The document matcher was left from the hamcrest API and I didn't have a good alias name for it, what do you think of toMatch.

perfom(get)
andExpect ->
   html ->
       toMatch -> anElement
       toMatch -> eachElement.hasAttribute
       ...

Regarding AssertJ, I just called the fluent-api Assertj-style assertions since they more closely match Assertj than Hamcrest-style, without actually having a dependency on Assertj.

I created an issue at Jsoup to ask if they would be interested in incorporating the Hamcrest matchers into Jsoup.

Regarding public-private API, you are correct I haven't really made the decision or cleaned up the visibility. What I definitely want is to enable users to write their own matchers if they so choose, but maybe the predicate is sufficient.