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

Combine @URLPattern and @ForwardTo into a single new annotation @Join #60

Closed
lincolnthree opened this issue Jul 16, 2012 · 7 comments
Closed
Assignees

Comments

@lincolnthree
Copy link
Member

No description provided.

@chkal
Copy link
Member

chkal commented Jul 18, 2012

Hey there,

I just started working on this. But unfortunately it seems to be harder than I initially thought. I'm commenting on this ticket to keep you up to date and to sort my own thought regarding this. :)

Much of the existing handler codes assumes that we are building the rule using a RuleBuilder which isn't the case any more. I'm not completely sure how to handle that in a consistent way.

A very central part of the problem seems to be the ClassContext currently provides the handlers a RuleBuilder that is created by the annotation scanning API, handed over to the handlers and then added to the configuration if the handlers added any conditions or operations to it. That is essential the currently way the annotation handler API works.

This concept won't work any more because Joins are custom rule implementations that work completely different. At first I thought about letting ClassContext expose both a ConditionBuilder and a OperationBuilder for the handlers to work with, but I think that also won't work correctly for Joins.

I think what is currently missing is a simple API to add conditions and operations to rules (Joins or RuleBuilders).

Thoughts?

Christian

@lincolnthree
Copy link
Member Author

Good question. What do you think this API would look like? I don't particularly like the idea of the Rule interface being modifiable. I prefer it to remain immutable.

@chkal
Copy link
Member

chkal commented Jul 18, 2012

I agree. The Rule interface shouldn't be changed. It should be immutable.

Join and RuleBuilder are currently very focused on a fluent style of configuration. That makes it hard to use the API in the handlers. See for example this code to add a condition to a rule:

https://github.com/ocpsoft/prettyfaces/blob/master/annotations/src/main/java/org/ocpsoft/prettyfaces/annotation/handlers/RolesRequiredHandler.java#L24

It works, but it's not nice. And essentially this code creates a new composite condition and sets this as the "main" condition of the rule. But Join doesn't provide something like when() to overwrite the existing condition. And I don't think it should.

What about adding an interface like this:

public interface Something {
    void addCondition(Condition c);
}

Both Join and RuleBuilder could implement this. At first I thought that ConditionBuilder could be used for this, but it works differently and use for useful for the fluent API.

Just my thought on this. Haven't thought about all the details yet. :)

chkal added a commit to chkal/rewrite that referenced this issue Jul 28, 2012
@ghost ghost assigned chkal Jul 28, 2012
@chkal
Copy link
Member

chkal commented Jul 28, 2012

@chkal chkal closed this as completed Jul 28, 2012
@lincolnthree
Copy link
Member Author

I like it!

I have an idea.

Do you think we can remove the need for the target path if the user has
specified a @URLAction on the bean? Basically create a simple action method
for a URL. Similar to using the Invoke.typesafe()?

Just a thought :) Thinking of ways we could eliminate the need for EL and
or JSF here. Trying to think pure CDI.

~Lincoln

On Sat, Jul 28, 2012 at 1:40 AM, Christian Kaltepoth <
[email protected]

wrote:

Done! See ed4c99dde69409c44b995698d7c523052a55e094 in the PrettyFaces
repository.

Looks nice and simple:

https://github.com/ocpsoft/prettyfaces/blob/master/annotations/src/test/java/org/ocpsoft/prettyfaces/annotation/basic/BasicJoinBean.java


Reply to this email directly or view it on GitHub:
#60 (comment)

Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."

@lincolnthree
Copy link
Member Author

Btw. You know the only reason more ideas happen is because of good ideas :)
Joining @path and @forwardto was a great idea :)

On Sat, Jul 28, 2012 at 11:03 PM, Lincoln Baxter, III <
[email protected]> wrote:

I like it!

I have an idea.

Do you think we can remove the need for the target path if the user has
specified a @URLAction on the bean? Basically create a simple action method
for a URL. Similar to using the Invoke.typesafe()?

Just a thought :) Thinking of ways we could eliminate the need for EL and
or JSF here. Trying to think pure CDI.

~Lincoln

On Sat, Jul 28, 2012 at 1:40 AM, Christian Kaltepoth <
[email protected]

wrote:

Done! See ed4c99dde69409c44b995698d7c523052a55e094 in the PrettyFaces
repository.

Looks nice and simple:

https://github.com/ocpsoft/prettyfaces/blob/master/annotations/src/test/java/org/ocpsoft/prettyfaces/annotation/basic/BasicJoinBean.java


Reply to this email directly or view it on GitHub:
#60 (comment)

Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."

Lincoln Baxter, III
http://ocpsoft.org
"Simpler is better."

@chkal
Copy link
Member

chkal commented Jul 29, 2012

So, you mean to remove the to part and instead only invoke the method? Which then creates the response?

Something like this:

@URLPattern("/user/{id}")
public class MyBean {

  @Action
  public String action( @ActionParam int id ) {
    // Do something
    return "....";
  }

}

I think this should be very easy. @URLPattern is still there. We would just have to create a new annotation for which the corresponding handler adds an operation that perform the method invocation and then sends the response. So the method should create the response content, right? Using a string like in the above example would certainly be to restricting. The method should also be able to set the content type and other stuff.

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

No branches or pull requests

2 participants