Great analysis and explanation! Thank you :) I think that having a scenario where you scan and use both scanned and app resources might be tricky. We don’t control the application libraries, so we may end up scanning more resources than the user actually wants.
We already know which endpoints we want to add and we already have the tomee.mp.scan option to enable / disable MP. How about if we add the MP rest classes in org.apache.tomee.microprofile.TomEEMicroProfileListener#processApplication before any logic and then let them be removed again with the logic of the servlet root in /*? Cheers, Roberto > On 16 Feb 2019, at 11:19, j4fm <[email protected]> wrote: > > I've gone through the CXF resource adding behaviour. I think the logic can > be improved. This is how I see it works as it is: > > (Scenario for "MyApplication extends Application" class) > > It looks like the openejb.jaxrs.application is intended to choose whether to > use MyApplication's classes/singletons or not but actually in both cases of > true/false it invokes the same sort of logic that uses MyApplication's > classes/singletons if provided but restClass if not. (There is also no > option of both). > > 1. If openejb.jaxrs.application is true... > a. ...then it uses the classes/singletons returned by the MyApplication > instance if it returns some. In this case it does not add what's in > restClass (i.e. scanned classes which does include MP endpoints). > b. ...if the MyApplication class does not return any classes/singletons, > it processes restClass (i.e. scanned classes which includes MP endpoints and > in this case the same classes as MyApplication too) > 2. If openejb.jaxrs.application is false, it uses the fullServletDeployment > logic. This setting and also flag name (!deploymentWithApplication) > indicate this should be without using MyApplication, i.e. using only the > runtime scanned classes. But the comments header in fullServletDeployment > indicates that it will use MyApplication if it exists. That's the same as > the logic in 1 above and when checking the code in fullServletDeployment, it > does in fact have the same behaviour as 1a and 1b. (i.e. if MyApplication > provides classes/singletons, it ignores restClass) - "useApp = useApp || > classes.size() + singletons.size() > 0"; So this seems to be a bit of an > odd setting in this scenario but I'm sure there are other scenarios too. > > So right, now to get scanned classes picked up (i.e. MP resources) when > providing MyApplication resource classes, we'd have to change MyApplication > to return no classes/singletons. This means changing the openapi TCK and > breaking portability for MyApplication. So this is not an option. > > If I put existing behaviour and backwards compatibility to one side for a > moment, I'm thinking a setting such as the following would be best... > > openejb.jaxrs.use_rest_classes_from = scanned|application|both|auto > > scanned = Use only rest classes detected by runtime scanner. > application = Use only rest classes provided by MyApplication. > both = Use the combined unique list of rest classes from scanned and > application. > auto = If MyApplication provides rest classes, use them; Otherwise, use > scanned (current behaviour regardless of openejb.jaxrs.application setting) > > Choice of default: > 1. Thinking about backwards compatibility again, I guess this new setting > could be introduced with the default value of "auto". But that will not > resolve the openapi test case or add the MP endpoints to any rest > application that provides some rest classes itself... to resolve those, then > either "scanned" or "both" would work. We could set those just for openapi > test case and the rest app I have that extends Application that I would like > to see MP endpoints. > > 2. A default value of "both", could be best as default as it enables app > portability but also server configured resources, but would change existing > behaviour (users see MP endpoints added, possibly others in some apps). It > would be easy for a user to switch back to "auto" for existing behaviour > though. > > (While I see openejb.cxf-rs.cache-application wraps MyApplication's rest > classes/singletons when provided, I don't see it taking into account scanned > classes. While it could be updated to do that, I'm not sure the "caching" > setting should be confused with determining which source(s) to use for rest > classes - I don't think it does today either.) > > Am interested in your thoughts, I think this would provide missing > flexibility as well as fix the openapi test case at least. If agreeing, > would you prefer default 1 or 2? I understand I'm pretty fresh to this code > base so there may be functionality/behaviours around the above I haven't > seen and am not aware of. > > Depending on your views, I would be happy to create a JIRA for it. > > > > -- > Sent from: http://tomee-openejb.979440.n4.nabble.com/TomEE-Dev-f982480.html
