Re: Looking for contributors as part of Apache incubation

On 6/3/15 2:50 AM, Daniel Dekany wrote:
> Wednesday, June 3, 2015, 5:13:00 AM, Woonsan Ko wrote:
> 
> [snip]
>>>>  - #setContentType() invocation can check if it was already set before
>>>> template execution.
>>>>    Currently, it invoked on #setContentType() every time regardless of
>>>> the situation that (H)MVC framework sets it before executing the
>>>> template. I'd like to have an option to not set if already existing at
>>>> least. (ref: https://issues.onehippo.com/browse/HSTTWO-3012)
>>>
>>> Wouldn't that (the Content-type being set or not) be confusing and
>>> fragile to build on? Is there a guarantee at all that the Servlet
>>> container itself doesn't fill it with some default?
>>>
>>> Since FreemarkerServlet meant to be the only one who fiddles with the
>>> response content (it's the View after all), it's naturally also
>>> responsible for ensuring that the content type is consistently set.
>>
>> I think it's a good practice to ensure setting Content-Type in the view
>> servlet. My point was if someone sets Content-Type in the controller
>> before dispatching to the view then the view servlet might skip setting
>> it again to a default value (unless developer sets in template again).
> 
> So how reliable that is, detecting if it wasn't set? Does the Servlet
> spec. say something about this?

According to Servlet Spec 2.4 (SRV.14.2.22.1),
ServletResponse#getContentType() should "Returns the content type used
for the MIME body sent in this response. The content type proper must
have been specified using setContentType(String) before the response is
committed. If no content type has been specified, this method returns
null. If a content type has been specified and a character encoding has
been explicitly or implicitly specified as described in
getCharacterEncoding() , the charset parameter is included in the string
returned. If no character encoding has been specified, the charset
parameter is omitted.
Returns: a String specifying the content type, for example, text/html;
charset=UTF-8, or null
Since: 2.4"

So, if #getContentType() returns null, then I think we can detect that
the Content-Type was never set before.

> 
>> Comparing with JSP, the current behavior seems okay, but I was thinking
>> in line of flexibility in our HMVC framework.
>>
>>> So, maybe it would be better if we don't take away that duty from
>>> FreemarkerServlet, instead it should be smarted about it:
>>>
>>> - You could just tell FreemarkerServlet explicitly to set a given
>>>   content type with a ServletRequest attribute. It's analogous to
>>>   specifying data-model variables to tell Freemarker what to show,
>>>   which you also do in the ServlerRequest via attributes.
>>>
>>> - Support a new init-param value: ContentType=dontSet
>>>
>>> - Adding a protected method where you can override how content type
>>>   detection happens, and if it happens (returning null means "don't
>>>   set").
>>
>> I would like an init-param better than custom request attribute or
>> protected method. Thanks for the suggestion!
> 
> So then, let's just extend the existing "ContentType" init-param to
> support "dontSet"/"dont_set" value, or whatever it should be (and
> indeed what should it be?). Then we will *never* set the content type,
> not even if it wasn't set. Will the servlet container add a default
> content type then? Had to check the specs and try it at least on
> Tomcat and Jetty...

According to SRV.5.2, "Servlet programmers are responsible for ensuring
that the Content-Type header is appropriately set in the response object
for the content the servlet is generating. The HTTP 1.1 specification
does not require that this header be set in an HTTP response. Servlet
containers must not set a default content type when the servlet
programmer does not set the type."

So, actually, frameworks or containers don't have to set Content-Type if
programmers don't. However, I have seen JSP compiler (of Tomcat)
generates source with #setContentType() before (maybe it's a legacy from
old spec?), so I still think setting Content-Type header in the
framework level (freemarker servlet in this case) to a reasonable
default value seems fine.

Regarding the special value to not set the default Content-Type, how
about an empty string for that? If I set ContentType="" in init param,
then it can skip setting the default Content-Type.

> 
>>>>  - Configuration settings could be set through init parameters.
>>>>    Some servlet-related parameters are set through init parameters, but
>>>> it could be even better if there's any specific init-param(s) to set
>>>> Configuration settings without code changes.
>>>
>>> I'm not sure what you mean here. Pretty much all the Freemarker
>>> settings can be set through the init-params, because the init-params
>>> that are not recognized are feed to Configuration.setSetting(String,
>>> String).
>>
>> Ah you're right. I was totally confused. I've just realized that even
>> the following issue was about our misuse of the configuration parameter
>> at the moment. Thanks for the pointer!
>>
>>>
>>>>    (ref: https://issues.onehippo.com/browse/HSTTWO-2525,
>>>
>>> If you want to specify a setting that doesn't exist in
>>> FreemarkerServlet, then naturally you have to extends
>>> FreemarkerServlet so that it can deal with that setting.
>>>
>>>>          https://issues.onehippo.com/browse/HSTTWO-2460)
>>>
>>> Says "Errors in freemarker templates are logged on SEVERE level" (not
>>> by FM). Is that what you meant to link?
>>>
>>> BTW, note that in 2.3.22 you can tell Freemarker not to log the
>>> template errors which will cause Template.process to throw an
>>> exception on you anyway. So then it's up to you if and how do you log
>>> them.
>>
>> Ah this is also my mistake. We did override it there to ignore exception
>> by default unless users sets TemplateExceptionHandler init parameter
>> explicitly. Sorry for my misunderstanding.
> 
> Things like this can cause confusion as they interfere with similar FM
> features. It's better to figure out how to address the problem in
> Freemarker itself and then contribute... (-;
> 
>>>>  - Locale issue.
>>>>    Currently templates are executed with a specific global locale all
>>>> the time (FreemarkerServlet.deduceLocale() uses config.getLocale()). It
>>>> can be overriden though. This can result in wrong date/currency
>>>> formatting issue for instance (when using <@fmt.format* /> JSTL tags).
>>>>    It could have read HttpServletRequest#getLocale() by default.
>>>> Normally (H)MVC frameworks sets the request locale before rendering a view.
>>>
>>> Is that default also desirable in other frameworks that use
>>> FreemarkerServlet? It's a breaking change, so I don't think we can
>>> change the default any time soon, but there could be an init-param for
>>> this. (Though Hippo is perhaps not affected, as you already provide a
>>> customized FreemarkerServler anyway.)
>>
>> I think so. For example, Spring Framework has a built-in component
>> interface, LocaleResolver [1]. Depending on implementation component, it
>> resolves the current request locale based on header, cookie, session,
>> param, etc. So, even if they use the same component/template, they can
>> change the locale and use different i18n resources dynamically.
> 
> Doesn't that just sets the locale in the RequestContext? That's a
> Spring-specific object. Because then overriding deduceLocale(...) is
> still the only option. If they were using FreemarkerSerlvet at all,
> that is...

Spring Framework primarily store request specific locale into its own
RequestContext. However, it also falls back to
ServletRequest#getLocale() if not set in its framework level
(org.springframework.web.servlet.support.RequestContextUtils.getLocale(HttpServletRequest)).
ServletRequest#getLocale() returns the preferred locale that the client
will accept content in, based on the Accept-Language header, by default.
However, many other frameworks can wrap servlet request to override this
method. In our case, our HMVC framework (HST-2) supports URL prefix
based locale setting feature and wraps request to support seamless JSTL
tags integration for instance.
In portal/portlet world, it's very common/crucial to see request
wrappers for portlets where users are able to set locale preferences.
Wicket also has wrappers to support this kind of things.
In hierarchical aggregation frameworks (portal, HMVC, Wicket, etc), they
have to wrap request/response objects anyway, and if they want to invoke
controller and dispatch a view in a part of the page rendering, then
they need to control #getLocale() somehow from the page request
processing level, not from each part rendering level.

So, if #deduceLocale() can somehow use either ServletRequest#getLocale()
or global setting more intelligently (not sure how we can make it easier
to config though), then it would be more ideal to me.

> 
>> [1]
>> http://docs.spring.io/spring/docs/current/spring-framework-reference/htmlsingle/#mvc-localeresolver
>>
>>>
>>>>  -  More lightweight version.
>>>>     Sometimes our users want to use a more lightweight version instead
>>>> of FreemarkerServlet. Especially people who are very familiar with
>>>> Spring MVC Framework want that without having to set up a Freemarker
>>>> servlet configuration in web.xml. One hurdle for them was they cannot
>>>> easily take advantage of JSTL tag libraries if they don't use the
>>>> servlet. I wish we could have more lightweight component support with
>>>> which we can take advantage of JSTL tag support independently, without
>>>> having to set up a servlet.
>>>>     By the way, this might be a byproduct of spring mvc framework
>>>> integration probably.
>>>
>>> The JavaDoc of freemarker.ext.jsp.TaglibFactory says: "It can be added
>>> to custom servlets as well to enable JSP taglib integration in them as
>>> well." So at least at some point in the past that was an intent. It
>>> should be piloted out if it indeed works, and maybe an example should
>>> be added where it's applied.
>>
>> IIRC, it seemed difficult to do. But I can try it later again. I'll let
>> you know later.
> 
> Well, it's entirely possible that it needs some substantial changes...
> I don't know. The whole servlet/JSP integration stuff was always out
> of my focus somehow.

I'll give it a try soon.

Thanks for your remarks!

Cheers,

Woonsan

> 
>>>> I think this is it for now. :-)
>>>
>>> So it seems, a good place for contributions for Hippo is all these JSP
>>> integration stuff. This is where I'm weaker anyway... ;-)
>>
>> Yeah, and maybe (H)MVC integration things (Spring Framework, Struts2 for
>> instance).
>>
>>>
>>> And if someone at Hippo needs to do a hack to make FreemarkerServlet
>>> behave better, always consider if it could be part of
>>> FreemarkerServlet instead.
>>
>> Yeah, that's the right way to go. Agreed.
>>
>> Cheers,
>>
>> Woonsan
> 


-- 
[email protected]     www.onehippo.com
Boston - 745 Atlantic Ave, 8th Floor, Boston MA 02111
Amsterdam - Oosteinde 11, 1017 WT Amsterdam
US +1 877 414 4776 (toll free)
Europe +31(0)20 522 4466

------------------------------------------------------------------------------