[jira] Created: (JCR-1555) Add support for boolean values to QValue

Add support for boolean values to QValue
----------------------------------------

                 Key: JCR-1555
                 URL: https://issues.apache.org/jira/browse/JCR-1555
             Project: Jackrabbit
          Issue Type: Improvement
          Components: jackrabbit-spi, jackrabbit-spi-commons
            Reporter: Michael Dürig
            Priority: Minor


I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael Dürig updated JCR-1555:
-------------------------------

    Attachment: JCR-1555.patch

Suggested path. Includes test cases for the new methods

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12592409#action_12592409 ]

angela commented on JCR-1555:
-----------------------------

regarding the patch:
isn't Boolean#parseBoolean(String)  a new method added with 1.5? if this is true, i think we should avoid it since as far as i know we never dropped the support for 1.4.

general:
i know that we discussed that at least once in the past and for reasons i don't remember thought
the method would not be needed... but apparently the question arises on a regular basis (included me asking julian). if adding the method solves that, i am in favor of the extension.

julian/marcel, do you remember what exactly we decided in the past and why?

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12594842#action_12594842 ]

Marcel Reutegger commented on JCR-1555:
---------------------------------------

I don't remember exactly, but I guess we wanted to keep the interface as simple as possible.

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12594844#action_12594844 ]

Michael Dürig commented on JCR-1555:
------------------------------------

Sure, but I think this is a source of confusion. And it is not consistent with the rest of the api. After all one could also create longs, dates and all other types from a string.

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12594860#action_12594860 ]

Julian Reschke commented on JCR-1555:
-------------------------------------

The difference is that for Boolean, no real parsing is required.

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12594864#action_12594864 ]

Michael Dürig commented on JCR-1555:
------------------------------------

Yes but from the api consumer's perspective this is an implementation detail.

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12595178#action_12595178 ]

angela commented on JCR-1555:
-----------------------------

so... can we reach consensus here? i don't have a strong feeling.

if the missing method is source for confusion and we feel that i would not do any
harm we should probably add it, shouldn't we?

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12595212#action_12595212 ]

Julian Reschke commented on JCR-1555:
-------------------------------------

I personally do not really care. If Mchael really wants it, let's add it :-)

That being said, we are just in the process of removing fluff elsewhere, where the rational is to keep the API minimal. It seems adding the methods discussed here is contrary to that goal :-)

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12595214#action_12595214 ]

Jukka Zitting commented on JCR-1555:
------------------------------------

How about if we only added the method to QValueFactoryImpl in spi-commons? An SPI implementation (that AFAIUI is the main use case behind this request) can use the implementation classes directly.

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12596319#action_12596319 ]

Michael Dürig commented on JCR-1555:
------------------------------------

I dont think adding the method to QValueFactoryImpl will fix the issue. People will still look at the interface and wonder how to create a boolean.

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12596328#action_12596328 ]

Thomas Mueller commented on JCR-1555:
-------------------------------------

> How about if we only added the method to QValueFactoryImpl in spi-commons?

Why?

+1 for JCR-1555(2).patch as is.

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12596335#action_12596335 ]

angela commented on JCR-1555:
-----------------------------

julian:

> I personally do not really care. If Mchael really wants it, let's add it :-)

ok.

> That being said, we are just in the process of removing fluff elsewhere, where the rational
> is to keep the API minimal. It seems adding the methods discussed here is contrary to
> that goal :-)

... maybe. but with respect to keep the API minimal we only had creation from String
initially... so... we already compromised and extended the QValue/Factory here. adding
the extra methods for the boolean property type somehow is a logic consequence, isn't it?
i remember that i was wondering once, why it is missing.

jukka:

> How about if we only added the method to QValueFactoryImpl in spi-commons?
> An SPI implementation can use the implementation classes directly.

no... i am not in favour of this.
adding the method to the interfaces is rather a question of consistency and maybe it is
cosmetics only. it would be convenient to have it, but not having it isn't a problem.

so. let's come to a conclusion.

> michi, thomas are in favour of the patch.
> julian wouldn't object if michi wants the method but added a concern.

is there anybody that does object?
otherwise i would add the patch (and in addition replace the QValueFactory.create(string, int) to the new method where the type represents PropertyType.Boolean... there is one or two places...

angela

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Marcel Reutegger updated JCR-1555:
----------------------------------

    Attachment: JCR-1555-mreutegg.patch

I'm +1 on adding the methods for boolean properties, but would rather not create a new QValueImpl instance on each call. See attached patch.

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555-mreutegg.patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

    [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel&focusedCommentId=12596339#action_12596339 ]

Michael Dürig commented on JCR-1555:
------------------------------------

+1 for Marcel's patch

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555-mreutegg.patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

angela resolved JCR-1555.
-------------------------

    Resolution: Fixed
      Assignee: angela

committed marcels patch at revision 656637.

in addition adjusted
- BooleanConstraintTest
- ValueFormat
- ValueFactoryQImpl
to use the QValueFactory.create(boolean) method.

and modified QValueFactoryImpl.create(String, int) to return the boolean-type
constants.

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Assignee: angela
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555-mreutegg.patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.

     [ https://issues.apache.org/jira/browse/JCR-1555?page=com.atlassian.jira.plugin.system.issuetabpanels:all-tabpanel ]

Michael Dürig closed JCR-1555.
------------------------------

> Add support for boolean values to QValue
> ----------------------------------------
>
>                 Key: JCR-1555
>                 URL: https://issues.apache.org/jira/browse/JCR-1555
>             Project: Jackrabbit
>          Issue Type: Improvement
>          Components: jackrabbit-spi, jackrabbit-spi-commons
>            Reporter: Michael Dürig
>            Assignee: angela
>            Priority: Minor
>         Attachments: JCR-1555(2).patch, JCR-1555-mreutegg.patch, JCR-1555.patch
>
>
> I suggest to add support for reading and writing boolean values to QValue and QValueFactory. I find it strange that there is such support for the other data types but booleans must be constructed via strings.

--
This message is automatically generated by JIRA.
-
You can reply to this email to add a comment to the issue online.