eclipse / microprofile-open-api

Microprofile open api
Apache License 2.0
131 stars 82 forks source link

TCK Tag Collection Test contains() side effect #453

Closed MattGill98 closed 2 years ago

MattGill98 commented 3 years ago

This line is supposed to check if a list is immutable:

https://github.com/eclipse/microprofile-open-api/blob/b7a437d9efd7ad113895ceb4ba8a1f10614fd779/tck/src/main/java/org/eclipse/microprofile/openapi/tck/ModelConstructionTest.java#L421-L422

The test method adds a few default Tag objects beforehand, and this one has no unique data. This means that if the OpenAPI implementation properly implements equals() andhashCode() methods it will actually fail this test, as even if the list is immutable it will contain the previously added Tag (with identical fields).

Is this intended behaviour?

MikeEdgar commented 3 years ago

@MattGill98 , this is a good catch and I suspect the answer is not straight forward. Something worth discussing for future versions of the specification may be whether the equals and hashCode contract are defined at the MP spec level. I suppose currently it is implicitly defined to be unimplemented for at least some of the model objects.

aubi commented 3 years ago

@MikeEdgar The Tag class has the "name" field, which will definitely be a part of the equals -- can the field be set in the test? From Tag's javadoc: "It is a REQUIRED property unless this is only a reference to a tag instance."

Tag otherTag  = createConstructibleInstance(Tag.class); 
otherTag.setName("user");
checkListImmutable(o, OpenAPI::getTags, otherTag);