[
https://issues.apache.org/jira/browse/PLUTO-609?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=13105053#comment-13105053
]
Ate Douma commented on PLUTO-609:
---------------------------------
For the record, if no (persistent) default value has been defined for a
preference, the reset method is specified to actually remove the preference, so
that is supported by the spec.
One thing the specification is not explicit (enough) about is: is it allowed to
call setValues(key, null) or setValues(key, new String[0])?
The javadoc for setValues only says: "null values in the values parameter are
allowed". It doesn't say a null or zero length array is allowed/supported.
Please note the difference between setValues(key, new String[0]) and
setValues(key, new String[]{null}).
This is particularly important with how setValue(key, null) should be
interpreted: as setValues(key, null), as setValues(key, new String[]{null}) or
as setValues(key, new String[0])?
My interpretation and proposal for all this now is:
- setValues(key, new String[]{null}) should lead to returning [null] (array
size 1) from getValues(key, new String[]{"DEFAULT"}) but "DEFAULT" from
getValue(key, "DEFAULT")
(this is your adjusted proposal above)
- setValues(key, null) should be interpreted/translated into setValues(key, new
String[0])
- setValues(key, new String[0]) should lead to returning a [] (array size 0)
from getValues(key, new String[]{"DEFAULT"}) *and* a null value from
getValue(key, "DEFAULT")
(this is currently implemented through the changes for this issue)
Now the question remains: how should setValue(key, null) be treated?
To keep semantical balance with getValue(key,<default>) which accesses the
first element of the value array, it seems logical to translate it into
setValues(key, new String[]{null}).
This is also the currently implementation!
However, with the above semantics for handling setValues, this leads to the
illogical result that setValue(key, null) != getValue(key, "DEFAULT").
So it might actually be better to reimplement setValue(key, null) as
setValues(key, null) -> setValues(key, new String[0]).
If we then implement the above, I think there are actually only a few changes
to make based on the *original* implementation (so after first reverting the
current changes):
a) in getValue: if getValues(...) returns a array length 0 ([]), return null,
else return either the first array element or the default if either null
b) in setValue: delegate to setValues(key, value == null ? null : new
String[]{value})
c) in setValues: if value == null then value = new String[0]
I've tested these changes against the TCK and Pluto now still passes all tests,
as is Jetspeed using this version of Pluto.
However, the Pluto testsuite now fails on the
PreferenceCommonTest.checkSetPreferenceNull test, which still assumed the
original behavior (setting a null value resulting in the default being returned
from getValue). So, if to implement these logic changes, that test now has to
be adjusted likewise (returning null instead of the provided default).
Please note that it depends on the *PortletPreferenceImpl* backing model *if*
String[0] actually gets "stored"...
For Pluto which by default only implements a transient/in-memory model, setting
a String[0] value will be retained.
Jetspeed however, which uses database persistency, this *currently* will result
in the existing value(s) being destroyed (and thus fallback to returning the
"default" values).
We probably should review if we can/should adapt Jetspeed for this new
interpretation of the spec, but technically the above changes won't change its
current behavior AFAIK.
I'll preliminary implement these proposed changes so they can be properly
reviewed and tested.
If this still isn't according to the desired interpretation it is pretty easy
to revert again or adjust otherwise.
> PortletPreferencesImpl doesn't handle null preferences correctly
> ----------------------------------------------------------------
>
> Key: PLUTO-609
> URL: https://issues.apache.org/jira/browse/PLUTO-609
> Project: Pluto
> Issue Type: Bug
> Affects Versions: 2.0.2
> Reporter: Eric Dalquist
> Assignee: Ate Douma
> Fix For: 2.0.3, 2.1.0
>
>
> PLT.17.1 states "Preference attributes are String array objects. Preferences
> attributes can be set to null." In Pluto if you call
> PortletPreference.setValue("name", null), PortletPreference.setValues("name",
> String[] {null}), or PortletPreference.setValues("name", null) the correct
> data is passed to the underlying preference storage SPI.
> The problem is when calling getValue("name", "DEFAULT") or getValues("name",
> new String[] { "DEFAULT" }) for any of the three previous cases "DEFAULT" is
> returned. From my reading of the spec this is not correct as in each case the
> preference has been set but with a single null value or a null values array.
--
This message is automatically generated by JIRA.
For more information on JIRA, see: http://www.atlassian.com/software/jira
