Values in oak-core

[Michael Dürig <md...@apache.org>]

Hi,

I worked on OAK-33: Values in oak-core today and came up with a default 
implementation of Scalar [1]. Some things, especially in the area of 
binary handling, are still unfinished and marked with todo.

Some thoughts:

- I think we should add a method to PropertyState for accessing the 
value as Scalar. However this would introduce a circular dependency 
between oak-core and oak-mk. How should we proceed here?

- Does it really make sense to allow for multiple Scalar 
implementations? Shouldn't we just have one final class for that?

Michael

[1] 
http://svn.apache.org/viewvc/jackrabbit/oak/trunk/oak-core/src/main/java/org/apache/jackrabbit/oak/ScalarImpl.java?view=markup

Re: Values in oak-core

[Michael Dürig <md...@apache.org>]

On 26.3.12 11:18, Jukka Zitting wrote:
> Hi,
>
> On Sun, Mar 25, 2012 at 12:16 AM, Michael Dürig<md...@apache.org>  wrote:
>> That's actually the way I look at it. However moving Scalar to oak-mk would
>> require us to have a dependency from oak-jcr to oak-mk which I think we
>> shouldn't.
>
> See why I would have like to keep oak-core and -mk in the same
> component until we figure out where the ideal component boundary lies?
> :-)

Hehe ;-)

> For now I'd say we move Scalar to oak-mk to be a part of the internal
> tree model there, and use it also in oak-core and ultimately oak-jcr
> through the .oak.kernel package. Once the design is finished, we can
> make a clearer separation between the components.

Ok, I did the respective changes in revision 1305295, 1305296 and 
1305303. Note that I left some todo's in SimplePropertyState. I can take 
care of these but it'll take me until tomorrow.

Michael

>
>> But let's not get caught up with this. I can live with Scalar being an
>> interface.
>
> OK. We can revisit this later on as the implementation stabilizes.
>
> BR,
>
> Jukka Zitting

Re: Values in oak-core

[Jukka Zitting <ju...@gmail.com>]

Hi,

On Sun, Mar 25, 2012 at 12:16 AM, Michael Dürig <md...@apache.org> wrote:
> That's actually the way I look at it. However moving Scalar to oak-mk would
> require us to have a dependency from oak-jcr to oak-mk which I think we
> shouldn't.

See why I would have like to keep oak-core and -mk in the same
component until we figure out where the ideal component boundary lies?
:-)

For now I'd say we move Scalar to oak-mk to be a part of the internal
tree model there, and use it also in oak-core and ultimately oak-jcr
through the .oak.kernel package. Once the design is finished, we can
make a clearer separation between the components.

> But let's not get caught up with this. I can live with Scalar being an
> interface.

OK. We can revisit this later on as the implementation stabilizes.

BR,

Jukka Zitting

Re: Values in oak-core

[Michael Dürig <md...@apache.org>]

On 24.3.12 11:17, Jukka Zitting wrote:
> Hi,
>
> On Fri, Mar 23, 2012 at 7:48 PM, Michael Dürig<md...@apache.org>  wrote:
>> - I think we should add a method to PropertyState for accessing the value as
>> Scalar. However this would introduce a circular dependency between oak-core
>> and oak-mk. How should we proceed here?
>
> For now I was using the tree model interfaces from oak-mk to avoid
> having to maintain identical copies in oak-core.
>
> If we have a good case (e.g. handling of values) of why the internal
> tree model should look different in -core than in -mk, then we should
> come up with separate tree interfaces in -core.
>
> On the other hand, if we restrict Scalars to just a strongly typed
> version of JSON values then it might as well live inside .mk.model
> where it could come in handy if we want to do stuff like treat
> "\u0058" and "X" as equal when merging concurrent changes, etc.

That's actually the way I look at it. However moving Scalar to oak-mk 
would require us to have a dependency from oak-jcr to oak-mk which I 
think we shouldn't.

>
>> - Does it really make sense to allow for multiple Scalar implementations?
>> Shouldn't we just have one final class for that?
>
> The ScalarImpl you wrote already consists of eight classes. The one
> Thomas has in .oak.query is a single class but has more memory
> overhead. There might also be a case for lazy scalars that only parse
> the underlying JSON representation when actually needed. That's a
> whole lot of implementation detail that I really would like to keep
> clearly separate from the API.

To me Scalar is just a data object so having possibly different 
implementations just adds unnecessary complexity. (Like always, I'm 
afraid of broken equals ;-)) While I see the need for lazy loading, we 
could just as well provide factory methods which take a thunk to the 
value instead of the value itself. Like for binaries in ScalarImpl:

public static Scalar binaryScalar(final Callable<InputStream> 
valueProvider){}

But let's not get caught up with this. I can live with Scalar being an 
interface.

>
> PS. .oak.ScalarImpl.java is missing the Apache license header.

Sorry about that, fixed it. IntelliJ's Copyright profiles don't seem to 
work any more and I have kind of given up on this.

Michael

>
> BR,
>
> Jukka Zitting

Re: Values in oak-core

[Jukka Zitting <ju...@gmail.com>]

Hi,

On Fri, Mar 23, 2012 at 7:48 PM, Michael Dürig <md...@apache.org> wrote:
> - I think we should add a method to PropertyState for accessing the value as
> Scalar. However this would introduce a circular dependency between oak-core
> and oak-mk. How should we proceed here?

For now I was using the tree model interfaces from oak-mk to avoid
having to maintain identical copies in oak-core.

If we have a good case (e.g. handling of values) of why the internal
tree model should look different in -core than in -mk, then we should
come up with separate tree interfaces in -core.

On the other hand, if we restrict Scalars to just a strongly typed
version of JSON values then it might as well live inside .mk.model
where it could come in handy if we want to do stuff like treat
"\u0058" and "X" as equal when merging concurrent changes, etc.

> - Does it really make sense to allow for multiple Scalar implementations?
> Shouldn't we just have one final class for that?

The ScalarImpl you wrote already consists of eight classes. The one
Thomas has in .oak.query is a single class but has more memory
overhead. There might also be a case for lazy scalars that only parse
the underlying JSON representation when actually needed. That's a
whole lot of implementation detail that I really would like to keep
clearly separate from the API.

PS. .oak.ScalarImpl.java is missing the Apache license header.

BR,

Jukka Zitting

Re: Values in oak-core

[Thomas Mueller <mu...@adobe.com>]

Hi,

>- Does it really make sense to allow for multiple Scalar
>implementations? Shouldn't we just have one final class for that?

I would prefer one final class.

Regards,
Thomas