Jaxrs-api: Clarify Configuration#getClasses() return value

Created on 29 Aug 2017  路  28Comments  路  Source: eclipse-ee4j/jaxrs-api

It must be obvious what is the difference between Application#getClasses and Configuration#getClasses.

Configuration version of that method should return current configuration including auto-discovered features etc; it is not clear (from the javadoc) whether it should include root resource classes. Santiagos point is that resource classes shouldn't be included.

bug

Most helpful comment

I think part of the problem is that the Javadoc for Configuration is a bit client biased, perhaps due to how it was initially proposed. For example, the set returned by Configuration.getClasses is said to possibly be empty, which should not be the case with root classes.

It's clearly useful for some apps to get information about all classes, including root classes etc. I think we should update the docs to make this clear and target this change to 2.2. Any objections?

All 28 comments

@mkarg Commented
I would suggest to also clearly say whether interceptors are to be contained.

Hi! I reported this issue back then. Please see this discussion on the mailing list for details.

I would love to see this one clarified in the javadocs. So, let me sum up the discussion:

  • Application.getClasses() allows the developer to manually specify root resources and providers.
  • Configuration.getClasses() corresponds to the "runtime view". It returns classes that are active because they are either discovered automatically by the implementation, are listed in Application.getClasses() or were manually added via Configurable.register().

So the question is:

Should Configuration.getClasses() return root resource classes or not?

My personal view on this is that Configuration.getClasses() SHOULD return root resources. I don't see any good argument why it shouldn't. And that's also the behavior implemented by Jersey.

Any thoughts about this?

@chkal The issue was discussed in the EG but frankly spoken I forgot the outcome. I will dive into the archives. If it bears that the issue is really open for discussion, I will try to get Jersey, CXF and RestEasy into the boat so we can find a concensus.

Thanks, Markus. This would be great!

This one is bit tricky as it was clearly somewhat open to interpretation. I was originally of the opinion that root resource classes should not be there, but it seems many implementations have adopted the opposite approach. I agree we should look at the implementations and at some point create a TCK for this.

@spericas Do you have updated contact information of CXF and RESTeasy? AFAIK Sergey and Bill are not active anymore. So whom to ask?

@mkarg Back then Bill Burke wrote that Alessio Soldano (@asoldano) will take over as Red Hat's representative.

@mkarg @chkal , right ;-)

@asoldano: Thanks for your quick response! It would be great if you could join the new jaxrs-dev mailing list which has been created for the Eclipse EE4J subproject for JAX-RS.

@mkarg @chkal done; actually I missed that mailing list had been created... sorry

@asoldano Great to have you here! Can you please review _this_ issue, and #583, from RESTeasy's angle of view? That would be terrific! :-)

@asoldano: Great! No worries. The list has been created a few days ago. So you are actually early in the game. :smile:

@mkarg I sent a message to Sergey about the mailing list. He has actually joined RH now. I'll try to find out who is working on CXF now.

@spericas Great! Thanks a lot! The people I so far contacted from CXF all left the project meanwhile or are working on different corners of CXF.

Sergey already described the behaviour for CXF in https://javaee.groups.io/g/jaxrs-spec/message/249, it hasn't changed since then:

For the record, in Configuration.getClasses(), on the server side, CXF returns the Application.getClasses() if Application is available, empty set otherwise.

I think part of the problem is that the Javadoc for Configuration is a bit client biased, perhaps due to how it was initially proposed. For example, the set returned by Configuration.getClasses is said to possibly be empty, which should not be the case with root classes.

It's clearly useful for some apps to get information about all classes, including root classes etc. I think we should update the docs to make this clear and target this change to 2.2. Any objections?

@mkarg @spericas RESTEasy does not return root resources in Configuration#getClasses()

@asoldano: Thanks for confirming. This RESTEasy behavior is exactly what I discovered back then.

@asoldano Would you be opposed to the idea of broadening the return value to include more classes?

@spericas not necessarily, it would be good to mention usecases that justify / require this, though.

@asoldano See the rationale from @chkal above. Couple of reasons: (1) developers may be interested in the complete runtime view of the system (2) implementations aren't consistent, so a clarification is needed.

@spericas I agree on (2). What I was really looking for was actual examples of usecases needing the changes, but yes, complete runtime view...

@chkal How to go on?

@mkarg I'll prepare a pull request in the next days.

@chkal Cool! :-)

Please see #621 for the pull request

Fixed by #621.

Was this page helpful?
0 / 5 - 0 ratings

Related issues

sdaschner picture sdaschner  路  5Comments

kwsutter picture kwsutter  路  11Comments

rfelcman picture rfelcman  路  6Comments

derekm picture derekm  路  12Comments

chkal picture chkal  路  10Comments