jackrabbit-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Jukka Zitting ...@yukatan.fi>
Subject Re: JCR-RMI 0.16.4.1 diffs
Date Mon, 09 May 2005 11:01:28 GMT
Hi,

Felix Meschberger wrote:
> I just finished the implementation of the Value stuff in the jcr-rmi 
> project.

Very nice, thanks! I'll commit the changes as-is.

> My patch is centered around your proposal of Value and ValueFactory 
> implementation with some modifications:

To list readers: we had a short private discussion about how to 
implement the JCR Value interface. I sent a proposal based on the State 
design pattern that quite nicely captures the stream/non-stream state 
changes of specified in the Value interface. See the JCR-RMI patch for 
details.

> (1) SerialValue is the only class implementing the Value interface. The 
> state classes implement the (similar) interface StatefullValue - if you 
> come up with a better name, please go ahead :-)

I'm not sure if a separate interface is needed. The Value state classes 
implement the *exact* semantics of the Value interface in a specific 
state. Thus I think that a separate interface only complicates matters 
needlessly. If the state instances need to be specifically marked, then 
an empty ValueState marker interface that extends the standard Value 
interface might be a better solution.

> (2) All instance creation is done in SerialValueFactory, where protected 
> creator methods have been defined for all constructors. This allows for 
> extension of given StatefullValue classes by also extending the 
> SerialValueFactory class but prevents breaking the State pattern contract.
 >
> (3) The concrete StatefullValue implementation classes are public for 
> them to be extensible. The constructors are protected for extensibility 
> and to prevent inadvertent instantation.

OK. My initial thought was that there would be little need for extending 
the value implementation, but you could well be right in allowing 
extensions.

This however has the drawback that you still need to use the special 
serialization mechanism instead of standard object serialization.

> (4) Added BaseNonStreamValue abstract class providing default 
> implementations for the getter methods throwing the correct exceptions. 
> Concrete non-stream state classes extend this class and overwrite 
> getters allowed according to the spec.

I'm not perfectly OK with this. Implementation inheritance always adds 
some semantic complexity (the full behaviour of a class is not defined 
in a single file) and in this case I'm not sure if the benefit of 
sharing code outweights the drawbacks. Especially since I think that the 
throwing of ValueFormatExceptions is an integral part of the

> (a) The InputStream on which a BinaryValue is based is not necessairily 
> closed. Should we provide a finalize method to close the stream or wrap 
> the InputStream in an anonymous class providing such a finalizer method ?

I'm not sure that the Value instance is the best place to close the 
stream. A client could easily call Value.getStream(), discard the Value 
instance, and continue using the acquired stream.

Instead I think it should be the responsibility of the Value creator 
(who calls ValueFactory.createValue(InputStream)) to provide appropriate 
management of the stream.

> (b) The parser/formatter of the DateValue class does not work correctly 
> according to the spec. See below.

OK. I think we could include the ISO8601 utility class somewhere.

> (c) The NameValue, PathValue, and ReferenceValue classes do not 
> currently check the syntax of the string value. See below.

Proper support for these formats (NameValue, PathValue) requires access 
to the current namespace mappings (the getString() output depends on the 
current namespace prefixes). I have some supporting code for this in the 
JCR-EXT package.

> Currently the implementation is in the org.apache.jackrabbit.value 
> package in the jcr-rmi contribution.

I'll make a copy of the Value implementation also in the JCR-EXT contrib 
project. My long term plan is actually to migrate JCR-RMI into JCR-EXT, 
as there's already quite a lot of code in JCR-EXT that would greatly 
simplify the RMI classes.

> I suggest this to be moved to some common project within jackrabbit such 
> that all jackrabbit users - and possibly external users - may reuse the 
> same code to prevent code duplication. This would probably also mean to 
> move some helper classes from jackrabbit core to that common project 
> such as the ISO8601 helper class providing Calendar formatting and 
> parsing according to the spec.

My proposal is to use the JCR-EXT contrib package as the common ground. 
The design goal of JCR-EXT is to provide general purpose utility classes 
for JCR implementations. The package is still in alpha state but already 
contains a reasonable amount of code that I've either written from the 
scratch or taken from Jackrabbit core and generalized.

BR,

Jukka Zitting

Mime
View raw message