[PORTLETSPEC3-30] Add createActionURL & createRenderURL methods that allow for automatically copying parameters Created: 31/Jul/13  Updated: 29/Aug/13  Resolved: 29/Aug/13

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: New Feature Priority: Major
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

There should be two new methods in MimeResponse for creating action URLs and render URLs that conditionally allow for copying the current render parameters when the URL is created. The current createActionURL() and createRenderURL() methods would remain unchanged.

The proposal would add the following fields and methods to MimeResponse:

COPY_NO_PARAMETERS
Specifies that no parameters are to be copied when a URL is created.
COPY_RENDER_PARAMETERS
Specifies that the render parameters set for the current request be copied to the URL when it is created.

PortletURL createActionURL(int copyFlag)

Creates a portlet URL targeting the portlet. If no portlet mode, window state or security modifier is set in the PortletURL the current values are preserved. If a request is triggered by the PortletURL, it results in an action request.

The new action URL will contain private render parameters from the current request as specified by the copyFlag parameter.

The returned URL can be further extended by adding portlet-specific action parameters and portlet modes and window states. Any parameter added to the action URL is automatically an action parameter.

Public render parameters do not need to be explicitly added to the new action URL. Any public render parameters associated with the portlet will automatically be available during action request processing resulting from activation of the URL.

If an action parameter has the same name as a public render parameter, then both the action parameter value(s) and the render parameter value(s) will be available when the action request is triggered. The action parameter value(s) will appear before the render parameter value(s) in the parameter values array.

Parameters:
copyFlag - Specifies how current parameters are to be copied to the URL
Returns:
a portlet action URL
Since:
3.0
See Also:
COPY_NO_PARAMETERS, COPY_RENDER_PARAMETERS

PortletURL createRenderURL(int copyFlag)

Creates a portlet URL targeting the portlet. If no portlet mode, window state or security modifier is set in the PortletURL the current values are preserved. If a request is triggered by the PortletURL, it results in a render request.

The new render URL will contain private render parameters from the current request as specified by the copyFlag parameter.

The returned URL can be further extended by adding portlet-specific render parameters and portlet modes and window states. Any parameter added to the render URL is automatically a render parameter.

Public render parameters do not need to be explicitly added to the new render URL, unless the public render parameter value is to be changed. Any public render parameters associated with the portlet will automatically be available during render request processing resulting from activation of the URL.

If a public render parameter value is changed on a render URL, then the public render parameter will be set to the new value when the URL is activated.

Parameters:
copyFlag - Specifies how current parameters are to be copied to the URL
Returns:
a portlet render URL
Since:
3.0
See Also:
COPY_NO_PARAMETERS, COPY_RENDER_PARAMETERS



 Comments   
Comment by andre.hagemeier [ 31/Jul/13 ]

Very helpful idea. Still, the default must remain COPY_NO_PARAMETERS to verify existing implementations still work

Comment by msnicklous [ 01/Aug/13 ]

The idea is to add two new methods and leave the current ones in place. People using the JSR 286 methods will get the same behavior that they are used to. People using the new JSR 362 methods with the parameter set appropriately will get the new behavior.

So I think it should be pretty safe - developers will have to make a conscious effort in order to get the new functionality.

Comment by Neil Griffin [ 06/Aug/13 ]

+1 from Liferay, but since there are only two possible values for copyFlag, recommend renaming it to copyRenderParameters as a boolean (default false).

Comment by msnicklous [ 29/Aug/13 ]

Resolved by adding the proposed methods. See:

MimeResponse.createRenderURL(UrlFlag)
MimeResponse.createActionURL(UrlFlag)

No spec update was necessary for this issue.





[PORTLETSPEC3-26] Errata: Clarify that certain methods that modify headers can only be called prior to writing to the response Created: 15/May/13  Updated: 20/Aug/13  Resolved: 20/Aug/13

Status: Resolved
Project: portletspec3
Component/s: JSR 286 Portlet Specification Errata
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Major
Reporter: Neil Griffin Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to PORTLETSPEC3-28 Add setStatus method on ResourceResponse Resolved

 Description   

ResourceResponse.setContentLength(int) must be set before the response is written.

The same technically holds true for:

resourceResponse.setProperty(ResourceResponse.HTTP_STATUS_CODE, Integer.toString(statusCode));

Does this merit any changes to the JavaDoc?



 Comments   
Comment by Neil Griffin [ 28/May/13 ]

If PORTLETSPEC3-28 is implemented as an improvement, then I think that this issue (PORTLETSPEC3-26) applies there as well.

Comment by msnicklous [ 19/Aug/13 ]

I agree that we should make a clarification. I'll implement a proposal and put it out for review. I would try to use descriptions similar to those provided for the corresponding HTTPServletResponse methods.

Note that the setProperty description in MimeResponse contains the following description:

void setProperty(String key,
String value)

Sets a String property to be returned to the portal.

Response properties can be viewed as header values set for the portal application. If these header values are intended to be transmitted to the client they should be set before the response is committed.

This method resets all properties previously added with the same key.

In spite of that, I'll explicitly note that the status code must be set before the response is committed in the field description for HTTP_STATUS_CODE.

I agree that this would be applicable if we implement PORTLETSPEC3-28.

See the test portlet PORTLETSPEC3_26 in portletbox for tests that exercise the setting of header information and getting the WindowState and PortletMode (issue PORTLETSPEC3-27) in the resource phase.

It's interesting that WebSphere works slightly differently than Pluto. Both allow HTTP header information to be changed after getWriter() is called and after data is written, but that is probably an accident of implementation rather than design. There are two significant differences between the Pluto and the WebSphere implementations:

1) WebSphere allows an undefined, but "good" 2xx status code (such as "235"), to be set and sent back to the client, while Pluto converts it to "200", even if the status code is set before a call to getWriter().

I think the Pluto implementation is wrong in this area, since the HTTP spec states that the status codes are to be extensible. The client is required by the HTTP spec to understand unknown 2xx status codes as being equivalent to 200.

2) Pluto doesn't seem to allow a Locale with a variant string to be set. The variant portion never gets into the HTTP content-language header.

I think Pluto is wrong here, as well.

Comment by msnicklous [ 20/Aug/13 ]

Updated apidocs with suggested changes to the following ResourceResponse fields / methods:

HTTP_STATUS_CODE
setContentLength
setLocale





[PORTLETSPEC3-46] Portlet XML namespace needs to conform to Java EE 7 pattern Created: 06/Feb/15  Updated: 25/Feb/15  Resolved: 25/Feb/15

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Major
Reporter: Neil Griffin Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

When a developer creates a WEB-INF/portlet.xml descriptor its contents typically looks like this:

<portlet-app
  xmlns="http://java.sun.com/xml/ns/portlet/portlet-app_2_0.xsd"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://java.sun.com/xml/ns/portlet/portlet-app_2_0.xsd http://java.sun.com/xml/ns/portlet/portlet-app_2_0.xsd"
  version="2.0">

But according to the Java EE: XML Schemas for Java EE Deployment Descriptors page, for Portlet 3.0 + Java EE 7 alignment we should use "xmlns.jcp.org" in the namespace. For example:

<portlet-app
  xmlns="http://xmlns.jcp.org/xml/ns/portlet/portlet-app_3_0.xsd"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/portlet/portlet-app_3_0.xsd http://xmlns.jcp.org/xml/ns/portlet/portlet-app_3_0.xsd"
  version="3.0">

However, unless there is some technical problem (such as the ability to deploy Portlet 2.0 and 3.0 portlets on the same page), I would like to suggest that we improve the syntax a little, in order to make it more consistent with other Java EE usage (such as a descriptor like WEB-INF/web.xml):

<portlet-app
  xmlns="http://xmlns.jcp.org/xml/ns/portlet"
  xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
  xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/portlet http://xmlns.jcp.org/xml/ns/portlet/portlet-app_3_0.xsd"
  version="3.0">

For more examples, see Antonio Goncalves' blog entry titled Java EE 7 Deployment Descriptors.



 Comments   
Comment by msnicklous [ 25/Feb/15 ]

I agree with Neil's suggestion. From what I gather, the best practice is not to include version information in the namespace, but rather put the version in a separate attribute. So far, I haven't run into a technical problem that would stand in the way.

In the V3 prototype reference implementation, available for download here, I implemented the portlet namespace so that it can be specified as follows:

<portlet-app xmlns="http://xmlns.jcp.org/xml/ns/portlet" 
   xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance" 
   xsi:schemaLocation="http://xmlns.jcp.org/xml/ns/portlet http://xmlns.jcp.org/xml/ns/portlet/portlet-app_3_0.xsd" 
   version="3.0">




[PORTLETSPEC3-47] Portlet JSP taglib namespace needs to be updated to 3_0 Created: 10/Feb/15  Updated: 25/Feb/15  Resolved: 25/Feb/15

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Major
Reporter: Neil Griffin Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to JSP_SPEC_PUBLIC-188 Change XMLNamespace for all files tha... Open

 Description   

Portlet 2.0 JSPs require the following namespace for usage of JSP tags like <portlet:renderURL />:

xmlns:portlet="http://java.sun.com/portlet_2_0” 

But this needs to be upgraded for Portlet 3.0 JSPs to the following:

xmlns:portlet="http://xmlns.jcp.org/portlet_3_0” 


 Comments   
Comment by keilw [ 10/Feb/15 ]

Would it still be

xmlns:portlet="http://java.sun.com/portlet_3_0”

for historic reasons, even 5 years after the Oracle takeover?

Comment by Neil Griffin [ 11/Feb/15 ]

I had a conversation with Ed Burns of Oracle and he indicated that moving forward, xmlns.jcp.org is the namespace hostname for documents that use namespaces in JCP specifications. I have update the description of this issue accordingly.

Comment by msnicklous [ 17/Feb/15 ]

I like this because it's concise. Would anything speak against using the following namespaces:

For portlets:
xmlns:portlet="http://xmlns.jcp.org/portlet/portlet_3_0”

example:
<portlet-app xmlns="http://xmlns.jcp.org/portlet/portlet_3_0"
xmlns:xsi="http://www.w3.org/2001/XMLSchema-instance"
xsi:schemaLocation="http://xmlns.jcp.org/portlet/portlet_3_0 http://xmlns.jcp.org/portlet/portlet-app_3_0.xsd"
version="3.0">

For JSPs:
xmlns:portlet="http://xmlns.jcp.org/jsp/portlet_3_0”

example:
<%@ taglib uri="http://xmlns.jcp.org/jsp/portlet_3_0" prefix="portlet" %>

Comment by Neil Griffin [ 17/Feb/15 ]

The portlet.xml namespace is discussed in PORTLETSPEC3-46. In all examples of Java EE I see that /xml/ns/ is included in the namespace.

Regarding JSP, I do see examples in JSTL that include /jsp/ in the namespace.

Comment by msnicklous [ 25/Feb/15 ]

After reflecting, I agree with Neil's original suggestion. The V3 prototype of the Pluto reference implementation, available for download here, is implemented so that the JSP namespace can be specified as follows:

<%@ taglib uri="http://xmlns.jcp.org/portlet_3_0"  prefix="portlet" %>




[PORTLETSPEC3-35] Errata: apidoc description of MimeResponse.setContentType() needs to specify exception Created: 21/Aug/13  Updated: 22/Aug/13  Resolved: 22/Aug/13

Status: Resolved
Project: portletspec3
Component/s: JSR 286 Portlet Specification Errata
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

The portlet specification in section 12.5.1 Content Type states:

For the render response the setContentType method must throw an IllegalArgumentException if the content type set does not match (including wildcard matching) any of the content types returned by the getResponseContentType method of the PortletRequest objectcxii.

However, this is not reflected in the apidocs for the method.

void setContentType(String type)

Sets the MIME type for the response. The portlet should set the content type before calling getWriter() or getPortletOutputStream(). If the content type is not the PortletRequest.getResponseContentType() value is set as response content type by the portlet container.

Calling setContentType after getWriter or getOutputStream does not change the content type.

Parameters:
type - the content MIME type
See Also:
PortletRequest.getResponseContentTypes(), getContentType()

We need to add a "Throws" section to the description. I would also use the opportunity to fix the description ... seems to be worded strangely. I believe the following captures the intention:

void setContentType(String type)

Sets the MIME type for the response. The portlet should set the content type before calling getWriter() or getPortletOutputStream(). If the content type is not set using this method, the preferred content type as returned by PortletRequest.getResponseContentType() is used.

Calling setContentType after getWriter or getOutputStream does not change the content type.

Parameters:
type - the content MIME type
See Also:
PortletRequest.getResponseContentTypes(), getContentType()
Throws:
IllegalArgumentException - if the content type is invalid.



 Comments   
Comment by msnicklous [ 22/Aug/13 ]

Updated description to MimeResponse.setContentType to add exception that is thrown when the content type is invalid and to clarify the description of that method.





[PORTLETSPEC3-32] Change the parameter handling on the action URL to match the parameter handling on the render URL Created: 31/Jul/13  Updated: 29/Aug/13  Resolved: 29/Aug/13

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: New Feature Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

This addresses the differences in parameter handling on URLs created through the MimeResponse createActionURL() and createRenderURL() methods. It would be logical for the behavior to be the same for both URL types, since both are represented by a PortletURL object. Currently, the parameter handling differs in only one area - the handling of public render parameters.

A public render parameter can be set on a render URL. When the render URL is activated, the public render parameter is set to the new value, so the code in the render request will behave as follows:

// returns the public render parameter as set on the render URL (or the first value of a multi-valued parameter)
String value = renderRequest.getParameter("myPublicRenderParameter");    

// returns an array containing the value or values of the public render parameter as set on the render URL
String[] values = renderRequest.getParameterValues("myPublicRenderParameter");

// returns the current value of the public render parameter as set on the render URL
Map<String, String[]> parms = renderRequest.getPublicParameterMap();

However, when a parameter with the same name as a public render parameter is set on an action URL, an action parameter by that name is set on the URL. During action request processing, BOTH the action parameter value AND the public render parameter value are available under the same parameter name, so the code in the action request will behave as follows:

// returns the action parameter (or the first value of a multi-valued parameter)
String value = actionRequest.getParameter("myPublicRenderParameter");    

// returns an array containing the value or values of the action parameter followed by the value or values of the public render parameter
String[] values = actionRequest.getParameterValues("myPublicRenderParameter");

// will return the current value of the public render parameter (which may have been set in the past by this or another portlet,
// or which may have been removed)
Map<String, String[]> parms = actionRequest.getPublicParameterMap();

From the point of view of the getParameter() and getParameterValues() methods, the action URL parameter handling behavior is confusing. It seems that there might not be many use cases that would count on this behavior. In fact, a negative use case might be more common - the programmer sets a parameter on an action URL, believing that a public render parameter is being set. But in reality, the actual public render parameter ever gets set.

The proposal calls for making action URL behavior in this area to be identical to the render URL behavior.

However, it would represent a change in specified behavior as compared to JSR 286, so it needs to be considered carefully.



 Comments   
Comment by andre.hagemeier [ 31/Jul/13 ]

A colleague of mine just reported an issue, which I am pretty sure, is related to this. He set a public render parameter and during the procession of a preprocessor, he somehow retrieved two parameters with the same name.
Surely, there was some problem with the implementation as well, but as Scott already mentioned, the negative use cases could be more common than positive side effects.

I wonder, how you would want to adjust the action behavior to behave similar to the render behavior. In your example, would setting the prp on the action request update the prp from the renderrequest?

Comment by msnicklous [ 01/Aug/13 ]

Yes, that's the proposal - change the implementation so that setting a public render parameter on an action URL will result the public render parameter being set when the URL is activated.

This would require changes to the appropriate section of the spec, to the RI, and to the TCK.

Comment by Neil Griffin [ 06/Aug/13 ]

+1 This is the way Liferay already works

Even though some of our other PortletBox tests cover this, here is a new tester portlet that contains minimal code for testing this particular issue:
https://github.com/ngriffin7a/portletbox/tree/master/issues/PORTLETSPEC3-32-portlet

Comment by msnicklous [ 29/Aug/13 ]

Resolved by updating API descriptions with accompanying spec update. See:

MimeResponse.createActionURL(UrlFlag)
MimeResponse.createActionURL()
Spec update, section on Portal URLs





[PORTLETSPEC3-28] Add setStatus method on ResourceResponse Created: 21/May/13  Updated: 26/Aug/13  Resolved: 26/Aug/13

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: julien_viet Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to PORTLETSPEC3-26 Errata: Clarify that certain methods ... Resolved

 Description   

Resource serving can provide an HTTP response status with the ResourceResponse.HTTP_STATUS_CODE property. Adding a setStatus (like in javax.servlet.http.HttpServletResponse) method on the ResourceResponse that would simply delegate to the status code property would be more explicit and developer friendly, instead of:

response.addProperty(ResourceResponse.HTTP_STATUS_CODE, "404");

we would have:

response.setStatus404);



 Comments   
Comment by msnicklous [ 19/Aug/13 ]

I agree that we should add this helper method.

Comment by msnicklous [ 26/Aug/13 ]

Added method ResourceResponse.setStatus(int sc)





[PORTLETSPEC3-27] Errata: PortletMode and WindowState request during resource serving wit FULL cacheability Created: 20/May/13  Updated: 22/Aug/13  Resolved: 22/Aug/13

Status: Resolved
Project: portletspec3
Component/s: JSR 286 Portlet Specification Errata
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: julien_viet Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

During a resource serving with cacheability FULL a portlet cannot access the window state and portlet mode (neither the render parameters).

The specification says is vague and say: "Thus the portlet should not access the portlet mode, window state, or render parameters in the serveResource call."

The current Javadoc does not specify anything particular , for example PortletRequest#getPortletMode() say "Returns the current portlet mode of the portlet" .

I think we lack of clarity as we don't specify what PortletRequest#getPortletMode() and PortletRequest#getWindowState() should return in this case.

I believe it should return null and we should say it clearly in the Javadoc, for example we could redeclare those methods in the ResourceRequest interface and add to the javadoc that in case of FULL cacheability then null is returned.



 Comments   
Comment by msnicklous [ 19/Aug/13 ]

See the test portlet PORTLETSPEC3_26 in portletbox for tests that exercise the setting of header information and getting the WindowState and PortletMode (issue PORTLETSPEC3-27) in the resource phase.

As it turns out, both WebSphere and Pluto return the PortletMode and WindowState information regardless of the cacheability setting on the URL.

With the parameters, it's more complicated, and there is a difference between Pluto & WebSphere.

Both portals provide the resource parameters (as opposed to render parameters) to serveResource() regardless of the cacheability setting. If cacheability is set to PORTLET or PAGE, both portals provide the public and private render parameters to serveResouce() as well.

However, if cacheability is set to FULL, WebSphere provides neither the public nor the private render parameters to serveResource(), while Pluto provides ONLY the public render parameters to serveResource(). I think this is a bug in the Pluto implementation.

But the big question is: How do we want it to be?

Is it really necessary to suppress the render parameters, the PortletMode, and the WindowState when cacheability is set to FULL? Are there use cases that require that behavior? Or does it in some manner make it easier for a portal implementation if this information is not required to be present on a resourceURL with cacheability set to FULL?

The current wording of the spec states:

FULL – The resource URL does not need to contain the current state of the page or the current render parameters, portlet mode, or window state of the portlet.

So it does not require this information to be present on a ResourceURL, but it does not forbid it, either.

Shouldn't we just require it to be present, so that the parameter, window state, and portlet mode handling would not be dependent on the cacheability setting?

I think we should answer these questions before making a clarification for this issue.

Comment by Ed Burns [ 20/Aug/13 ]

In general, I'm not fond of returning null. It's like throwing away valuable semantic information and getting back to a C-language type approach.

Maybe it's better to add a new value to the PortletMode enum?

Comment by msnicklous [ 22/Aug/13 ]

To start off with, I answered my own question - on a resource URL will cache level set to FULL, neither the render parameters nor the portlet mode or window state should be present, as their presence would reduce the ability of the URL to be cached.

Following Ed's suggestion to add a new value to the PortletMode and WindowState enum classes rather than return null, the values PortletMode.UNDEFINED and WindowState.UNDEFINED were added.

The method definitions ResourceRequest.getPortletMode() and ResourceRequest.getWindowState() were added to ResourceRequest, overriding the corresponding definitions in PortletRequest, in order to add the description about the return value when the cache level is set to FULL.

I went ahead and made the change without first posting a detailed proposal, because I felt the basic issue was clear. Naturally, if there would be reasons to modify the change, we can reopen the issue.





[PORTLETSPEC3-41] Some minor changes for the Portlet State proposal Created: 27/May/14  Updated: 27/May/14  Resolved: 27/May/14

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

I received a few comments about the portlet state proposal.

1) Refers to issue PORTLETSPEC3-13. The current proposal adds four methods to handle fragment identifiers: getFragmentIdentifier, setFragmentIdentifier, isFragmentIdentifierPermitted, and setFragmentIdentifierPermitted. The intention of the "permitted" methods is to be able to suppress a fragment identifier set by the portal. A better way to accomplish that goal would be to drop both of the "permitted" methods and redefine the "get" method to return either the fragment identifier previously set
by the portal, or that set by the portlet. The portlet could then use the setter to set the value to "null" if no fragment identifier is desired.

2) Refers to issue PORTLETSPEC3-21. The "write" methods on BaseURL that take an Appendable argument should be renamed to "append" and should return the modified Appendable input object in order to allow for chaining.

3) In the new MutablePortletParameters#setParameter(String, String[]) method, a null value for the array or an empty array should be allowed.

4) Add a new MutablePortletParameters#getParameterNames(String) method that returns a Set, overriding the corresponding PortletParameters method. In the mutable case, removing a parameter from the set will remove that parameter from the PortletParameters object.



 Comments   
Comment by msnicklous [ 27/May/14 ]

I don't believe these changes are critical, so I just went ahead and fixed them. Naturally, I'm open for comments / concerns on them anyway.

See:

RenderURL.html#getFragmentIdentifier()
BaseURL.html#append(java.lang.Appendable)
BaseURL.html#append(java.lang.Appendable,%20boolean)
PortletParameters.html#getParameterNames()
MutablePortletParameters.html#getParameterNames()
MutablePortletParameters.html#setParameter(java.lang.String,%20java.lang.String...)
MutablePortletParameters.html#add(javax.portlet.PortletParameters)





[PORTLETSPEC3-17] Errata: Correction in setNextPossiblePortletModes Created: 03/May/13  Updated: 16/Aug/13  Resolved: 16/Aug/13

Status: Resolved
Project: portletspec3
Component/s: JSR 286 Portlet Specification Errata
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

I believe we need a correction here, see below.

Original:
-------------------------
void setNextPossiblePortletModes(java.util.Collection<PortletMode> portletModes)
-------------------------

Corrected:
-------------------------
void setNextPossiblePortletModes(java.util.Collection<? extends PortletMode> portletModes)
-------------------------



 Comments   
Comment by julien_viet [ 10/May/13 ]

I think this would be only useful if people were using PortletMode subclasses.

This makes the API (a bit) more complex to understand and does not bring much value imho.

Comment by Neil Griffin [ 14/May/13 ]

+1 Seems like a nice feature in that it adds some flexibility:

List<PortletMode> portletModes = new ArrayList<PortletMode>();
response.setNextPossiblePortletModes(portletModes);

List<ExtendedPortletMode> extendedPortletModes = new ArrayList<ExtendedPortletMode>();
response.setNextPossiblePortletModes(extendedPortletModes);
Comment by msnicklous [ 16/Aug/13 ]

Updated method signature & javadoc comment here.





[PORTLETSPEC3-15] Issue in GenericPortlet Annotation Processing Created: 03/May/13  Updated: 19/Aug/13  Resolved: 16/Aug/13

Status: Resolved
Project: portletspec3
Component/s: JSR 286 Portlet Specification Errata
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

This problem is a bit esoteric, and I hope I can adequately present it.
It has to do with Action dispatching using annotations in the case of inheritance.

Working Document 3 (22.04.13) Section "PLT.5.4.5.1 Action Dispatching", beginning at line 8
on page 48, it states:
-------------------------
For a received action the processAction method in the GenericPortlet class tries
to dispatch to public methods annotated with the tag
@ProcessAction(name=<action name>), where the action name must be set on the
ActionURL ...
-------------------------

An attempt was made to use this feature in the following manner:
-------------------------
class PortletA extends GenericPortlet {
@ProcessAction(name="actionA")
void processMyActionAinA(ActionRequest req, ActionResponse resp)
throws PortletException, java.io.IOException;
};

class PortletB extends PortletA

{ @ProcessAction(name="actionA") void processMyActionAinB(ActionRequest req, ActionResponse resp) throws PortletException, java.io.IOException;}

;
-------------------------

Now the question is, for portlet B, which "processMyActionA" is called when actionA occurs?

We would want it to be "processMyActionAinB", I believe.

However, GenericPortlet uses .getClass().getMethods() to obtain the methods for annotation processing.

The javadoc for Class.getMethods() states:
-------------------------
Returns an array containing Method objects reflecting all the public member methods of the
class or interface represented by this Class object, including those declared by the class
or interface and those inherited from superclasses and superinterfaces. Array classes
return all the (public) member methods inherited from the Object class. The elements in the
array returned are not sorted and are not in any particular order.
-------------------------

Since GenericPortlet doesn't do any particular processing to handle methods inherited from
superclasses, it turns out that the actual method for PortletB called when actionA occurs
is not deterministic. It can be either "processMyActionAinB" or "processMyActionAinA", depending on
how .getMethods() happens to order the method array.

We may want to look into this at some point.



 Comments   
Comment by Neil Griffin [ 14/May/13 ]

I think a simple fix for the private GenericPortlet.cacheAnnotatedMethods() method would be to call Method.getDeclaringClass() to see if if that is the same as this.getClass(). If true, then it should "win" over all other candidate methods in the array.

Comment by msnicklous [ 16/Aug/13 ]

updated GenericPortlet.java. Code change only; no apidoc change. See: GenericPortlet.java

Test portlet: PORTLETSPEC3_15 from portletbox.





[PORTLETSPEC3-13] Two extensions for BaseURL Created: 03/May/13  Updated: 29/Aug/13  Resolved: 28/Aug/13

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Attachments: GIF File NamedAnchor-FragmentIdentifier.gif    

 Description   

Add methods to the BaseURL class to provide additional function.

1) add a way to manage fragment identifiers in URLs in order to be able to create URLs such as:

http://www.w3.org/TR/REC-html40/intro/intro.html#h-2.1.2

void setFragmentIdentifier(String)
String getFragmentIdentifier()

2) add a method to create URLs that required the user to log in before they can access the URL

void setProtected(Boolean)



 Comments   
Comment by julien_viet [ 10/May/13 ]

I am in favor of this, however I think we should not use the "protected" name as it is a reserved java keyword and that could raise issues with scripting language such as Groovy that supports natively properties (i.e write url.protected = true).

We have similar concern with GateIn portlet container and we named it "authenticated".

Comment by msnicklous [ 13/May/13 ]

I agree on the naming. So then it would become:

2) add a method to create URLs that required the user to log in before they can access the URL

void setAuthenticated(Boolean)

Comment by Neil Griffin [ 14/May/13 ]

+1 these are very nice additions.

The term fragment identifier is used by the HTML spec but I wonder if getNamedAnchor/setNamedAnchor would be more meaningful to portlet developers?

Comment by andre.hagemeier [ 31/Jul/13 ]

Why would the wording "fragment identifier" be a problem? That's basically what the idea is about, isn't it?

Comment by Neil Griffin [ 13/Aug/13 ]

My impression is that "named anchor" is a term that would be more easily recognized by developers.

Comment by msnicklous [ 23/Aug/13 ]

Google trend information about terms "Fragment Identifier" and "Named Anchor". Original information found here

Comment by msnicklous [ 23/Aug/13 ]

Based on info from Google trends, it looks like "Named Anchor" would win out. Neither term is particularly popular compared to "Ben Affleck" for example, but quite a few more people seem to search for "named anchor" than for "fragment identifier".

(Note that I wasn't able to use the "embed" string provided by Google, because Jira doesn't let me paste script into the post. But you can follow the link above to view the original data.)

On the other hand, "fragment identifier" is the official technical term that can be found in RFC 3986 and other technical literature, while "named anchor" tends more towards web site design (imho). And the fragment identifier can also be used for more than just addressing a named HTML anchor tag - it could provide additional information on a resource URL, for example.

So I am kind of partial to using the precise term as the method name, but explaining it to be a named anchor in the description.

I would also note that adding a fragment identifier to a render URL or a resource URL makes sense, but adding one to an action URL not so much. For now, I would add setFragmentIdentifier to BaseURL, but if we decide to add dedicated interfaces for ActionURL and RenderURL, I would move setFragmentIdentifier to RenderURL and ResourceURL, just for the sake of clarity.

And naturally setAuthenticated makes sense for all URLs, so it clearly belongs in BaseURL.

Comment by msnicklous [ 23/Aug/13 ]

Here are my concrete proposals:

1) for set authenticated

void setAuthenticated(boolean authenticated)

Indicates whether authentication is required for this URL.

When the parameter is set to true, user authentication will be required when accessing the URL. A setting of false indicates that authentication will not be required.

If authentication is not explicitly set on the URL through this method, the default will be true if PortletRequest.getAuthType() indicates that the current request is authenticated and false if the current request is not authenticated.

Parameters:
authenticated - true, if the URL requires authentication. false, if the URL does not require authentication.
Since:
3.0

boolean getAuthenticated()

Returns the authentication setting for the URL.

Returns:
true if the URL requires authentication
Since:
3.0

2) for the fragment identifier

void setFragmentIdentifier(String fragment)

Sets a fragment identifier on the URL.

The fragment identifier consists of additional information appended to the URL after a '#' character. A URL can have only a single fragment identifier. The fragment identifier may not contain the '#' character.

The fragment identifier is often used to address a named anchor such as <a name="#fragmentIdentifier">, but it can also be used for other purposes such as to provide additional information for resource serving.

The fragment identifier will not be namespaced. The portlet is responsible for performing any required namespacing.

Any previously set fragment identifier will be replaced.

Parameters:
fragment - The fragment identifier to be added to the URL
Throws:
IllegalArgumentException - if the fragment identifier is null, the empty string (""), or contains the '#' character.
Since:
3.0

String getFragmentIdentifier()

Gets the fragment identifier set on the URL.

Returns:
The fragment identifier set on the URL, or null if no fragment identifier has been set.
Since:
3.0

Comment by msnicklous [ 28/Aug/13 ]

Resolved by adding several methods to BaseURL:

setAuthenticated
getAuthenticated
setFragmentIdentifier
getFragmentIdentifier
permitFragmentIdentifier
fragmentIdentifierPermitted

Handling the fragment identifier was more complicated than I initially anticipated. I ended up introducing two additional API methods that we hadn't discussed - I apologize for that. I should have thought more carefully before I committed. I should have posted the proposals in the issue in order to allow discussion and perhaps look for alternatives.

Anyway, the background is that a portal can set the fragment identifier on portlet URLs (WebSphere does so in certain situations). That isn't wrong or non-compliant, since JSR286 doesn't mention fragment identifiers at all, and since the fragment identifier is never transported back to the server.

The basic idea behind the latter four APIs in the list above is to allow the portal implementation to append a fragment identifier to a portlet URL if the portlet itself isn't using it. In addition, it should be possible for a portlet to explicitly request that absolutely no fragment identifier gets appended to a URL, even if the portlet itself is not using it.

It seemed to best to implement both of the ideas of the getting / setting of the fragment identifier and the permitting / suppression of the fragment identifier by introducing the two pair of getter / setter methods - setFragmentIdentifier/getFragmentidentifier and permitFragmentIdentifier/fragmentIdentifierPermitted.

I also updated the corresponding section in the spec - please see link.

If there is need for discussion on this point, please reopen and we can discuss.

Comment by Neil Griffin [ 28/Aug/13 ]

For JavaBean getter/setting consistency, should the permitFragmentIdentifier and fragmentIdentifierPermitted methods be named isFragmentIdentifierPermitted() and setFragmentIdentifierPermitted(boolean) respectively?

Comment by msnicklous [ 29/Aug/13 ]

good idea, thanks for the feedback! implemented. See:

setFragmentIdentifierPermitted
isFragmentIdentifierPermitted





[PORTLETSPEC3-8] Errata: Clarify inconsistencies regarding getting and setting parameters Created: 02/May/13  Updated: 13/Aug/13  Resolved: 13/Aug/13

Status: Resolved
Project: portletspec3
Component/s: JSR 286 Portlet Specification Errata
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

Reading the spec document on parameters and comparing it with the javadoc on
"get...Parameter..." and "set...Parameter..." calls on various Response, Request, and
URL classes, it seems to me that there are a number of inconsistencies that should
be addressed, for example:

  1. Is a parameter allowed to have a value of null?
  2. Under which cases (if any) should setting a parameter to a value of null remove the parameter?
  3. In which cases exactly are public parameters get and set?
  4. In which cases are public render parameters set or updated?
  5. In which cases are public render parameters removed?
  6. Should the "getParameterMap" methods always return an immutable map?
  7. probably more that I haven't noticed yet ...

To #6 -

The javadoc for PortletRequest.getParameterMap states:
-------------------------
    Returns:
        an immutable Map containing parameter names as keys and parameter values as map 
        values, or an empty Map if no parameters exist. 
        The keys in the parameter map are of type String. 
        The values in the parameter map are of type String array (String[]).
-------------------------

While the javadoc for BaseURL.getParameterMap states:
-------------------------
Returns:
    Map containing parameter names as keys and parameter values as map 
    values, or an empty Map if no parameters exist. 
    The keys in the parameter map are of type String. 
    The values in the parameter map are of type String array (String[]).
-------------------------

Shouldn't the returned Map values be consistently immutable?

If not, and if we want to be able to update the actual parameters by updating the
Map returned by BaseURL.getParameterMap, it needs to be described clearly.



 Comments   
Comment by Neil Griffin [ 13/May/13 ]

Regarding #1 - #5, I would recommend creating separate issues.

Regarding #6, I checked the Liferay and Pluto source code, and neither return an immutable map. It is possible that portlet developers have been calling BaseURL.getParameterMap().put(name, value) in order to set values, and changing to an immutable map would cause legacy applications to be non-backward-compatible.

Comment by msnicklous [ 15/May/13 ]

We discussed this issue, made some progress, but also identified areas where more work is needed.

A new issue will be opened to look at the following points in more detail:

  • Under which cases (if any) should setting a parameter to a value of null remove the parameter?
  • In which cases are public render parameters removed?

The following points were cleared up:

  • Should the "getParameterMap" methods always return an immutable map?
    • No, that would break existing code
    • No action necessary

The following point needs to be examined in more detail:

  • Is a parameter allowed to have a value of null?
    • No, a parameter may not have a null value
    • This may need clarification in the javadoc / spec - need to have a look
  • In which cases are public render parameters set or updated?
Comment by Neil Griffin [ 15/May/13 ]

For the following questions:

Under which cases (if any) should setting a parameter to a value of null remove the parameter?

In which cases are public render parameters removed?

See: PORTLETSPEC3-25

Comment by Neil Griffin [ 15/May/13 ]

Test Portlet for this issue: https://github.com/ngriffin7a/portletbox/tree/master/issues/PORTLETSPEC3-8-portlet

Comment by mfreedma [ 21/May/13 ]

On your statement:

"The following points were cleared up:

Should the "getParameterMap" methods always return an immutable map?
No, that would break existing code
No action necessary"

I agree that map returned is mutable. I disagree if we/the developer expects that manipulating this Map directly impacts the BaseURL. Rather we/developer should manipulate this Map and the call baseURL.setParameters(Map). Right?

Comment by Neil Griffin [ 28/May/13 ]

@Mike: It looks like Scott has fortified the PORTLETSPEC3-8 test portlet with a test for this.

There are two new buttons:

  • Button with label "actionURL.setParameters() before" (test 12)
  • Button with label "actionURL.setParameters() after" (test 13)

On Pluto, clicking on these buttons does not affect the toString() value of the ActionURL that is rendered in the subsequent portal page. I think that this confirms your statement:

I disagree if we/the developer expects that manipulating this Map directly impacts the BaseURL.

On Liferay, clicking on these buttons does indeed affect the toString() value. Because of this, my preference would be that interpretation be left to the implementation, rather than tighten-up the Spec requirement.

I'd be interested to know what happens in other implementations.

Comment by msnicklous [ 26/Jul/13 ]

I believe the only remaining open point here is whether an an update to the map returned by one of the get parameter map functions should directly cause an update to the actual underlying parameters. In other words, should the following code actually change the parameters:

String[] values = { "Fred", "Wilma", "Pebbles" };
actionResponse.getRenderParameterMap().put("privateRenderParameter3", values);

My take would be that by itself no, it should not. The correct code should be:

String[] values = { "Fred", "Wilma", "Pebbles" };
Map<String, String[]> parmMap = actionResponse.getRenderParameterMap();
parmMap.put("privateRenderParameter3", values);
actionResponse.setRenderParameters(parmMap);

I would suggest the following change for the getRenderParameter map method and corresponding updates for the getParameterMap method on BaseURL:

Map<String,String[]> getRenderParameterMap()

Returns a Map of the parameters currently set on this portlet URL via the setParameter or setParameters methods.

Contents of this map may be modified, but modifying the map does not directly affect the render parameters. In order to set the
parameters to the values in the modified map, the setRenderParameters(Map) with the modified map as a parameter must be used.

The keys in the returned map are of type String, and the values are of type String array (String[]).

If no parameters exist this method returns an empty Map.

Returns:
    Map containing parameter names as keys and parameter values as map values, or an empty Map if no parameters exist. The keys in the parameter map are of type String. The values in the parameter map are of type String array (String[]).
Comment by Neil Griffin [ 30/Jul/13 ]

Regarding the following sentence:

Contents of this map may be modified, but modifying the map does not directly affect the render parameters. In order to set the parameters to the values in the modified map, the setRenderParameters(Map) with the modified map as a parameter must be used.

Using the "Set Parms direct Map #1" (case 4) and "Set Parms direct Map#2" (case 5) buttons, I found that modifying the following code will indeed cause the underlying map to be changed on Liferay Portal:

actionResponse.getRenderParameterMap().put("privateRenderParameter3", values);

It is possible that portlet developers have been calling getRenderParameterMap().put(name, value) in order to set values, and changing to an immutable map would cause legacy applications to be non-backward-compatible.

Because of this, my vote would be that interpretation be left to the implementation, rather than tighten-up the Spec requirement. Therefore I would recommend changing part of the sentence to the following:

"modifying the map is implementation dependent and therefore may or may not directly affect the render parameters. In order to guarantee that the modified map affects the render parameters, the setRenderParameters(Map) with the modified map as a parameter must be used."

Comment by msnicklous [ 31/Jul/13 ]

Note that Pluto does not allow the parameters to be changed through the map directly. The map obtained from getRenderParameterMap() can be updated,
but the updates are ignored without the setParameters() call. So Pluto behaves according to the proposed change.

WebSphere allows a parameter update directly through the map ... sometimes. Namely, if the map is initially empty, it cannot be updated. If a private parameter has been set previously
so that the map is non-empty, a direct parameter update through the map will be allowed. This seems to me to be an "accident of implementation".

It would be good if we could understand how other implementations work and see if we can come to a conclusion on this.

Comment by msnicklous [ 31/Jul/13 ]

Neil,

I see that I was working on an stale copy of the Jira issue when I made my update, and I hadn't seen your update. Thanks for posting how Liferay works in this area.
It might indeed be best to leave that point as an implementation detail.

Comment by andre.hagemeier [ 31/Jul/13 ]

Neil,
"Regarding #6, I checked the Liferay and Pluto source code, and neither return an immutable map. It is possible that portlet developers have been calling BaseURL.getParameterMap().put(name, value) in order to set values, and changing to an immutable map would cause legacy applications to be non-backward-compatible."

I have seen this being done, and I have done it personally. E.g. I implemented a preprocessor to pass url parameters as PRP to Portlets under certain circumstances. So restricting this point would break existing implementations.

In addition, Scott,:
"String[] values =

{ "Fred", "Wilma", "Pebbles" };
actionResponse.getRenderParameterMap().put("privateRenderParameter3", values);
My take would be that by itself no, it should not. The correct code should be:

String[] values = { "Fred", "Wilma", "Pebbles" }

;
Map<String, String[]> parmMap = actionResponse.getRenderParameterMap();
parmMap.put("privateRenderParameter3", values);
actionResponse.setRenderParameters(parmMap);"

With that, you would enforce immutable maps. Although I would favor the second approach as well, making the first one impossible would break existing code. Since the first approach cannot be deprecated (as you would have to deprecate a method from java.util.Map), sth like this could be done:

public class DeprecatedMutableMap<String, String[]> extends HashMap<String, String[]> {
@Override
public boolean put(String name, String[] values)

{ logger.warn("calling the put method on this map instance is deprecated. Better use ..."); return super.put(name, values); }

}

Just a rough idea. But that would be highly vendor specific, so it actually would not have to be related to the JSR. The JSR would only have to specify, that setting the render parameter directly should not be possible (and if it is already, it should at least be deprecated).

Comment by msnicklous [ 01/Aug/13 ]

I gave this more thought, and I think we have to work our way through the ideas and specify this, one way or the other. I don't think we can leave it as an implementation detail.

With the portlet title issue, where we said if the title is not specified in the portlet descriptor, it's up to the implementation to determine the title,
it wasn't critical, because the implementation could decide to leave it empty, just call the portlet "Bob", or do something maybe more sensible. It wouldn't
break the portlet.

However, leaving the map issue up to the implementation will result in a real incompatibility, where a portlet written to assume that changing the map changes the parameters would
not work on a portlet container that does not support that (for example, it wouldn't work on the current reference implementation).

I understand the point about the map on many implementations currently being mutable and directly effecting a change in the parameters. However, if we leave the map like this, we run into other issues.

Earlier we agreed on certain rules for the portlet parameters: A parameter may not have a null value, but it can have the empty string as a value. Setting a public
render parameter to null does not remove the public render parameter; the removePublicRenderParameter() method must be used to remove it. Not including a public render parameter
in the map passed to setRenderParameters() does not mean that the public render parameter is removed.

If we stick with these rules (and I believe we should), then the map returned by getParameters() or getRenderParameters() would have to enforce these rules as well.
It looks to me like this would lead to a custom map implementation in any case.

I do not believe we could just say, "well, we have these rules about parameters, but if you get a map through getRenderParameters() you can do whatever the map happens to allow."

And would we really want to say that setting a parameter within the map to null would remove it? That would be .... ugly in imho. But that might be just my personal feeling of aesthetics.

In light of this, I thought it would be cleaner to force the map to be passed to setParameters() and setRenderParameters() in order for the new values to take effect. the setParameters() call could then enforce the parameter rules.

Andre - I don't think the proposal above would mean that the map is immutable. The map could still be changed. It would just no
longer directly effect a change in the actual parameters - this is the way it currently works on Pluto.

Comment by andre.hagemeier [ 01/Aug/13 ]

Scott,
i think whether this map is immutable or not depends on the point of view. I assume your point of view is:
The map is not immutable since you can actually change its values, and once you set this map with setRenderParameters(), it will take effect.

My point of view:
The map is immutable, because i cannot change the values of the parameters of this specific map (or at least is has no effect). I need create a new map or use the same map to re-initiate the values of the backed map.

With your example:

params.java
String[] values = { "Fred", "Wilma", "Pebbles" };
actionResponse.getRenderParameterMap().put("privateRenderParameter3", values);
// this way, you could directly change the values of the prp map. You will not receive a clone, but the real backed map, so any change to this map is directly reflected in the request

String[] values = { "Fred", "Wilma", "Pebbles" };
Map<String, String[]> parmMap = actionResponse.getRenderParameterMap();
parmMap.put("privateRenderParameter3", values);
actionResponse.setRenderParameters(parmMap);"
// most likely you will receive a clone of the internal map here. otherwise you would not have to call setRenderParameters, but could just use the first approach.
// the setRenderParameters method would then iterate over the paramMap and put its names and values to the internal map of the request

//this is basically the same, as if I created a totally new map and copied all existing values. //There is just no reference to the map instance
String[] values = { "Fred", "Wilma", "Pebbles" };
Map<String, String[]> parmMap = new HashMap<String, String[]>();
copy(actionResponse.getRenderParameterMap(), parmMap);
parmMap.put("privateRenderParameter3", values);
actionResponse.setRenderParameters(parmMap);"

To me, the second approach is immutable, because changing the returned map has no effect. Only, if I use the returned map and set it using setRenderParameters, the change will take effect. But that would happen due to the fact, that all names and values are copied to the internal map of the request.

The only thing, I want to make sure, is that existing implementations still work. Since the methods mentioned don't exist in portlet2.0, i assume, that the old ones will be deprecated, but still be available. And with that getParameterMap needs to return a mutable map, so that
request.getParameterMap().put(x,y) will still work, even if it Eclipse will show a warning, because getParameterMap may be deprecated.

Apart from that, as I said, I agree, that the second way would be the more accurate.

Comment by msnicklous [ 01/Aug/13 ]

Andre, sorry, don't really follow you. All methods listed currently exist. In fact, the code in the examples is copy/pasted from test portlets that were run with
varying degrees of success against several portal implementations. Nothing would be deprecated.

I don't think we should make assumptions about the internal implementation of a portlet container. We just need to describe external behavior.

Comment by andre.hagemeier [ 01/Aug/13 ]

Sorry Scott, I guess I mixed up some things here.
The only question to me is, what to do with the spec and the implementations. I think the implementations cannot be forced to return immutable maps.
So two possibilities would be left:

  • allow mutable maps in the spec
  • deprecate mutable maps in 362 (but still allow them for backward compatibility) and make immutable ones the default. Therefore, a subclassing approach could be used like above stated.

Anyway, this issue needs to be documented properly, to make clear why there is a problem at all.

Comment by Neil Griffin [ 06/Aug/13 ]

@Scott: I received feedback from my colleagues, and the unanimous response was to have StateAwareResponse.getRenderParameterMap() return an immutable map. This would make the API more clear that the only way to set parameters (with a Map) would be to call StateAwareResponse.setRenderParameters(java.util.Map).

Comment by msnicklous [ 13/Aug/13 ]

implemented updates for descriptions of BaseURL.getParameterMap() and StateAwareResponse.getRenderParameterMap().





[PORTLETSPEC3-6] Errata: Clarification for section "PLT.25.8.2 Locales supported by the Portlet" Created: 02/May/13  Updated: 07/Aug/13  Resolved: 09/Jul/13

Status: Resolved
Project: portletspec3
Component/s: JSR 286 Portlet Specification Errata
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified

Issue Links:
Related
is related to JAVASERVERFACES_SPEC_PUBLIC-1210 Account for BCP47 Closed

 Description   

Referring to the working document "JavaTM Portlet Specification, working document 3 (22.04.13)",
there is a mistake under section PLT.25.8.2 at line 15. RFC 1766 specifies the locale format
as a language tag that is composed of subtags separated by hyphens, rather than underscore
characters. The text should be corrected as follows:

Original:
-------------------------
The supported locales declared in the deployment descriptor should follow the
lang_COUNTRY_variant format as defined by RFC 1766
(http://www.faqs.org/rfcs/rfc1766.html).
-------------------------

Corrected:
-------------------------
The supported locales declared in the deployment descriptor should follow the
lang-COUNTRY-variant format as defined by RFC 1766
(http://www.faqs.org/rfcs/rfc1766.html).
-------------------------



 Comments   
Comment by julien_viet [ 10/May/13 ]

I believe that such change would mean portlet container behavior changes but also breakage of existing applications using the current format.

We should keep the current behavior. We should remove the mention of RFC 1766 that cannot be respected and instead accept a definition that is correct.

I reviewed the web-facesconfig_2_0.xsd configuration schema that accept both syntaxes and specifies:

<xsd:complexType name="faces-config-supported-localeType">
<xsd:annotation>
<xsd:documentation>

The "supported-locale" element allows authors to declare
which locales are supported in this application instance.

It must be specified as :language:[_:country:[_:variant:]]
without the colons, for example "ja_JP_SJIS". The
separators between the segments may be '-' or '_'.

</xsd:documentation>
</xsd:annotation>
<xsd:simpleContent>

Comment by msnicklous [ 13/May/13 ]

Ok, I agree that we can't break existing portlets. In light of this, I would formulate the text as follows.

Corrected:
-------------------------
The supported locales declared in the deployment descriptor should be specified as :language:[:country:[_:variant:]] without the colons, for example "ja_JP_SJIS". The separators between the segments may be '-' or ''.
-------------------------

Question – the current text uses “should” … shouldn't it be “must”? (... declared in the deployment descriptor must be specified ...)

Comment by Neil Griffin [ 13/May/13 ]

@julien: Excellent detective work with the faces-config schema!

@msnicklous: I agree that the word "must" is necessary when describing firm requirements like this.

Comment by msnicklous [ 09/Jul/13 ]

Updated spec document accordingly.

Comment by Neil Griffin [ 23/Jul/13 ]

I think during the call we decided to re-open this issue for discussion to see what the Servlet API supports.

This page lists all of the Java EE 7 XSD files:
http://www.oracle.com/webfolder/technetwork/jsc/xml/ns/javaee/index.html

And here is the URL for the web-common_3_1.xsd
http://www.oracle.com/webfolder/technetwork/jsc/xml/ns/javaee/web-common_3_1.xsd

Inside the file it contains documentation for "localeType" and "locale-encoding-mappingType" elements. If I understand the docs correctly, then I think that both dash and underscore are permitted.

Comment by msnicklous [ 26/Jul/13 ]

Fix discussed on 20130723. Revisit with additional information on locale handling in Java 7.

Comment by Ed Burns [ 26/Jul/13 ]

I've been working on a JSF issue that manifests with IE10 in Chinese locales. The is related to the new "script" subtag in the language tag spec as defined by Internet Best Current Practice 47. <http://tools.ietf.org/html/bcp47>. We are going to have to revise the JSF spec to take this into account. Basically, we need to specify how language tags that include the "script" subtag are converted to Locales that may not.

This is an issue for JSF 2.2 because we do not require JavaSE 7, and JavaSE 6 does not support these script subtags at all.

For JSF 2.3, we'll have to revise that language to allow for the script subtag.

Comment by keilw [ 29/Jul/13 ]

While individual JSRs like JSF 2.2 or JBatch (352) indeed require a minimum version of Java (EE) 6, the basis for JSR 362 is said to be Java EE 7, hence the improvements to Locale in Java 7 should be available. Or is there anything on the detail page that's wrong or shall be applied differently?

Comment by msnicklous [ 30/Jul/13 ]

In light of the the new Locale support in Java 7, it seems that we should move in that direction also. A description similar to the following might be appropriate:

(updated on 7 Aug 13 to remove the statement regarding compatibility in the text proposal, and to add some additional new information after the proposal)

The supported locales declared in the deployment descriptor must be specified as a valid language tag according to IETF BCP 47, "Tags for Identifying Languages". A valid language tag is composed of subtags separated by hyphens ("-", ABNF [RFC4234] %x2D) as delimiters. The language tag must begin with the primary language subtag and may be followed by subsequent optional subtags. If specified, the subtags must appear in the order shown below.

  • language - ISO 639 alpha-2 or alpha-3 language code, or registered language subtags up to 8 alpha letters (for future enhancements).
  • script - ISO 15924 alpha-4 script code. You can find a full list of valid script codes in the IANA Language Subtag Registry (search for "Type: script").
  • country (region) - ISO 3166 alpha-2 country code or UN M.49 numeric-3 area code. You can find a full list of valid country and region codes in the IANA Language Subtag Registry (search for "Type: region").
  • variant - Any arbitrary value used to indicate a variation of a Locale.
  • extensions - A map from single character keys to string values, indicating extensions apart from language identification. The extensions in Locale implement the semantics and syntax of BCP 47 extension subtags and private use subtags.

A valid language tag example would be "ja-JP-SJIS". This tag contains "ja" as the mandatory primary language tag, followed by the "JP" country and the "SJIS" variant subtags. The optional script and extensions subtags are not specified.

See IETF BCP 47, "Tags for Identifying Languages" for a complete description. See the descriptions of the Java SE 7 Locale, Locale.Builder, and ResourceBundle classes for additional information on how Java processes IETF BCP 47 language tag information.

Note that this will require a change to the portlet descriptor v3.0 schema as well.

Backward compatibility will be retained since v2.0 portlet descriptors will be validated with the v2.0 schema, so the old language tag formato for 2.0 portlets will continue to be accepted.





[PORTLETSPEC3-23] Errata: Issue in GenericPortlet Exception Handling for Annotations Created: 13/May/13  Updated: 19/Aug/13  Resolved: 19/Aug/13

Status: Resolved
Project: portletspec3
Component/s: JSR 286 Portlet Specification Errata
Affects Version/s: None
Fix Version/s: None

Type: Bug Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

The doView Method is defined to throw two exceptions - a PortletException
and a java.io.IOException.

-------------------------
protected void doView(RenderRequest request, RenderResponse response)
throws PortletException, java.io.IOException

{ ... }

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

However, the annotation handling code in the GenericPortlet doDispatch() method
wraps all exceptions from an annotated method as a PortletException so that
effectively a potential java.io.IOException is never thrown.

-------------------------
try {
// check if mode is cached
if ( renderModeHandlingMethodsMap.containsKey(mode) )

{ renderModeHandlingMethodsMap.get(mode).invoke(this, request, response); return; }
} catch (Exception e) { throw new PortletException(e); }
-------------------------

the processAction() and processEvent() code wraps all exceptions from annotated
methods as PortletExceptions in the same manner. The corrected exception handling
code should be something like:

Corrected:
-------------------------
try {
// check if mode is cached
if ( renderModeHandlingMethodsMap.containsKey(mode) ) { renderModeHandlingMethodsMap.get(mode).invoke(this, request, response); return; }


} catch (InvocationTargetException ex) {
// root cause
final Throwable th = ex.getCause();
// fallthru
if (th instanceof PortletException)

{ throw (PortletException) th; }

else
if (th instanceof IOException)

{ throw (IOException) th; }

else
if (th instanceof RuntimeException)

{ throw (RuntimeException) th; }

else

{ // rethrow throw new PortletException(ex); }

}
-------------------------



 Comments   
Comment by msnicklous [ 19/Aug/13 ]

updated GenericPortlet.java. Code change only; no apidoc change. See: GenericPortlet.java

Test portlet: PORTLETSPEC3_23 from portletbox.





[PORTLETSPEC3-22] Use varargs when it is possible Created: 10/May/13  Updated: 25/Aug/13  Resolved: 25/Aug/13

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: julien_viet Assignee: msnicklous
Resolution: Fixed Votes: 1
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

Some methods of the API uses array based last argument like in BaseURL:

public void setParameter (String name, String[] values);

I suggest we could replace this by a varargs, providing the same signature and improving the usability of the API to:

public void setParameter (String name, String... values);



 Comments   
Comment by Neil Griffin [ 14/May/13 ]

+1 great flexibility

Comment by andre.hagemeier [ 31/Jul/13 ]

+1

Comment by msnicklous [ 24/Aug/13 ]

I can find the following methods where this would be useful:

BaseURL.setParameter (String name, String[] values)
PortletPreferences.setValues(String key, String[] values)
StateAwareResponse.setRenderParameter(String key, String[] values)

did I miss any?

How do you actually document a vararg argument in the javadoc comments? For example, the first line of BaseURL.setParameter reads

Sets the given String array parameter to this URL.

Should we just leave it like that?

"Sets the String vararg ..." sounds weird to me.

Comment by msnicklous [ 25/Aug/13 ]

Changed String[] parameter to String... parameter on the methods BaseURL.setParameter StateAwareResponse.setRenderParameter PortletPreferences.setValues.

I decided to call them multi-valued parameters, as in:

"Sets a multi-valued String parameter for the render request."

Hope this terminology is found to be appropriate.





[PORTLETSPEC3-21] Add a method for writing a BaseURL to a java.lang.Appendable Created: 10/May/13  Updated: 23/Aug/13  Resolved: 23/Aug/13

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: julien_viet Assignee: Unassigned
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

The current BaseURL interface provides capability to write to a java.io.Writer .

It would be a good thing to add the same method for a java.lang.Appendable as the appendable is an abstraction implemented by java.io.Writer but also by many other objects such as java.lang.StringBuilder.



 Comments   
Comment by Neil Griffin [ 14/May/13 ]

I suppose that the standard approach would be to @Deprecate the java.io.Writer methods, but since java.io.Writer implements java.lang.Appendable, the method signatures could simply be changed to:

BaseURL.write(java.lang.Appendable out);
BaseURL.write(java.lang.Appendable out, boolean escapeXML);

All existing code would still compile...

Comment by julien_viet [ 14/May/13 ]

We need to keep the existing method otherwise it will create binary incompatible classes for classes compiled with JSR 286 jar and executed with this API change by throwing a NoSuchMethodError on write(Ljava/io/Writer

Comment by Neil Griffin [ 14/May/13 ]

@julien_viet: You're right – I was thinking of source-compatible and not binary-compatible. Need to keep things binary compatible.

Comment by msnicklous [ 23/Aug/13 ]

Added the proposed BaseURL.write(Appendable) methods.

I did not deprecate the write(Writer) methods due to feedback that we should make it obvious to people that a method accepting a Writer exists and should be used when possible to avoid overhead associated with String / CharSequence processing.





[PORTLETSPEC3-20]  Errata: Clarification in Javadoc for an illegal scope use in PortletSession methods Created: 07/May/13  Updated: 17/Aug/13  Resolved: 17/Aug/13

Status: Resolved
Project: portletspec3
Component/s: JSR 286 Portlet Specification Errata
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: julien_viet Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

Some PortletSession attribute get methods do not specify their behavior when they are invoked with an unknown container scope:

  • PortletSession#getAttribute(String,int)
  • PortletSession#getAttributeNames(int)
  • PortletSession#getAttributeMap(int)
  • PortletSession#removeAttribute(String,int)

There are other methods of the PortletSession interface that must throw an IllegalArgumentException when the parameter null is provided such as PortletSession#setAttribute(String,Object,int): "java.lang.IllegalArgumentException if name is <code>null</code> or scope is unknown to the container."

We should change clarify how the getAttribute

The possible correction are:
1. the methods should fail fail with throwing an `IllegalArgumentException`
2. the methods should ignore it (return null or empty map)

The removeAttribute method should likely be corrected to 1. as the setAttribute(String,Object,int) says clearly "If the value is <code>null</code>, this has the same effect as calling
removeAttribute()".

The three other methods could use 1 or 2.



 Comments   
Comment by msnicklous [ 13/May/13 ]

I agree that this should be corrected. I think an IllegalArgumentException should be thrown in whenever the scope is unknown to the container.

Note that in the JSR 286 Javadoc for:

PortletSession#getAttribute(String,int)
PortletSession#getAttributeNames(int)

it states:

----------
Throws:
java.lang.IllegalStateException - if this method is called on an invalidated session*, or the scope is unknown to the container.*
----------

so we would be changing the type of exception thrown. But an IllegalArgumentException makes more sense to me that an IllegalStateException in this case.

For:

PortletSession#getAttributeMap(int)
PortletSession#removeAttribute(String,int)

we could just add the IllegalArgumentException when the scope is unknown to the container.

Comment by msnicklous [ 17/Aug/13 ]

updated javadoc comments in PortletSession to clarify behavior when the scope is unknown.





[PORTLETSPEC3-18] Provide a method in PortletConfig to query the supported portlet modes Created: 03/May/13  Updated: 02/Feb/15  Resolved: 26/Aug/13

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: Improvement Priority: Minor
Reporter: msnicklous Assignee: msnicklous
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: Not Specified
Time Spent: Not Specified
Original Estimate: Not Specified


 Description   

It would be good to add a method to the PortletConfig interface that would
provide the portlet modes supported my the portlet.



 Comments   
Comment by Neil Griffin [ 14/May/13 ]

+1 Always seems helpful to expose elements configured in the portlet.xml descriptor to the developer.

Comment by andre.hagemeier [ 31/Jul/13 ]

+1 why not extend this to the whole portlet descriptor? I could imagine that supported languages, portlet name, title etc might be interesting as well

Comment by msnicklous [ 26/Aug/13 ]

I think that most of the other information is available through one method or the other already. Supported languages can be obtained through Portletconfig.getSupportedLocales() and the portlet name through PortletConfig.getPortletName(). The supported processing and publishing events are available along with the supported public render parameters. The portlet title, short title, keywords, display name, etc. are available through the resource bundle (even if no real resource bundle is provided ).

What I find to be missing completely is a way of obtaining the defined and supported portlet modes and window states. You can get that information sort of indirectly through use of PortletRequest.isPortletModeAllowed(PortletMode) and PortletRequest.isWindowStateAllowed(WindowState), but you have to sort of try it out ... it isn't very elegant.

So I think we need a way to get an enumeration of the available portlet modes and window states.

It might be that we need other information in a nice API as well, but so far I haven't found anything else to be missing.

Comment by msnicklous [ 26/Aug/13 ]

Added methods:

PortletConfig.getPortletModes()
PortletConfig.getWindowStates()

Comment by msnicklous [ 02/Feb/15 ]

Implemented methods in Pluto branch V3Prototype. While doing so, I discovered an oversight - both the portlet modes and window states are dependent on the MIME type. I updated the method signatures to reflect the fact. I also added a method to obtain the public render parameter QNames, since that information wasn't available elsewhere. The method signatures are:

Enumeration<PortletMode> PortletConfig.getPortletModes(String) Enumeration<WindowState> PortletConfig.getWindowStates(String)
Map<String, QName> PortletConfig.getPublicRenderParameterDefinitions()

See:
V3.0 javadoc





[PORTLETSPEC3-34] More tutorial content for API changebars Created: 16/Aug/13  Updated: 25/Aug/13  Resolved: 25/Aug/13

Status: Resolved
Project: portletspec3
Component/s: Ideas for JSR 362 Extensions
Affects Version/s: None
Fix Version/s: None

Type: Task Priority: Trivial
Reporter: Ed Burns Assignee: Ed Burns
Resolution: Fixed Votes: 0
Labels: None
Remaining Estimate: 30 minutes
Time Spent: Not Specified
Original Estimate: 30 minutes


 Description   

Scott,

Thanks for applying the changebars to a few more classes, specifically:

MimeResponse
RenderResponse
StateAwareResponse

This issue will cause a pull request that will apply some changes to those classes that you appear to have missed, assuming the goal is to achieve parity with the multi-level system we use in JSF.



 Comments   
Comment by Ed Burns [ 16/Aug/13 ]

Sent pull request.

https://github.com/edburns/portletspec3/commit/49faade0e70a3f03145dcac6019da9d43b52d547

Comment by msnicklous [ 25/Aug/13 ]

Proposed changes have been assimilated.





Generated at Tue Jun 02 18:14:31 UTC 2015 using JIRA 6.2.3#6260-sha1:63ef1d6dac3f4f4d7db4c1effd405ba38ccdc558.