Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Updated to support Apache CXF 3 #5

Open
wants to merge 2 commits into
base: master
Choose a base branch
from

Conversation

dennisa
Copy link

@dennisa dennisa commented Mar 26, 2015

This is n initial stub at getting the guice-cxf to run against Apache CXF 3

@jakub-bochenski
Copy link
Owner

Thanks a lot.
Can you explain the changes in Matchers - I'm not sure why those were needed?

Also I think we should bump the version with this change

@dennisa
Copy link
Author

dennisa commented Mar 27, 2015

Yeah the AnnotationUtils was modified to also pass the Service class so I
modified it to accept the instace of the service with the Method in
context. Perhaps you can suggest a better way without depending on cxf's
AnnotationUtil
On 28/03/2015 5:20 AM, "jakub-bochenski" [email protected] wrote:

Thanks a lot.
Can you explain the changes in Matchers - I'm not sure why those were
needed?

Also I think we should bump the version with this change


Reply to this email directly or view it on GitHub
#5 (comment)
.

@jakub-bochenski
Copy link
Owner

Going with getAnnotatedMethod(m.getDeclaringClass(), m) sounds like a better idea although I'd have to check why did they remove the old signature (probbably to handle some CGLIb-like shenanigans).

I don't think your approach with passing any() there would work - I mean you'd be effectively doing getAnnotedMethod(AnyMatcher.class, m) am I right?

@dennisa
Copy link
Author

dennisa commented Mar 27, 2015

Ok yeah you're right. Maybe your approach is better. What is the general
intention of that matcher anyhow? Is it to find all methods that have a
particular annotation?
On 28/03/2015 7:51 AM, "jakub-bochenski" [email protected] wrote:

Going with getAnnotatedMethod(m.getDeclaringClass(), m) sounds like a
better idea although I'd have to check why did they remove the old
signature (probbably to handle some CGLIb-like shenanigans).

I don't think your approach with passing any() there would work - I mean
you'd be effectively doing getAnnotedMethod(AnyMatcher.class, m) am I
right?


Reply to this email directly or view it on GitHub
#5 (comment)
.

@jakub-bochenski
Copy link
Owner

Yes we want to install the advice that would provide subresource instances from Guice, as described here

PS. To be more clear: the matchers are general purpose (for finding a JAX-RS resource method) but we use them to scan for JAX-RS resouce methods annoteted with @Injected

@dennisa
Copy link
Author

dennisa commented Mar 27, 2015

Ok I might spend a bit of time un derstanding this code a bit. Thats also
probably why it works for us currently as we don't use the subresource
injection. Meanwhile I'll change it to your suggestion as that is more
correct than it is now.
On 28/03/2015 8:23 AM, "jakub-bochenski" [email protected] wrote:

Yes we want to install the advice that would provide subresource instances
from Guice, as described here
https://github.com/jakub-bochenski/guice-cxf/wiki/Features#subresource-injection


Reply to this email directly or view it on GitHub
#5 (comment)
.

@dennisa
Copy link
Author

dennisa commented Mar 29, 2015

Have reverted the change to the Matcher and used the getDeclaringClass instead

@jakub-bochenski
Copy link
Owner

It looks like the argument was removed in https://git-wip-us.apache.org/repos/asf?p=cxf.git;a=commit;h=30390cc755f58eab3b346dc7b035e6286d6662b8 to handle CXF-6078.

I have to think through how to handle this - then I'll merge your PR

@dennisa
Copy link
Author

dennisa commented Mar 30, 2015

Ok thanks
On 31/03/2015 8:49 AM, "jakub-bochenski" [email protected] wrote:

It looks like the argument was removed in
https://git-wip-us.apache.org/repos/asf?p=cxf.git;a=commit;h=30390cc755f58eab3b346dc7b035e6286d6662b8
to handle CXF-6078 https://issues.apache.org/jira/browse/CXF-6078.

I have to think through how to handle this - then I'll merge your PR


Reply to this email directly or view it on GitHub
#5 (comment)
.

@jakub-bochenski
Copy link
Owner

Update: looking at the source of com.google.inject.internal.ProxyFactory there might be a hacky solution -- I still havent decided wether to try to hack it or just leave a disclaimer subresouce injection won't handle CXF-6078

@dennisa
Copy link
Author

dennisa commented Apr 1, 2015

Ok thanks for the update. Just keep me informed about this please.
On 2/04/2015 6:30 AM, "jakub-bochenski" [email protected] wrote:

Update: looking at the source of com.google.inject.internal.ProxyFactory
there might be a hacky solution -- I still havent decided wether to try to
hack it or just leave a disclaimer subresouce injection won't handle
CXF-6078


Reply to this email directly or view it on GitHub
#5 (comment)
.

@taurus227
Copy link

taurus227 commented Aug 18, 2017

Forked from dennisa and tried to make it run. We're not using subresource injection either, so in theory it should work.

But it doesn't, for the /rest URL my normal servlet is displayed, even though on startup I get this in the console:

22:53:42.939 [localhost-startStop-1] INFO  org.apache.cxf.endpoint.ServerImpl - 
Setting the server's publish address to be /rest

If I remove this line form MyServletModule :

        serve("/*").with(MyServlet.class);    // this is my usual servlet

then I get error 404 for the /rest URL.

Changed some of the dependency versions in my repo to the following: maven-compiler-plugin 3.6.2, guice 4.1.0, cxf 3.1.12.

Created the servlet in the descendant of GuiceServletContextListener. Not sure if this is correct, but I couldn't find a better place.

Here's my code:

@WebListener
public class GuiceServletConfig extends GuiceServletContextListener
{
    protected static Injector injector;

    @Override
    public void contextInitialized(final ServletContextEvent servletContextEvent) {
        final ServletContext servletContext = servletContextEvent.getServletContext();
        super.contextInitialized(servletContextEvent);

        final JAXRSServerFactoryBean sf = getInjector().getInstance(JAXRSServerFactoryBean.class);
        sf.setResourceClasses(MyRestServiceImpl.class);
        sf.setBindingId(JAXRSBindingFactory.JAXRS_BINDING_ID);
        sf.setAddress("/rest");
        final Server myServer = sf.create();
    }

    @Override
    protected Injector getInjector()
    {
        if (null == injector) {
            injector = Guice.createInjector(getModules());
            final SecurityManager securityManager = injector.getInstance(SecurityManager.class);
            SecurityUtils.setSecurityManager(securityManager);
        }
        return injector;
    }

    private List<Module> getModules()
    {
        final List<Module> modules = new ArrayList<>();
        modules.add(new MyCXFServerModule());
        modules.add(new MyServletModule());
        return modules;
    }
}

public class MyCXFServerModule extends CXFServerModule
{
    @Override
    protected void configure()
    {
        serve().atAddress("/rest");
        publish(MyRestService.class);   // interface, implementation will be bound in the other module
        writeAndReadBody(JAXBElementProvider.class);
//        writeAndReadBody(JSONProvider.class);
//        mapExceptions(ApplicationExceptionMapper.class);
    }
}

public class MyServletModule extends ServletModule
{
    @Override
    protected void configureServlets()
    {
        bind(MyRestService.class).to(MyRestServiceImpl.class);

        serve("/*").with(MyServlet.class);    // this is my usual servlet
        ...
    }
    ...
}

@Path("serv")
public interface MyRestService
{
    @GET
    @Path("mydto")
    @Produces(MediaType.APPLICATION_XML)
    @Valid @NotNull
    public MyDTO get();

    @POST
    @Path("mydtos")
    @Consumes(MediaType.APPLICATION_XML)
    @Valid @NotNull
    Response add(@Valid MyDTO myDto);
}

Any ideas what am I missing or doing wrong?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants