Skip to content
This repository has been archived by the owner on Jun 14, 2018. It is now read-only.

Support specifying resources per application #26

Open
fabiocarvalho777 opened this issue Jul 21, 2016 · 12 comments
Open

Support specifying resources per application #26

fabiocarvalho777 opened this issue Jul 21, 2016 · 12 comments
Assignees

Comments

@fabiocarvalho777
Copy link
Member

fabiocarvalho777 commented Jul 21, 2016

Support specifying JAX-RS resources per JAX-RS application, based on section 2.3.2 in the JAX-RS 2.0 specification.

"If either getClasses or getSingletons returns a non-empty collection then only those classes or singletons returned MUST be included in the published JAX-RS application"

That must be respected even in the case of multiple JAX-RS applications.

Important note: this only applies to resources (not providers). Specific providers registration per resource can be achieved by using JAX-RS DynamicFeature.

@fabiocarvalho777 fabiocarvalho777 self-assigned this Jul 21, 2016
@fabiocarvalho777 fabiocarvalho777 modified the milestone: 2.2.0-RELEASE Aug 9, 2016
@fabiocarvalho777 fabiocarvalho777 removed this from the 2.2.0-RELEASE milestone Aug 29, 2016
@cwiejack
Copy link

cwiejack commented May 9, 2017

Hi,
first of all, thank you for your work. I just found your project and it helped me very much!
I'm now at the point, where I want to have different spring beans used for different applications.

After a short look into the code, I came up with the idea to add some kind of a filter annotation to the spring beans and then using that filter in ResteasyEmbedededServletInitializer#createApplicationServlet to only pass the matching beans to the given Application class. I'm not sure if this will work, but if you are interested I would give it a try and submit a pull request if it works.

@fabiocarvalho777
Copy link
Member Author

Hello Christian,

Thanks for offering to contribute to this project.
Please feel free to send a PR with your contribution. Please make sure you follow this document.
Regarding this issue in particular, the API itself is very important. Ideally, adding extra and specific API should be avoid at all costs. JAX-RS spec should be followed first of all, and then RESTEasy API.
If you think you can contribute for this issue by following that, please feel free to send a PR.

I have to say also that I have actually already started working on this issue a long time ago, but I didn't finished it, I got side tracked.

Anyways, thanks for offering to contribute.

@cwiejack
Copy link

Hi,
I have some ideas how it could be done. Perhaps I find some time and if I accomplish something I will send you a note and/or a PR.

@fabiocarvalho777
Copy link
Member Author

Cool, thanks!

@cwiejack
Copy link

Hi Fabio,
as always things are more complicated than one would have thought. I thought that your code in the class ResteasyEmbeddedServletInitializer is responsible for binding the spring beans annotated with "@path" to the Jaxrs-Application. But I realized this is not the case. Even if I pass an empty list of resources/providers to the ResteasyApplicationBuilder (ResteasyEmbeddedServletInitializer#createApplicationServlet) all your tests pass.

ConstructorArgumentValues values = new ConstructorArgumentValues();
values.addIndexedArgumentValue(0, applicationClass.getName());
values.addIndexedArgumentValue(1, path);
values.addIndexedArgumentValue(2, Collections.emptySet());
values.addIndexedArgumentValue(3, Collections.emptySet());
applicationServletBean.setConstructorArgumentValues(values);

I started some digging into the code and as far as I currently understand, it's the SpringBeanProcessor from resteasy itself where the "magic" happens.

You mentioned you started with this issue yourself some time ago. Do you remember your approach or do you have some code left? From my point of view the code from resteasy itself is not that easy to read :), and I'm not sure if I am on the right track.

@fabiocarvalho777
Copy link
Member Author

Hello Christian,

Yes, unfortunately all of this is not straightforward.

I believe the unfinished solution I had was on the right track though, but it's been a while.
So, I have created this new branch here just so we can work with this issue 26.

The only problem though is that right now I am working with two other issues in parallel, so I am little busy. But, hopefully sometime this week I will have some time to push my unfinished changes to that branch, so we can go from there. I still have it in my laptop, I just need to merge it properly, since its base code is old.

Does that sound like a good plan?

Have a good week.

@cwiejack
Copy link

Hi,
yes this sounds like a really good plan. There is no hurry. If I find some time, I will look further into the code and try to understand what is really happening there ;) Keep me up to date if you manage to merge the code properly.
Regards
Christian

@fabiocarvalho777
Copy link
Member Author

Hello,

I believe I finished implementing this feature (at least to make it functional). Here is the commit (not in the main development branch yet).

There are some pending tasks to complete it entirely, I documented them the commit. Here they are:

  1. Review carefully JAX-RS 2.0 specification to make sure everything is in compliance and designed accordingly.
  2. Add some meaningful comments around the code that implement this feature.
  3. Add a sample test case (using an additional JAX-RS application class) to the sample application. This application should have its own resources (two non Spring bean classes), to be registered manually, and should not include the Echo resource, which is a Spring bean. One of its resource should be added as class and the other as singleton. Both should work fine.
  4. Integration test to assure item 1 works perfectly in the sample application.
  5. Integration test to assure the two resources registered manually do not get included to the original application, which should still have only the Echo resource only.
  6. Add an item to mention this feature in the features list int the README md file.
  7. Add an section to describe with details this feature in the USAGE md file.

There is one more that is important: fully understand the usage for getClasses and getSingletons. Currently, the assumption is that both are used to register resources (and this partial solution was implemented based on that). However, if that is true, who would providers be manually registered?

Christian, I plan to keep working on this issue and conclude it. But feel free to take a look at it if you want.

Fábio.

@cwiejack
Copy link

Hi Fabio,
thanks for your work. I have taken a look at your changes. I have taken a similar approach on my first attempt earlier this week. In my simple testcase (https://github.com/cwiejack/resteasy-spring-boot/tree/issue_26_appdefinition) this doesn't work. I'm currently on vacation, but next week I should have some time to take a closer look.
Regards
Christian

@fabiocarvalho777
Copy link
Member Author

Cool. If you work on it, please continue out of this issue_26 branch and my latest changes on it.

@cwiejack
Copy link

Hi Fabio,

based on your latest changes on issue_26 branch I created a small test case to show you the problem I also faced with my implementation.
Here is a little summary what I have done:

  • create a new Spring-Boot-Application with a new JaxRS-Application
  • Create a non springbean endpoint "EchoV1" and use in in "getClasses" of the JaxRS-Application
  • Create another endpoint "EchoV2" and mark it as a spring component

If I understand the spec you quoted above correctly the EchoV2 Endpoint must not be reachable for the given JaxRS-Application because it is not defined in getClasses. But unfortunately it is.

I attached a small patch file with my changes.

I haven't yet fully understand why this is the case. But I think the SpringBeanProcessor from resteasy is the right place to start looking.

I will keep you informed if I find something.

_26__current_implementation_does_not_fulfill_the_JAX_RS_spec.zip

@fabiocarvalho777
Copy link
Member Author

Thanks Christian,

Yes, everything you said makes sense to me, that is how I understand it too.
Your test should have worked, if it didn't, there is definitely something still wrong in there.
Let me know if you find the root cause or more clues.
I will also take a look at it when I have some free time.

Thanks!

@fabiocarvalho777 fabiocarvalho777 changed the title Support specifying resources and providers per application Support specifying resources per application Jun 7, 2017
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Projects
None yet
Development

No branches or pull requests

2 participants