<s:link action="#{itemHome.remove(i)}">Delete</s:link>

Remember that this is the most evil way possible how you can abuse s:link action. Yes, some examples might even do this, we need to document it better.

This is a GET request that is neither safe nor idempotent. Would you like it if the Googlebot clicks on it?

The correct way is to use DELETE requests, but since browsers don't support that, we all should use overloaded POST. In other words, h:commandButton/Link with a no-argument remove() action, in a POST form that wraps the datatable, with @DataModel/@DataModelSelection on the backend.

Finally, what you have shown is not really parameters passed to method expressions, that would be:

<s:link action="#{itemHome.remove(i.id)}">Delete</s:link>

Because i.getId().toString() is evaluated at render-time. What you have shown is called s:link can do special magic tricks with the datamodel selection if you put it inside an h:datatable, just look at the generated URL with lots of magic parameters. There is an open JIRA issues that discusses removing this particular mis-feature.

It's useful for hiding the ids of your entities though.

Well thanks for the responses all. I didn't realise I could pass variables to my methods now. As that's evil, I'll leave that as the backup plan :)

Although I've got the @DataModel and @DataModelSelection set, those are set in the ItemBrowserclass and not in the ItemHome class (where I am trying to call remove) I don't think that's going to work.

So I'll take a stab at the child parameter. I gave that a quick once over earlier but never tried it. If that fails, guess I'll be taking the evil route till I figure it out.

Christian,

We used h:commandLinks which are POSTS so the GET issue wasn't really an issue.

As far as the magic tricks part - this was on Seam1.2.1.GA so it may no longer apply - the whole reason we did it was because when we used DataModels we had numerous other issues that resulted:

1. DataModelSelection not always being set correctly (oftentimes it would reset to the first item in the list).
2. Trying to reuse the same code and templated xhtml in many places resulted in naming conflicts on the DataModel when it was evaluated by Seam. Seam does it for a valid reason, but it can be difficult to workaround.
3. and a few others issues that currently escape me.

So...since we had a deadline, we took the lazy approach, gave up, and did the workaround :)

Thanks, Christian!

Christian Bauer wrote on Mar 05, 2008 03:38 PM:

<s:link action="#{itemHome.remove(i)}">Delete</s:link>

[...] This is a GET request that is neither safe nor idempotent. Would you like it if the Googlebot clicks on it? [...]

<s:link action="#{itemHome.remove(i.id)}">Delete</s:link>

Because i.getId().toString() is evaluated at render-time. What you have shown is called s:link can do special magic tricks with the datamodel selection if you put it inside an h:datatable, just look at the generated URL with lots of magic parameters. There is an open JIRA issues that discusses removing this particular mis-feature.

Sorry to come back on this, but could you please explain what's is wrong with the code

<s:link action="#{itemHome.remove(i)}">Delete</s:link>

• is it the fact that's inside a non-idempotent action in a s:link
• or that's generally a bad thing to use the datatable loop variable (don't know the real name of this) as parameter in an EL expression?

This is definitively an intuitive way of doing things - and it's very useful! So if it's inherently wrong, what's the best way of passing a table entry as parameter? By systematically using the Id of the object (in the case of an entity bean) as you suggest?

Thanks in advance for making things clearer,
Sylvain.

Use a commandLink and then instead of passing in i, pass in the primary key or ID of i.

After looking at our code again, we passed in the key, not the actual JPA object.

Sylvain Leroux wrote on Mar 07, 2008 06:44 PM:

• is it the fact that's inside a non-idempotent action in a s:link

• or that's generally a bad thing to use the datatable loop variable (don't know the real name of this) as parameter in an EL expression?

Both. It should be clear why GET is supposed to be safe and idempotent. As for the loop variable, think about it like this:

The loop variable is a temporary variable, it's only available during RENDER RESPONSE when your datatable is rendered. If you trigger another request, be it GET or POST, it's not going to be there. So there are several tricks how you can transport the row you clicked on into this subsequent request.

One is with @DataModelSelection and a POST request. The selection index will be part of the POST request body as a parameter (you can see that if you look at the rendered table HTML and the h:commandLink that triggers the POST with Javascript). On the server during request processing, Seam/JSF pull the item at this index out of the datamodel. (Which needs to be in a scope that lasts longer than a single request! PAGE or CONVERSATION are common choices.) It then injects it into your @DataModelSelection property. That's the clean solution because it uses overloaded POST for actions that might not be idempotent nor safe (which is what clients expect from POST).

The second option is s:link with an action that looks like it is passing the loop variable into the next GET request. Now, that is of course not what is happening. It's the same trick as before, just with a GET. Seam attaches a magic request parameter onto your s:link URL that says pull that index out of the datamodel and pass it into the action method as an argument. It's just a convenient shortcut. But it's conceptually wrong to use GET for that, so that is why we are discussing removing it.

In my experience, people have trouble with this because they are not fully aware of the JSF lifecycle and don't realize the scope of variables, especially variables that are only available during response rendering. There is no way how you can pass objects between request on the client. You can pass identifiers or keys (or whatever you like to call them) between requests as parameters and you need to make sure that the subsequent request can pull the stuff out of some data store that is in the right scope (data model, list, whatever) when that key needs to be resolved.

In my experience, people have trouble with this because they are not fully aware of the JSF lifecycle

I must admit this is my case...

Of course, it was surprising to be able to pass loop variables as an argument to EL expression. But since things were working like that... I thought that was some JSF/Seam white magic. So half by laziness, half by lack of time, I didn't look into the details! I think many people are just like me...

Now, I know that's not the good way of doing things!

So, thank you for your answers,
Sylvain.

The second option is s:link with an action that looks like it is passing the loop variable into the next GET request. Now, that is of course not what is happening. It's the same trick as before, just with a GET. Seam attaches a magic request parameter onto your s:link URL that says pull that index out of the datamodel and pass it into the action method as an argument. It's just a convenient shortcut. But it's conceptually wrong to use GET for that, so that is why we are discussing removing it.

Are you also deprecating this magic for the idempotent case of linking to a child view with an s:link and a method EL expression with a loop variable?

<h:dataTable value="#{someBean.children}" var="child">
  <h:column>
    <s:link value="edit"    
      action="#{anAction.editChild(child)}"/>
  </h:column>
</h:dataTable>

I see this case as equivalent to the use of @DataModelSelection, but easier to read (to me) in both the view and controller. In addition it allows right-click-open-in-new-window, which h:commandLink would not. In the case of editing children in a table, our users often expect to be able to do that in a new window.

I'm ok switching back to @DataModelSelection for this case if that's what the Seam team is suggesting as the best practice moving forward. (I definitely think Seam needs some best practice use-cases, so whatever they are, I'll code to them.)

There is no way how you can pass objects between request on the client. You can pass identifiers or keys (or whatever you like to call them) between requests as parameters and you need to make sure that the subsequent request can pull the stuff out of some data store that is in the right scope (data model, list, whatever) when that key needs to be resolved.

I tend to cringe at the idea of passing identifiers around explicitly; should I reconsider this position? Or is using an EL method parameter just not the right way to hide the identifiers (as opposed to @DataModel and friends)?

Christian Bauer wrote on Mar 05, 2008 03:38 PM:
Because i.getId().toString() is evaluated at render-time. What you have shown is called s:link can do special magic tricks with the datamodel selection if you put it inside an h:datatable, just look at the generated URL with lots of magic parameters. There is an open JIRA issues that discusses removing this particular mis-feature.

Which issue is this? I found jbseam://1734 and jbseam://2391, but they're closed.

I tend to cringe at the idea of passing identifiers around explicitly

Honestly, I think that URLs with primary keys in them is just about the only truly semantically correct way to use HTTP.

I'm speaking approximately, of course: it doesn't have to be the primary key, just some stable unique key of the entity.

Gavin King wrote on Mar 08, 2008 04:09 AM:
Honestly, I think that URLs with primary keys in them is just about the only truly semantically correct way to use HTTP.

Am I wrong, or that's the RESTful way of doing things? Or at least it enforce a resource oriented architecture (ROA). Does this advice apply to all POST requests as well as GET requests?

From the user point of view, it could be of great benefits to have URLs with primary keys: they will have an unique identifier (the URL) for any operation on that resource. But that require a browser supporting http methods like PUT or DELETE. That's currently not the case.

I tend to cringe at the idea of passing identifiers around explicitly

Maybe your problem with passing identifiers is related to security concerns. What if someone change the URL in such way that give him access to an other resource?

Since the solution based on @DataModelSelection use index on the list, as far as I understand, there's no way for the user to access resources other than those present in the corresponding @DataModel. This is a stateful way of doing things.

In the other hand, using identifiers in the URL requires to check if the user should really have access to the given resource before processing each request. This is a more stateless way of doing things. The good news is in that case you now have bookmarkable URL.

Restful architectures are stateless. And Seam is a stateful framework. So by re-reading this post again and again, I came to the conclusion that, in order to have the best of both worlds, we should:

• use stateless/restful/bookmarkable URL using resources identifiers for idempotent operations (GET requests). Not forgetting to check user rights on the resource before processing.
• use framework stateful capabilities (like @DataModel/@DataModelSelection) and a POST request for non-idempotent operations: in that case you don't need to check user rights before processing.

Even more: I feel like if any Seam conversation should begin with a GET request on a restful URI. In other words, that any entry point in an application should be a resource. I'm not sure I'm clear here (it's still a little bit confuse in my mind;). But I'm sure there's something to dig there...

Sylvain.

Sylvain Leroux wrote on Mar 08, 2008 04:04 PM:
Maybe your problem with passing identifiers is related to security concerns. What if someone change the URL in such way that give him access to an other resource? Since the solution based on @DataModelSelection use index on the list, as far as I understand, there's no way for the user to access resources other than those present in the corresponding @DataModel. This is a stateful way of doing things.

Security is not obscurity, so that needs to be resolved anyway with additional checks. No difference between providing the key/index of a datamodel item that I'm not supposed to select, or providing a database identifier I'm not allowed o load. Of course it feels more secure if the parameter in question is in the request body (POST) and not in the request URI (GET). But it's really the same thing.

• use framework stateful capabilities (like @DataModel/@DataModelSelection) and a POST request for non-idempotent operations: in that case you don't need to check user rights before processing.

No, this is not a good idea. You always need to check a request with your security policy, no matter how it is made.

Christian Bauer wrote on Mar 08, 2008 05:36 PM:
Security is not obscurity
[...]
You always need to check a request with your security policy, no matter how it is made.

I agree with that. And of course, using POST is no more secure than using GET.

No difference between providing the key/index of a datamodel item that I'm not supposed to select, or providing a database identifier I'm not allowed o load.

Yet, I'm not sure to understand that: To my mind, there is a difference:

• In the case of a database identifier, the item is identified as one in the set of all the items.
• In the case of an index or a key, the item is identified as one in the set of those I have presented to the user.

So, if I allow my user to select an item in a list, and if that selection is returned as an index in that list, the only thing I have to check is if the answer is not out of bounds. I don't think I need to check if the user has enough permissions to access that item, since that was done when I build up the list?

But maybe I missed something, so thanks in advance to point it out!
Sylvain.

Sylvain Leroux wrote on Mar 08, 2008 07:06 PM:

• In the case of an index or a key, the item is identified as one in the set of those I have presented to the user.

Not necessarily. You could have view(item) and remove(item) actions that are rendered conditionally and different for each row. I might tamper with the request and pass the index of the one I'm allowed to view into the remove action. So really, no difference on the backend wrt authorization checking.

You could have view(item) and remove(item) actions that are rendered conditionally and different for each row.

Of course you're right! That's exactly the point I missed: I was stuck all the time with the idea of one action per row :(

Thank you to have taken time to answer me!
Sylvain

Gavin King wrote on Mar 08, 2008 04:09 AM:

I tend to cringe at the idea of passing identifiers around explicitly

Honestly, I think that URLs with primary keys in them is just about the only truly semantically correct way to use HTTP.

Oh, certainly - it's the explicitly part that makes me squeamish. I've spent too long dealing with methods like frob(long fooId, long barId, long bazId). You had to know that Foo, Bar, and Baz were the expected types, and trust that people were careful. More often then not, the method would grab the objects by id, do some logic, then forward them on to other methods ... by id. This wasn't a Seam project, but it was using Hibernate, so there really wasn't any excuse.

But of course this code was well within the pretty, magical dreamworld of Java and Hibernate, so perhaps I should lower my expectations for HTTP. I do like the ways that Seam lets me forget the low-level messiness, though (goodbye HTTPSession!). I guess I'm hearing that needing to deal with ids remains a reality in this case (where EL method parameters are used).