remove @RequestScoped @ConfigProperties bean and the associated test cases

[JHahnHRO]

The test cases are fundamentally broken (see issue #762) and cannot possibly work the way they are written.

[eclipse-microprofile-bot]

Can one of the admins verify this patch?

[Emily-Jiang]

The TCK is absolutely fine. Removing the scope support is a big change. What you raised is an implementation issue.

[Emily-Jiang]

I am going to close this PR by 5th July if there is no other responses on this PR.

[Emily-Jiang]

Close this PR as per my comments above. Please reopen if you still think this should be discussed again.

[JHahnHRO]

@Emily-Jiang I'm sorry for the late reply. I had little time for old github issues because I was moving during the last few weeks.

As explained in the other issue: This is not just an implementation issue

TCK testcase is fundamentally broken, even if we were of the opinion that @RequestScoped @ConfigProperties beans should be allowed. The assertion in line 145 will always fail for a request scoped bean, because the instance is only a client proxy, not the "real" bean instance, so the field will have its default value, not the injected value.
(I'm also unsure if the method call in line 144 would fail with a ContextNotActiveException, because the request scope does not seem to be active. But maybe that's just my inexperience with Arquillian.)

SmallRye does not fail this test "by accident", because it does not produce @RequestScoped beans at all. It replaces any @ConfigProperties bean class it finds with a synthetic and @Dependent-scoped bean. That is an implementation issue.

The PR is about removing the test case altogether, because of the discussion with @radcortez in the other issue about whether or not this is even a reasonable use case. He suggested removing it. I'm fine with keeping @RequestScoped beans in principle, but then the test needs to be re-written by someone who knows what the expected behaviour really should be. I'm not sure I know that.

For example: This whole thing came about because of my other question about no-args constructors. Consider a bean class like this:

@RequestScoped
@ConfigProperties(prefix="server")
class Server {
   String host;
   int port;

  protected Server(){
     // required because @RequestScoped is a normal scope so that the bean needs to be proxyable
  }

  @Inject
  Server(HttpServletContext context){
    // whatever constructor logic the contextual instance really needs.
  }
  
  // getters ommitted
}

The no-args constructor exists, so this does not violate the Config Spec, but I'm not clear on how I should expect it to be used. Is a MicroProfile-Config implementation supposed to always use the no-args constructor to initialize the contextual instance of this bean? That would seem a violation of the CDI Spec in the presence of the @Inject constructor. Is it supposed to use the bean constructor (i.e. the second one in the example) ? That would mean replicating parts of the CDI container's core purpose. Is it supposed to not use any constructor directly and delegating instantiation to the CDI container? Then why have the no-args requirement in the MicroProfile-Spec in the first place if it is not required in the CDI spec (in case of @Dependent or @Singleton beans) ?

I'm simply not sure what the Spec intends to achieve here so I don't feel qualified to write a replace for the testcase.

[radcortez]

Yes, I believe that making these beans @RequestScoped was a mistake. In most cases, you are not going to observe any changes in the injected values. If we solely consider the specified sources of MP Config, both Env, and Properties are immutable, and System Properties could be changed programmatically (but not really usual). We have to question if this is really useful.

Yes, users could write their own dynamic source and observe new values being loaded on each request, but how many times does that happen on an app lifecycle? Is it worth paying the price for this?