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

Feature/injectable request params #120

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

Conversation

royteeuwen
Copy link
Contributor

Hey guys,

I have build this PR onto #119 , so please review that one first.

What I did in this one is go further on the request annotations, I have also added a RequestParameter annotation, where you can inject your request parameters in your slice model. Secondly I have also made it possible to actually adapt a request itself to the slice resource, where you could then achieve the following result(s):

Mapping to a servlet

@Component(
        service = Servlet.class,
        property = {
                "sling.servlet.paths=/bin/my-servlet"
        }
)
public class MyServlet extends SlingAllMethodsServlet {

    @Override
    protected void doGet(@Nonnull SlingHttpServletRequest request, @Nonnull SlingHttpServletResponse response) throws ServletException, IOException {
        MyServletModel myServletModel = request.adaptTo(MyServletModel.class);
    }
}

@SliceResource
public class MyServletModel {

    @RequestParameter
    private String parameterOne;

    @RequestParameter
    private List<String> parametersTwo;

}

Mapping to a resource type

@Component(
        service = Servlet.class,
        property = {
                "sling.servlet.resourceTypes=slice-project/components/page/content",
                "sling.servlet.selectors=myselector"
        }
)
public class MyServlet extends SlingAllMethodsServlet {

    @Override
    protected void doGet(@Nonnull SlingHttpServletRequest request, @Nonnull SlingHttpServletResponse response) throws ServletException, IOException {
        MyPageModel myServletModel = request.adaptTo(MyPageModel.class);
    }
}

@SliceResource
public class MyPageModel {

    @RequestParameter
    private String parameter;

    @JcrProperty
    private String title;

}

…y-use=${'com.cognifide.slice.TestModel' @ myAttribute='aValue'} to read out the attribute
…ys available, in contrast of sling servlet request)
… from a request immediatly instead of using request.getResource(). This way you could adapt your request in a servlet to a model with request parameters as fields.
@royteeuwen royteeuwen force-pushed the feature/injectable-request-params branch from 6ba0416 to cab9f7a Compare October 13, 2017 19:39
@coveralls
Copy link

Coverage Status

Coverage decreased (-1.4%) to 65.506% when pulling cab9f7a on royteeuwen:feature/injectable-request-params into 382453c on Cognifide:master.

@mmajchrzak
Copy link
Contributor

This a very nice feature! I'll review the code shortly, but having a quick glance it looks good too. Meanwhile, could you please create corresponding tickets in jira? This would be helpful. Cheers

@royteeuwen
Copy link
Contributor Author

royteeuwen commented Oct 17, 2017 via email

@rzasap rzasap requested review from maciusio and removed request for maciusio October 18, 2017 06:23
@@ -66,6 +60,12 @@
private ChildrenFieldProcessor childrenFieldProcessor;

@Inject
private RequestAttributeProcessor requestAttributeProcessor;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm wondering if this is a good place for this kind of logic. Conceptually, your class needs to be annotated with @SliceResource only if you want to map its fields from Sling resource. I can imagine a scenario when you don't need mapping to happen but still want to have request parameters/attributes injected. The mapper should not mix these things and fieldProcessors should rely on resource.

If not mapper, then what? I'd probably made RequestAttributeProcessor (and the other one) as Guice providers rather than FieldProcessor. This would make the design cleaner. The disadvantage would be that you'd need to use @Inject annotation whenever you want the values to be injected and probably specify the name of an attribute (I'm not sure if Guice will give you information about the field name - probably no):

@Inject
@RequestAttribute("attribute")
private Boolean attribute;

Could you try with this approach?

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