TCK Bugs?

[MattGill98]

Hi, I was hoping someone wouldn't mind helping me clarify a few problems I've encountered when working with the TCK.

Firstly, the adding of the microprofile-config.properties seems strange. In the Config API TCK, this file is added using this line:

.addAsWebInfResource(mpConfig, "classes/META-INF/microprofile-config.properties");

This adds the file to WEB-INF/classes/META-INF, which is on the application class path.
In the OpenAPI TCK however, this file is added using this line:

.addAsManifestResource("microprofile-config.properties", "microprofile-config.properties");

This adds the file to META-INF, which isn't on the application class path. Since the Config API doesn't test for this, it's behaviour is presumably undefined. Should this be an added test to the Config API TCK, or a required change in the OpenAPI TCK?

My second problem is regarding the AirlinesAppTest. This application seems to be invalid, and as such may not deploy to all application servers. The endpoint in question involves these two methods in the ReviewResource:

@GET
@Path("{user}")
@Produces("application/json")
public Response getReviewByUser(@PathParam("user") String user) {}

@GET
@Path("{id}")
@Produces("application/json")
public Response getReviewById(@PathParam("id") String id) {}

There's no problem with this from the perspective of the OpenAPI, as they will appear as different endpoints. For the JAX-RS server though these endpoints are functionally the same, and so may not deploy correctly on all application servers. In my opinion this should not cause a failed test however, and maybe one of these endpoints should be fixed to have a different @Produces type. Is this an accurate evaluation?

Thanks in advance,

Matt

[arthurdm]

hey @MattGill98 - thanks a lot for the feedback.

For the first issue, I believe it's a gap in the MP Config TCK, as its design outlines that META-INF/microprofile-config.properties should be picked up by the config processor.

I agree with your observation of the second issue. I propose that we update the path of getReviewByUser to be @Path("users/{user}"), which would solve this.

Thoughts?

[MattGill98]

Hi Arthur,

Thank you for your prompt response. That's not a problem. That being the case, I'll raise an issue for the Config API project!

That would work absolutely fine. I'll make a pull request making this change.

Kind regards,

Matt

[MattGill98]

Hi @arthurdm,

Just following up on the first issue again. I found that the issue raised here on the Config API project specified that for a WAR file the META-INF location should be WEB-INF/classes/META-INF, since it's on the classpath. Does this change anything with regard to the first issue?

Kind regards,

Matt

[smillidge]

My understanding of the Config API is that for a war file the properties should be under WEB-INF/classes/META-INF see eclipse/microprofile-config#268

[arthurdm]

thanks for the discussion guys.

My view on this subject is that the MP OpenAPI spec has defined META-INF as the place that we require users to place their static OAS3 files (if any), so it wouldn't make sense to prevent them from also putting their properties files in that directory. We have samples in the spec that demonstrate the properties file being picked up from META-INF/, but perhaps we can make that language more clear for version 1.1.

They can too, of course, put their properties files in anywhere else where MP Config picks them up (i.e they can pass the key/value pairs as env variables too), but I don't want to give the impression that META-INF/ is not supported by the MP OpenAPI spec - even if that's not a default location for MP Config.

From a vendor's perspective, to be compliant with MP OpenAPI they must look at META-INF for the static OAS3 docs, as well as property files - maybe that means they need to create a custom MP Config source (we didn't), it depends on their MP Config impl and overall their app server environment (e.g. some app servers will support OSGi web apps too, etc, so classpath settings may vary).

I hope that clarifies it, but if not, I will be happy to continue the discussion here or in the next OpenAPI hangout.

[smillidge]

I am pretty sure there is an explicit test in the Config api TCK that checks that in a war the properties file is in WEB-INF/classes/META-INF so not sure this will work

[MattGill98]

Hi @arthurdm,

Thanks for your reply. I see that the META-INF directory is specified by the specification, but it doesn't seem to explicitly say that whether it should be /META-INF or WEB-INF/classes/META-INF. As this is the case, the only clarification we have to work off is in eclipse/microprofile-config#268, where it states that it should be the latter as it's in the classpath. Until the former is explicitly added as well, I would argue that the changes made in #220 are more inkeeping with the specification as it stands.

[arthurdm]

Right - I agree that the MP Config TCK is checking for the WEB-INF/classes/META-INF as a default config source.

My argument is that the MP OpenAPI spec (not MP Config spec) defined that META-INF (top level in the web module, either WAR or WAB/OSGi) is a place where vendors need to look in-addition to any of the default MP Config sources. I think that wording needs to be expressed more explicitly in the MP OpenAPI spec, which I will open a separate issue for.

[arjantijms]

My argument is that the MP OpenAPI spec (not MP Config spec) defined that META-INF

Just curious, but what was the thought process for that particular directory to be chosen? META-INF in a war archive is a rather unusual location really.

[arthurdm]

hey @arjantijms - it was deemed a reasonable place for metadata (i.e. OpenAPI description) of the app.

[arthurdm]

PR #218 (first issue) was merged - thx again @MattGill98

I have opened #221 to continue the work outlined here in terms of clarifying the location of the specs.

Thanks everyone for the feedback! Closing this issue.

[OndroMih]

@arthurdm

For the first issue, I believe it's a gap in the MP Config TCK, as its design outlines that META-INF/microprofile-config.properties should be picked up by the config processor.

Just a note - I think you misunderstood the MP config design because it clearly says that the path should be on the classpath, while in a WAR, this is in WEB-INF/classes and not in the root. It's also mentioned in the spec itself besides the README. So there's no point in supporting META-INF in the root of a WAR file. I believe this applies to OpenAPI and I explained what I suggest to with that in #221

[arjantijms]

Hi @arthurdm

it was deemed a reasonable place for metadata (i.e. OpenAPI description) of the app.

Are there any minutes of that decision, and the people who deemed that, had any of them used or worked with war archives before? My feeling says someone may have thought about a regular .jar file and didn't know exactly what a .war file is. As I think this is a spec bug, I'll file a separate issue for this where it might be better to continue this conversation.