Hamcrest matcher to verify a header

[dmzaytsev]

Takes doesn't has a Hamcrest matcher to verify a header. I think it has to be.
Both request and response headers need to be verified.

something like this

public final class HmRqHeader extends TypeSafeMatcher<Request> {
    public HmRqHeader(final Matcher<? extends Entry<String,String>> mtchr)
}

public final class HmRsHeader extends TypeSafeMatcher<Response> {
    public HmRsHeader(final Matcher<? extends Entry<String,String>> mtchr)
}

---

- ~~`260-1c71c6a2`/#412~~ (by Dragan Bozanovic) - `260-de67913e`/#413 (by Dragan Bozanovic)

[davvd]

@dmzaytsev I will ask someone to take care of this task soon

[davvd]

@dmzaytsev I set milestone here to 1.0, let me know if it is wrong

[davvd]

@ekondrashev it's yours,please go ahead

[ekondrashev]

@davvd could you please let me know the budget here?

[ekondrashev]

@dmzaytsev is it Map.Entry used in the original code snippet?

[dmzaytsev]

@ekondrashev yes, it is Map.Entry
I believe it is the closest analogue for a Pair

[ekondrashev]

@dmzaytsev perhaps you meant Map<String, String> to be expected as ctor argument, since hamcrest Matchers class contains only hasEntry method returning Matcher parametrised with Map class.

like

public final class HmRqHeader extends TypeSafeMatcher<Request> {
    public HmRqHeader(final Matcher<? extends Map<String,String>> mtchr)
}

Could you please confirm?

[dmzaytsev]

@ekondrashev you're right we could use this matcher like this

MatcherAssert.assertThat(
    new RqWithHeaders(new RqFake(), "TestHeader: someValue")
    new HmRqHeader(Matchers.hasEntry("TestHeader", "someValue")
);

[yegor256]

@dmzaytsev @ekondrashev I think in this case we should call it HmRqHeaders

[dmzaytsev]

@yegor256 but an instance of HmRqHeader could check one header only. Isn't it ?

[yegor256]

@dmzaytsev yes, if we call it HmRqHeader, it should check one header. but this example checks all headers:

MatcherAssert.assertThat(
    new RqWithHeaders(new RqFake(), "TestHeader: someValue")
    new HmRqHeaders(Matchers.hasEntry("TestHeader", "someValue")
);

I can even use it like this:

MatcherAssert.assertThat(
    new RqWithHeaders(new RqFake(), "TestHeader: someValue")
    new HmRqHeaders(Matchers.iterableWithSize(1)))
);

[ekondrashev]

@yegor256 What do you mean by 'checks all headers'? In you example you are checking only one header. Could you please provide example usage for checking all headers?

From my side - current implementation (#269) checks only one pair of key/value. This is related to the fact that for checking all headers we need to expect at HmRqHeaders constructor Map<String, List<String>> but not Map<String, String>.
I'm also keeping in my that initial request was to check one pair only.

With regard to second usage example: Isn't it pretty different matcher? Lets say we can overload constructor to expect Matcher<Iterable>> also(which is already not good), but then you will have to if it at matchesSafely method which is really ugly.

So my point here is that for multiple value map matcher and checking size matcher it should be created different enhancement(s).

[dmzaytsev]

@yegor256 @ekondrashev well I agree Matcher<? extends Map<String,String>> isn't a very good choice.

let's try this

public HmRqHeader(CharSequence name, java.lang.Iterable<Matcher<CharSequence>> mtchr)

According RFC 2616

Multiple message-header fields with the same field-name MAY be present in a message if and only if the entire field-value for that header field is defined as a comma-separated list

I believe HmRqHeader should parse the header and pass Iterable with its values to the matcher.

What do you think ?

[yegor256]

@dmzaytsev I would rather work with a map...

[dmzaytsev]

@yegor256 what is the benefit a map in this case?
We can also check only one header at same time

[dmzaytsev]

@yegor256 Perhaps would be better to create a type e.g. org.takes.misc.Pair<K,V>
and to use public HmRqHeader(Iterable<Matcher<Pair> mtchr)
How about this?

[yegor256]

@dmzaytsev I see three scenarios:

1. assert that header is not present at all
2. assert that header has one value
3. assert that header has many values

How would you do them all with HmRqHeader?

[dmzaytsev]

@yegor256
let's look at both options

1. public HmRqHeader(CharSequence name, java.lang.Iterable<Matcher<CharSequence>> mtchr)
2. public HmRqHeader(Iterable<Matcher<Pair> mtchr)

• assert that header is not present at all

MatcherAssert.assertThat(
    new RqWithHeaders(new RqFake(), "TestHeader: someValue")
    new HmRqHeader("TestHeader", Matchers.emptyIterable())
);

MatcherAssert.assertThat(
    new RqWithHeaders(new RqFake(), "TestHeader: someValue")
    new HmRqHeader(Matcher.contains(Pair.empty("TestHeader")))
);

• assert that header has one value

MatcherAssert.assertThat(
    new RqWithHeaders(new RqFake(), "TestHeader: someValue")
    new HmRqHeader("TestHeader", Matchers.hasItem("someValue")))
);

MatcherAssert.assertThat(
    new RqWithHeaders(new RqFake(), "TestHeader: someValue")
    new HmRqHeader(Matcher.contains(Pair.of("TestHeader","someValue")))
);
// we could check more then one header
MatcherAssert.assertThat(
    new RqWithHeaders(new RqFake(), "HeaderA:ValueA", "HeaderB:ValueB")
    new HmRqHeader(
        Matcher.contains(
            Pair.of("HeaderA","ValueA"),
            Pair.of("HeaderB","ValueB")
        )
    )
);

• assert that header has many values

we can do so

MatcherAssert.assertThat(
    new RqWithHeaders(new RqFake(), "TestHeader: someValue")
    new HmRqHeader("TestHeader", Matchers.iterableWithSize(1))
);

however according RFC 2616 header should have only one value.
HmRqHeader is the high level matcher, it must generate an error if the Request contains more than one value for the header

[dmzaytsev]

@yegor256

• If we use the Pair, we could test all headers in the one matcher's call.
• If we use the Map we could check for partial header's content using the Mather hasEntry (Matcher <? Super K> keyMatcher, Matcher <? Super V> valueMatcher)

[davvd]

@dmzaytsev thank you for the ticket reported, I topped your account for 15 mins, transaction 56929062

[ekondrashev]

@davvd The enhancement is in process of clarifying the requirements, can't proceed.

[yegor256]

@davvd looks like we need more time here

[davvd]

@davvd looks like we need more time here

@yegor256 of course, take your time

[ekondrashev]

@yegor256 @dmzaytsev Any updates on enhancement requirements?

[ekondrashev]

@yegor256 could you please provide final decision here?
I'm not sure whether I should proceed here, since I'm not with this particular project no more.
Though I can.

[yegor256]

@ekondrashev sorry about the delay. this is how it should be (final decision):

public final class HmRqHeader extends TypeSafeMatcher<Request> {
  public HmRqHeader(final Matcher<? extends Map.Entry<String,String>> mtchr);
  public HmRqHeader(final String header, final Matcher<? extends Iterable<String>> mtchr);
  public HmRqHeader(final String header, final String value);
}
public final class HmRsHeader extends TypeSafeMatcher<Response> {
  public HmRsHeader(final Matcher<? extends Map.Entry<String,String>> mtchr);
  public HmRsHeader(final String header, final Matcher<? extends Iterable<String>> mtchr);
  public HmRsHeader(final String header, final String value);
}

[davvd]

@dmzaytsev these 2 puzzles were created in this ticket: 260-de67913e, 260-1c71c6a2/#412

[prondzyn]

@yegor256 I started work on #413 to implement constructors defined by you in 260#issuecomment-139857888. It is not possible to implement all five of them in Java due to language limitation described in Method has the same erasure as another method in type.

Constructor

public HmRsHeader(final Matcher<? extends Map<String,String>> mtchr);

clashes with

public HmRsHeader(final Matcher<? extends Map.Entry<String,String>> mtchr);

and

public HmRsHeader(final String header, final Matcher<? extends Iterable<String>> mtchr);

clashes with

public HmRsHeader(final String header, final Matcher<? extends String> mtchr);

so we can implement only three of the five listed.

Please decide which three of them we should implement or update your decision in other way.

[yegor256]

@prondzyn updated, see above

[davvd]

@dmzaytsev the last puzzle 260-de67913e/#413 originated from here solved

[ekondrashev]

@dmzaytsev can we close this issue please?

[dmzaytsev]

@ekondrashev sure, thanks

[davvd]

@elenavolokhova please, review this task for compliance with our quality rules

[elenavolokhova]

@davvd Looks good!

[davvd]

@davvd Looks good!

@elenavolokhova thanks a lot :)

[davvd]

@ekondrashev 10 mins was added to the account of @elenavolokhova (for QA review), in transaction 90067139

thanks, paid, 30 mins to your account, payment ID is 90067165, task took 9675 hours and 38 mins to complete

+30 to your rating, your total score is +1212