eclipse / microprofile-rest-client

MicroProfile Rest Client
Apache License 2.0
143 stars 72 forks source link

SimpleFeature shouldn't have @Provider on it #225

Closed kenfinnigan closed 5 years ago

kenfinnigan commented 5 years ago

SimpleFeature currently has @Provider present on it which results in it being auto-discovered in the CDI injection test.

Depending on the order the tests are executed this can lead to issues whereby the RestClientBuilder believes the provider is already registered.

It's safer to not have the @Provider annotation and just register it manually.

andymc12 commented 5 years ago

@kenfinnigan Good catch. Should we also remove the @Provider annotation from InjectedSimpleFeature? IIUC, the annotation is also unnecessary because the FeatureProviderClient interface has this annotation: @RegisterProvider(InjectedSimpleFeature.class)

wdyt?

kenfinnigan commented 5 years ago

We could do that too

kenfinnigan commented 5 years ago

would you like that in these PRs too?

andymc12 commented 5 years ago

Yeah, that would probably be simplest. If you'd rather do them in separate PRs though, that's fine too. Thanks for catching this.

kenfinnigan commented 5 years ago

@andymc12 I've updated both PRs