Support View annotation on Controller methods returning non void types (except Viewable)

[chkal]

Issue by mvcbot
Friday Aug 07, 2015 at 10:04 GMT
Originally opened as mvc-spec/mvc-spec#63

---

Original issue MVC_SPEC-51 created by beryozkin_sergey:

@Controller
@View("book.jsp")
public Book getBook() {}
{code}

I.e, if it is non-void, non-Viewable response with View then the response is treated as the content to be processed by the view engine.
 
The simplest and safe way to bind Book to HTTP request attribute is to assume a convention that a class name (Book.class.getName()) is used as a key. 
Additionally (or alternatively) an extra optional attribute can be added to @View:
{code:java}
@Controller
@View(value="book.jsp", name="book")
public Book getBook() {}
{code}

Furthermore the above style can also support:

@Controller
@View("book.jsp")
@Produces({"application/xml", "application/json", "text/html"})
public Book getBook() {}

given that it is very typical in JAX-RS to have a single method with multiple Produces values.

I think this style is very important to support because it is more natural to write than having to use a @nAmed sync such as Greetings in the spec example or dealing with Models directly, in both cases the application directly having to be aware of the MVC semantics.

IMHO it is not difficult to support it at a spec level. It is also another good example why a strict CDI binding requirement may need to be relaxed and limited to a subset of styles.

[chkal]

Comment by mvcbot
Friday Aug 07, 2015 at 10:06 GMT

---

Comment by beryozkin_sergey:

Please assign to Santiago

[chkal]

Comment by mvcbot
Friday Aug 07, 2015 at 12:41 GMT

---

Comment by Santiago Pericas-Geertsen:

It is certainly possible to consider using @view this way. However, MVC is now "reserving" the return type for view (or Viewable). In fact, the current behavior in the latest spec is to call toString() on a returned object to get a view path. Having said that, perhaps there should be a new Viewable constructor that takes a name/model pair to avoid the need for Models in that case.

As for using a controller to return non-HTML representations, it's better to separate that into two: a resource and a controller at this point.

[chkal]

Comment by mvcbot
Friday Aug 07, 2015 at 12:53 GMT

---

Comment by beryozkin_sergey:

The whole idea of this proposal is for users be able to avoid dealing directly with all the (JSP/etc) paths and MVC API (Viewable) directly. I'm not sure what is wrong with such an approach, at the very least it can be considered a reasonable design practice... I'm not against giving users a direct/fine-grained control but a less 'intrusive' option is not bad at all ?

You mentioned the current API calls toString() but specifying a rule that if @view is available on a method returning a type (non-void, non-Viewable) then it is a not a resource path but the content can work ? It would simply relax the current statement that View is ignored for non-void responses without introducing any contradiction into the text. It would integrate well with all the spec ?

Please ignore my reference to CDI here.

[chkal]

Comment by mvcbot
Tuesday Aug 11, 2015 at 06:54 GMT

---

Comment by Christian Kaltepoth:

Let me explain the reason why I suggested to call toString() on all controller results which doesn't match any of the specified types (String, Viewable, etc). The reason for that was that this allows to implement typesafe ways of specifying which view to render. You could for example return enums from the controller and implement toString() accordingly to return the view corresponding to the specific enum value.

However, I also see that providing some other default behavior could make sense. What we are currently talking about is something like this, right?

{quote}
If there is a @View on the controller method/class and the controller method returns a result of some other type (!= Viewable, String, Response), the result is stored in the model (using some default key like model) and the view specified via @View is rendered.
{quote}

I see that this may also be a reasonable default behavior. BUT it basically prevents to implement typesafe navigation as I described before.

[chkal]

Comment by mvcbot
Tuesday Aug 11, 2015 at 14:01 GMT

---

Comment by Santiago Pericas-Geertsen:

I generally like the suggestion, but I think we need to be careful not to add more complexity in the form of rules that developers would need to understand that don't really add new capabilities to the API. Sometimes adding more ways of doing something is not necessarily a good thing. Having said that, if we don't need name in @view, and use of a default name as suggested, that may be a little better.

[chkal]

Comment by mvcbot
Tuesday Aug 11, 2015 at 15:37 GMT

---

Comment by Christian Kaltepoth:

I agree that we should avoid too much complexity.

If we add support for something like this, we shouldn't support customizing the name. I see no good reason for this feature. We should choose one of these approaches to generate the name:

• Either use a fixed name like model
• Or create a name from the class name using the same rules that are used by @Named by default.

[chkal]

Comment by mvcbot
Tuesday Aug 11, 2015 at 16:13 GMT

---

Comment by beryozkin_sergey:

Hi Christian, Santiago

Thanks for the comments, I agree having an extra View name attribute adds to the complexity (it can always be reviewed later on if needed should users start asking).
Defaulting to a class name appears to be very safe. I hope users will like it in general, in addition to being able to do a fine-grained control of MVC flows

Cheers, Sergey

[chkal]

Comment by chkal
Sunday Feb 11, 2018 at 12:55 GMT

---

I don't think that we can provide this feature in version 1.0. So moving this to "Future". But we should make sure that something like this can be implemented later without breaking backwards compatibility.