JCR-RMI 0.16.4.1 diffs

classic Classic list List threaded Threaded
11 messages Options
Reply | Threaded
Open this post in threaded view
|

JCR-RMI 0.16.4.1 diffs

Ludovic Maître
Hello,

I have do some modifications to JCR RMI so it is compatible with the
last SVN release of Jackrabbit, i hope.

The modification has been for the following Jackrabbit version:
[powerg4:jackrabbit] svn info
Path: .
URL: http://svn.apache.org/repos/asf/incubator/jackrabbit/trunk
Repository UUID: 13f79535-47bb-0310-9956-ffa450edef68
Revision: 169133
Node Kind: directory
Schedule: normal
Last Changed Author: jukka
Last Changed Rev: 168578
Last Changed Date: 2005-05-06 11:07:00 +0200 (Fri, 06 May 2005)
Properties Last Updated: 2005-05-08 11:05:53 +0200 (Sun, 08 May 2005)

The diff file is the result of the following command:
[powerg4:jcr-rmi] pwd .../jackrabbit/contrib/jcr-rmi
[powerg4:jcr-rmi] svn diff > jcr-rmi-0.16.4.1.txt

The changes are:
- add a dependency to jackrabbit to use his valueholders class
(LongValue...)
- add the getValueFactory() to the remote and local sessions. I'm really
not sure if we should permit to get the valuefactory, but i have done
this so this compile.
- add an ignore rule for this new method into the test because i don't
know what is appropriate to use it.

So if somebody will check if this modifications are ok, they can be
committed.

I must precise that i have never use the jcr-rmi ext before so i perhaps
that i have modified this in an inapproriate manner.

Best regards,

--
Cordialement,
Ludo - http://www.ubik-products.com
---
"L'amour pour principe et l'ordre pour base; le progres pour but" (A.Comte)


Index: project.xml
===================================================================
--- project.xml (revision 169133)
+++ project.xml (working copy)
@@ -21,7 +21,7 @@
   <pomVersion>3</pomVersion>
   <id>jcr-rmi</id>
   <name>JCR-RMI</name>
-  <currentVersion>0.16.4</currentVersion>
+  <currentVersion>0.16.4.1</currentVersion>
   <inceptionYear>2004</inceptionYear>
   <package>org.apache.jackrabbit.rmi.*</package>
   <description>
@@ -60,10 +60,15 @@
     <dependency>
       <groupId>jsr170</groupId>
       <artifactId>jcr</artifactId>
-      <version>0.16.4</version>
+      <version>0.16.4.1</version>
       <url>http://www.day.com/maven/jsr170/jars/jcr-0.16.4.jar</url>
     </dependency>
     <dependency>
+      <groupId>jackrabbit</groupId>
+      <artifactId>jackrabbit</artifactId>
+      <version>0.16.4.1-dev</version>
+    </dependency>
+    <dependency>
       <groupId>xerces</groupId>
       <artifactId>xercesImpl</artifactId>
       <version>2.6.2</version>
Index: src/test/org/apache/jackrabbit/test/rmi/RemoteAdapterTest.java
===================================================================
--- src/test/org/apache/jackrabbit/test/rmi/RemoteAdapterTest.java (revision 169133)
+++ src/test/org/apache/jackrabbit/test/rmi/RemoteAdapterTest.java (working copy)
@@ -217,6 +217,7 @@
         ignoreMethod("getImportContentHandler"); // implemented locally
         ignoreMethod("exportSystemView");        // multiple methods
         ignoreMethod("exportDocumentView");      // multiple method
+        ignoreMethod("getValueFactory");         // LMA: I don't know which is the appropriate test
 
         Session session = (Session) mock;
         RemoteSession remote = remoteFactory.getRemoteSession(session);
Index: src/java/org/apache/jackrabbit/rmi/server/ServerSession.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/server/ServerSession.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/server/ServerSession.java (working copy)
@@ -25,6 +25,8 @@
 import javax.jcr.Credentials;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.UnsupportedRepositoryOperationException;
+import javax.jcr.ValueFactory;
 
 import org.apache.jackrabbit.rmi.remote.RemoteItem;
 import org.apache.jackrabbit.rmi.remote.RemoteNode;
@@ -271,4 +273,14 @@
         }
     }
 
+ /* (non-Javadoc)
+ * @see org.apache.jackrabbit.rmi.remote.RemoteSession#getValueFactory()
+ */
+ public ValueFactory getValueFactory() throws RepositoryException, RemoteException {
+ try {
+ return session.getValueFactory();
+ } catch (RepositoryException ex) {
+ throw getRepositoryException(ex);
+ }
+ }
 }
Index: src/java/org/apache/jackrabbit/rmi/server/ServerQueryManager.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/server/ServerQueryManager.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/server/ServerQueryManager.java (working copy)
@@ -80,7 +80,7 @@
     }
 
     /** {@inheritDoc} */
-    public String[] getSupportedQueryLanguages() throws RemoteException {
+    public String[] getSupportedQueryLanguages() throws RemoteException, RepositoryException {
         return manager.getSupportedQueryLanguages();
     }
 
Index: src/java/org/apache/jackrabbit/rmi/remote/RemoteSession.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/remote/RemoteSession.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/remote/RemoteSession.java (working copy)
@@ -23,6 +23,7 @@
 
 import javax.jcr.Credentials;
 import javax.jcr.RepositoryException;
+import javax.jcr.ValueFactory;
 
 /**
  * Remote version of the JCR {@link javax.jcr.Session Session} interface.
@@ -370,5 +371,13 @@
      */
     byte[] exportDocumentView(String path, boolean skipBinary, boolean noRecurse)
         throws IOException, RepositoryException, RemoteException;
-
+    /**
+     * Remote version of the
+     * {@link javax.jcr.Session#getValueFactory() Session.getValueFactory()} method.
+     *
+     * @return value factory
+     * @throws RemoteException on RMI errors
+     * @see javax.jcr.Session#getValueFactory()
+     */
+    ValueFactory getValueFactory() throws RepositoryException, RemoteException;
 }
Index: src/java/org/apache/jackrabbit/rmi/remote/SerialValue.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/remote/SerialValue.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/remote/SerialValue.java (working copy)
@@ -24,20 +24,21 @@
 import java.io.Serializable;
 import java.util.Calendar;
 
-import javax.jcr.BinaryValue;
-import javax.jcr.BooleanValue;
-import javax.jcr.DateValue;
-import javax.jcr.DoubleValue;
-import javax.jcr.LongValue;
-import javax.jcr.NameValue;
-import javax.jcr.PathValue;
 import javax.jcr.PropertyType;
-import javax.jcr.ReferenceValue;
 import javax.jcr.RepositoryException;
-import javax.jcr.StringValue;
 import javax.jcr.Value;
 import javax.jcr.ValueFormatException;
 
+import org.apache.jackrabbit.core.value.BinaryValue;
+import org.apache.jackrabbit.core.value.BooleanValue;
+import org.apache.jackrabbit.core.value.DateValue;
+import org.apache.jackrabbit.core.value.DoubleValue;
+import org.apache.jackrabbit.core.value.LongValue;
+import org.apache.jackrabbit.core.value.NameValue;
+import org.apache.jackrabbit.core.value.PathValue;
+import org.apache.jackrabbit.core.value.ReferenceValue;
+import org.apache.jackrabbit.core.value.StringValue;
+
 /**
  * Serializable {@link Value Value} decorator. A SerialValue decorator
  * makes it possible to serialize the contents of a Value object even
Index: src/java/org/apache/jackrabbit/rmi/remote/RemoteQueryManager.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/remote/RemoteQueryManager.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/remote/RemoteQueryManager.java (working copy)
@@ -66,6 +66,6 @@
      * @return An string array.
      * @throws RemoteException on RMI errors
      */
-    String[] getSupportedQueryLanguages() throws RemoteException;
+    String[] getSupportedQueryLanguages() throws RepositoryException, RemoteException;
 
 }
Index: src/java/org/apache/jackrabbit/rmi/client/ClientQueryManager.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/client/ClientQueryManager.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/client/ClientQueryManager.java (working copy)
@@ -82,7 +82,7 @@
     }
 
     /** {@inheritDoc} */
-    public String[] getSupportedQueryLanguages() {
+    public String[] getSupportedQueryLanguages() throws RepositoryException {
         try {
             return remote.getSupportedQueryLanguages();
         } catch (RemoteException ex) {
Index: src/java/org/apache/jackrabbit/rmi/client/ClientSession.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/client/ClientSession.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/client/ClientSession.java (working copy)
@@ -30,6 +30,8 @@
 import javax.jcr.Repository;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.UnsupportedRepositoryOperationException;
+import javax.jcr.ValueFactory;
 import javax.jcr.Workspace;
 import javax.xml.transform.Result;
 import javax.xml.transform.Source;
@@ -63,6 +65,7 @@
     /** The adapted remote session. */
     private RemoteSession remote;
 
+    
     /**
      * Creates a client adapter for the given remote session.
      *
@@ -425,4 +428,12 @@
             throw new RemoteRuntimeException(e);
         }
     }
+ /* (non-Javadoc)
+ * @see javax.jcr.Session#getValueFactory()
+ */
+ public ValueFactory getValueFactory()
+ throws UnsupportedRepositoryOperationException, RepositoryException {
+ // TODO Auto-generated method stub
+ return null;//remote;
+ }
 }
Index: src/java/org/apache/jackrabbit/rmi/client/ClientProperty.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/client/ClientProperty.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/client/ClientProperty.java (working copy)
@@ -20,21 +20,21 @@
 import java.rmi.RemoteException;
 import java.util.Calendar;
 
-import javax.jcr.BinaryValue;
-import javax.jcr.BooleanValue;
-import javax.jcr.DateValue;
-import javax.jcr.DoubleValue;
 import javax.jcr.ItemVisitor;
-import javax.jcr.LongValue;
 import javax.jcr.Node;
 import javax.jcr.Property;
-import javax.jcr.ReferenceValue;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
-import javax.jcr.StringValue;
 import javax.jcr.Value;
 import javax.jcr.nodetype.PropertyDefinition;
 
+import org.apache.jackrabbit.core.value.BinaryValue;
+import org.apache.jackrabbit.core.value.BooleanValue;
+import org.apache.jackrabbit.core.value.DateValue;
+import org.apache.jackrabbit.core.value.DoubleValue;
+import org.apache.jackrabbit.core.value.LongValue;
+import org.apache.jackrabbit.core.value.ReferenceValue;
+import org.apache.jackrabbit.core.value.StringValue;
 import org.apache.jackrabbit.rmi.remote.RemoteProperty;
 import org.apache.jackrabbit.rmi.remote.SerialValue;
 
Index: src/java/org/apache/jackrabbit/rmi/client/ClientNode.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/client/ClientNode.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/client/ClientNode.java (working copy)
@@ -20,28 +20,28 @@
 import java.rmi.RemoteException;
 import java.util.Calendar;
 
-import javax.jcr.BinaryValue;
-import javax.jcr.BooleanValue;
-import javax.jcr.DateValue;
-import javax.jcr.DoubleValue;
 import javax.jcr.Item;
 import javax.jcr.ItemVisitor;
-import javax.jcr.LongValue;
 import javax.jcr.Node;
 import javax.jcr.NodeIterator;
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
-import javax.jcr.ReferenceValue;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
-import javax.jcr.StringValue;
 import javax.jcr.Value;
 import javax.jcr.lock.Lock;
+import javax.jcr.nodetype.NodeDefinition;
 import javax.jcr.nodetype.NodeType;
-import javax.jcr.nodetype.NodeDefinition;
 import javax.jcr.version.Version;
 import javax.jcr.version.VersionHistory;
 
+import org.apache.jackrabbit.core.value.BinaryValue;
+import org.apache.jackrabbit.core.value.BooleanValue;
+import org.apache.jackrabbit.core.value.DateValue;
+import org.apache.jackrabbit.core.value.DoubleValue;
+import org.apache.jackrabbit.core.value.LongValue;
+import org.apache.jackrabbit.core.value.ReferenceValue;
+import org.apache.jackrabbit.core.value.StringValue;
 import org.apache.jackrabbit.rmi.remote.RemoteLock;
 import org.apache.jackrabbit.rmi.remote.RemoteNode;
 import org.apache.jackrabbit.rmi.remote.RemoteProperty;
Reply | Threaded
Open this post in threaded view
|

Re: JCR-RMI 0.16.4.1 diffs

Jukka Zitting-5
Hi,

Ludovic Maitre wrote:
> I have do some modifications to JCR RMI so it is compatible with the
> last SVN release of Jackrabbit, i hope.

Very nice, thanks! However, I'm sorry for being too quiet about working
with Felix Meschberger to do the same upgrade. :-( I'm expecting a final
patch from Felix within a few days.

I hope you are not taken back about this double work. I reviewed your
patch and it is fine except for the following issues.

> The changes are:
> - add a dependency to jackrabbit to use his valueholders class
> (LongValue...)

I'm not too happy about introducing a direct Jackrabbit dependency into
JCR-RMI. The idea of JCR-RMI is to be a general RMI binding to various
JCR repositories. A direct Jackrabbit dependency breaks this generality.

In this case the problem is not as big as it seems, as the dependency
only covers the Value implementation classes. It is possible to simply
copy those classes with minor modifications into JCR-RMI to avoid the
full dependency. (This is actually what Felix was doing.)

Better yet, we've been discussing with Felix about the possibility of
making this part of JCR-RMI into a general purpose Value implementation
that could be placed in the JCR-EXT package. More on this in a few days.

> - add the getValueFactory() to the remote and local sessions. I'm really
> not sure if we should permit to get the valuefactory, but i have done
> this so this compile.

I don't think the ValueFactory interface needs to be exported over RMI.
Just make ClientSession maintain it's own ValueFactory implementation.

> - add an ignore rule for this new method into the test because i don't
> know what is appropriate to use it.

That's fine.

> So if somebody will check if this modifications are ok, they can be
> committed.

Based on the unnecessary Jackrabbit dependency and the upcoming patch
from Felix, I must unfortunately reject your patch. I hope you are OK
with this and understand the reasoning.

> I must precise that i have never use the jcr-rmi ext before so i perhaps
> that i have modified this in an inapproriate manner.

I appreciate your effort and initiative! Unfortunately it got somewhat
wasted because of little communication on my part. I hope seeing more
contributions from you!

BR,

Jukka Zitting
Reply | Threaded
Open this post in threaded view
|

Re: JCR-RMI 0.16.4.1 diffs - Correction

Ludovic Maître
In reply to this post by Ludovic Maître
Ludovic Maitre wrote:

> Hello,
>
> I have do some modifications to JCR RMI so it is compatible with the
> last SVN release of Jackrabbit, i hope.

Hi,
I have forgot to implement the delegate method for retrieving the
ValueFactory in

src/java/org/apache/jackrabbit/rmi/client/ClientSession.java


The attached patch should be ok. The method getValueFactory(0 is tested
and the tests give no errors.
Sorry for having forgot to re-re-read my code before sending it,

Best Regards,

--
Cordialement,
Ludo - http://www.ubik-products.com
---
"L'amour pour principe et l'ordre pour base; le progres pour but" (A.Comte)


Index: project.xml
===================================================================
--- project.xml (revision 169133)
+++ project.xml (working copy)
@@ -21,7 +21,7 @@
   <pomVersion>3</pomVersion>
   <id>jcr-rmi</id>
   <name>JCR-RMI</name>
-  <currentVersion>0.16.4</currentVersion>
+  <currentVersion>0.16.4.1</currentVersion>
   <inceptionYear>2004</inceptionYear>
   <package>org.apache.jackrabbit.rmi.*</package>
   <description>
@@ -60,10 +60,15 @@
     <dependency>
       <groupId>jsr170</groupId>
       <artifactId>jcr</artifactId>
-      <version>0.16.4</version>
+      <version>0.16.4.1</version>
       <url>http://www.day.com/maven/jsr170/jars/jcr-0.16.4.jar</url>
     </dependency>
     <dependency>
+      <groupId>jackrabbit</groupId>
+      <artifactId>jackrabbit</artifactId>
+      <version>0.16.4.1-dev</version>
+    </dependency>
+    <dependency>
       <groupId>xerces</groupId>
       <artifactId>xercesImpl</artifactId>
       <version>2.6.2</version>
Index: src/java/org/apache/jackrabbit/rmi/server/ServerSession.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/server/ServerSession.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/server/ServerSession.java (working copy)
@@ -25,6 +25,8 @@
 import javax.jcr.Credentials;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.UnsupportedRepositoryOperationException;
+import javax.jcr.ValueFactory;
 
 import org.apache.jackrabbit.rmi.remote.RemoteItem;
 import org.apache.jackrabbit.rmi.remote.RemoteNode;
@@ -271,4 +273,14 @@
         }
     }
 
+ /* (non-Javadoc)
+ * @see org.apache.jackrabbit.rmi.remote.RemoteSession#getValueFactory()
+ */
+ public ValueFactory getValueFactory() throws RepositoryException, RemoteException {
+ try {
+ return session.getValueFactory();
+ } catch (RepositoryException ex) {
+ throw getRepositoryException(ex);
+ }
+ }
 }
Index: src/java/org/apache/jackrabbit/rmi/server/ServerQueryManager.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/server/ServerQueryManager.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/server/ServerQueryManager.java (working copy)
@@ -80,7 +80,7 @@
     }
 
     /** {@inheritDoc} */
-    public String[] getSupportedQueryLanguages() throws RemoteException {
+    public String[] getSupportedQueryLanguages() throws RemoteException, RepositoryException {
         return manager.getSupportedQueryLanguages();
     }
 
Index: src/java/org/apache/jackrabbit/rmi/remote/RemoteSession.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/remote/RemoteSession.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/remote/RemoteSession.java (working copy)
@@ -23,6 +23,7 @@
 
 import javax.jcr.Credentials;
 import javax.jcr.RepositoryException;
+import javax.jcr.ValueFactory;
 
 /**
  * Remote version of the JCR {@link javax.jcr.Session Session} interface.
@@ -370,5 +371,13 @@
      */
     byte[] exportDocumentView(String path, boolean skipBinary, boolean noRecurse)
         throws IOException, RepositoryException, RemoteException;
-
+    /**
+     * Remote version of the
+     * {@link javax.jcr.Session#getValueFactory() Session.getValueFactory()} method.
+     *
+     * @return value factory
+     * @throws RemoteException on RMI errors
+     * @see javax.jcr.Session#getValueFactory()
+     */
+    ValueFactory getValueFactory() throws RepositoryException, RemoteException;
 }
Index: src/java/org/apache/jackrabbit/rmi/remote/SerialValue.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/remote/SerialValue.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/remote/SerialValue.java (working copy)
@@ -24,20 +24,21 @@
 import java.io.Serializable;
 import java.util.Calendar;
 
-import javax.jcr.BinaryValue;
-import javax.jcr.BooleanValue;
-import javax.jcr.DateValue;
-import javax.jcr.DoubleValue;
-import javax.jcr.LongValue;
-import javax.jcr.NameValue;
-import javax.jcr.PathValue;
 import javax.jcr.PropertyType;
-import javax.jcr.ReferenceValue;
 import javax.jcr.RepositoryException;
-import javax.jcr.StringValue;
 import javax.jcr.Value;
 import javax.jcr.ValueFormatException;
 
+import org.apache.jackrabbit.core.value.BinaryValue;
+import org.apache.jackrabbit.core.value.BooleanValue;
+import org.apache.jackrabbit.core.value.DateValue;
+import org.apache.jackrabbit.core.value.DoubleValue;
+import org.apache.jackrabbit.core.value.LongValue;
+import org.apache.jackrabbit.core.value.NameValue;
+import org.apache.jackrabbit.core.value.PathValue;
+import org.apache.jackrabbit.core.value.ReferenceValue;
+import org.apache.jackrabbit.core.value.StringValue;
+
 /**
  * Serializable {@link Value Value} decorator. A SerialValue decorator
  * makes it possible to serialize the contents of a Value object even
Index: src/java/org/apache/jackrabbit/rmi/remote/RemoteQueryManager.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/remote/RemoteQueryManager.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/remote/RemoteQueryManager.java (working copy)
@@ -66,6 +66,6 @@
      * @return An string array.
      * @throws RemoteException on RMI errors
      */
-    String[] getSupportedQueryLanguages() throws RemoteException;
+    String[] getSupportedQueryLanguages() throws RepositoryException, RemoteException;
 
 }
Index: src/java/org/apache/jackrabbit/rmi/client/ClientQueryManager.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/client/ClientQueryManager.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/client/ClientQueryManager.java (working copy)
@@ -82,7 +82,7 @@
     }
 
     /** {@inheritDoc} */
-    public String[] getSupportedQueryLanguages() {
+    public String[] getSupportedQueryLanguages() throws RepositoryException {
         try {
             return remote.getSupportedQueryLanguages();
         } catch (RemoteException ex) {
Index: src/java/org/apache/jackrabbit/rmi/client/ClientSession.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/client/ClientSession.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/client/ClientSession.java (working copy)
@@ -30,6 +30,8 @@
 import javax.jcr.Repository;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
+import javax.jcr.UnsupportedRepositoryOperationException;
+import javax.jcr.ValueFactory;
 import javax.jcr.Workspace;
 import javax.xml.transform.Result;
 import javax.xml.transform.Source;
@@ -63,6 +65,7 @@
     /** The adapted remote session. */
     private RemoteSession remote;
 
+    
     /**
      * Creates a client adapter for the given remote session.
      *
@@ -425,4 +428,15 @@
             throw new RemoteRuntimeException(e);
         }
     }
+ /* (non-Javadoc)
+ * @see javax.jcr.Session#getValueFactory()
+ */
+ public ValueFactory getValueFactory()
+ throws UnsupportedRepositoryOperationException, RepositoryException {
+ try {
+ return remote.getValueFactory();
+ } catch (RemoteException e) {
+ throw new RemoteRuntimeException(e);
+ }
+ }
 }
Index: src/java/org/apache/jackrabbit/rmi/client/ClientProperty.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/client/ClientProperty.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/client/ClientProperty.java (working copy)
@@ -20,21 +20,21 @@
 import java.rmi.RemoteException;
 import java.util.Calendar;
 
-import javax.jcr.BinaryValue;
-import javax.jcr.BooleanValue;
-import javax.jcr.DateValue;
-import javax.jcr.DoubleValue;
 import javax.jcr.ItemVisitor;
-import javax.jcr.LongValue;
 import javax.jcr.Node;
 import javax.jcr.Property;
-import javax.jcr.ReferenceValue;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
-import javax.jcr.StringValue;
 import javax.jcr.Value;
 import javax.jcr.nodetype.PropertyDefinition;
 
+import org.apache.jackrabbit.core.value.BinaryValue;
+import org.apache.jackrabbit.core.value.BooleanValue;
+import org.apache.jackrabbit.core.value.DateValue;
+import org.apache.jackrabbit.core.value.DoubleValue;
+import org.apache.jackrabbit.core.value.LongValue;
+import org.apache.jackrabbit.core.value.ReferenceValue;
+import org.apache.jackrabbit.core.value.StringValue;
 import org.apache.jackrabbit.rmi.remote.RemoteProperty;
 import org.apache.jackrabbit.rmi.remote.SerialValue;
 
Index: src/java/org/apache/jackrabbit/rmi/client/ClientNode.java
===================================================================
--- src/java/org/apache/jackrabbit/rmi/client/ClientNode.java (revision 169133)
+++ src/java/org/apache/jackrabbit/rmi/client/ClientNode.java (working copy)
@@ -20,28 +20,28 @@
 import java.rmi.RemoteException;
 import java.util.Calendar;
 
-import javax.jcr.BinaryValue;
-import javax.jcr.BooleanValue;
-import javax.jcr.DateValue;
-import javax.jcr.DoubleValue;
 import javax.jcr.Item;
 import javax.jcr.ItemVisitor;
-import javax.jcr.LongValue;
 import javax.jcr.Node;
 import javax.jcr.NodeIterator;
 import javax.jcr.Property;
 import javax.jcr.PropertyIterator;
-import javax.jcr.ReferenceValue;
 import javax.jcr.RepositoryException;
 import javax.jcr.Session;
-import javax.jcr.StringValue;
 import javax.jcr.Value;
 import javax.jcr.lock.Lock;
+import javax.jcr.nodetype.NodeDefinition;
 import javax.jcr.nodetype.NodeType;
-import javax.jcr.nodetype.NodeDefinition;
 import javax.jcr.version.Version;
 import javax.jcr.version.VersionHistory;
 
+import org.apache.jackrabbit.core.value.BinaryValue;
+import org.apache.jackrabbit.core.value.BooleanValue;
+import org.apache.jackrabbit.core.value.DateValue;
+import org.apache.jackrabbit.core.value.DoubleValue;
+import org.apache.jackrabbit.core.value.LongValue;
+import org.apache.jackrabbit.core.value.ReferenceValue;
+import org.apache.jackrabbit.core.value.StringValue;
 import org.apache.jackrabbit.rmi.remote.RemoteLock;
 import org.apache.jackrabbit.rmi.remote.RemoteNode;
 import org.apache.jackrabbit.rmi.remote.RemoteProperty;
Reply | Threaded
Open this post in threaded view
|

Re: JCR-RMI 0.16.4.1 diffs

Ludovic Maître
In reply to this post by Jukka Zitting-5
Jukka Zitting wrote:

> Hi,
>
> Ludovic Maitre wrote:
>
>> I have do some modifications to JCR RMI so it is compatible with the
>> last SVN release of Jackrabbit, i hope.
>
>
> Very nice, thanks! However, I'm sorry for being too quiet about
> working with Felix Meschberger to do the same upgrade. :-( I'm
> expecting a final patch from Felix within a few days.
>
> I hope you are not taken back about this double work. I reviewed your
> patch and it is fine except for the following issues.
> [...]
> Based on the unnecessary Jackrabbit dependency and the upcoming patch
> from Felix, I must unfortunately reject your patch. I hope you are OK
> with this and understand the reasoning.
>
>> I must precise that i have never use the jcr-rmi ext before so i
>> perhaps that i have modified this in an inapproriate manner.
>
>
> I appreciate your effort and initiative! Unfortunately it got somewhat
> wasted because of little communication on my part. I hope seeing more
> contributions from you!
>
> BR,
>
> Jukka Zitting
>
>
No worry, it's not wasted time, as i must use it for a demo in one week,
so i must have a working implementation now (even if yours is better
mine will work i hope ;-).  Thanks for your review, i can use it safely
now,

Have a good day,

--
Cordialement,
Ludo - http://www.ubik-products.com
---
"L'amour pour principe et l'ordre pour base; le progres pour but" (A.Comte)

Reply | Threaded
Open this post in threaded view
|

Re: JCR-RMI 0.16.4.1 diffs

Jukka Zitting-5
Hi,

Ludovic Maitre wrote:
> No worry, it's not wasted time, as i must use it for a demo in one week,
> so i must have a working implementation now (even if yours is better
> mine will work i hope ;-).  Thanks for your review, i can use it safely
> now,

Good to hear that! Please let me know if you have any problems that I
can help you with.

BR,

Jukka Zitting


Reply | Threaded
Open this post in threaded view
|

Re: JCR-RMI 0.16.4.1 diffs

Felix Meschberger-4
In reply to this post by Jukka Zitting-5
Hi Jukka,

I just finished the implementation of the Value stuff in the jcr-rmi
project.

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

(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 :-)

(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.

(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.

There are still some issues to be resolved:

(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 ?

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

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

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

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 sommon project
such as the ISO8601 helper class providing Calendar formatting and
parsing according to the spec.

I will now go on to test this thing and would be happy to receive any
feedback regarding this issue.

The patch can be retrieved from here:
http://www.day.com/o.file/jcr-rmi-0.16.4.1-20050509fm.patch?get=7d837039f01bfbb0f0819bcafeae4168

Regards
Felix

Reply | Threaded
Open this post in threaded view
|

RE: JCR-RMI 0.16.4.1 diffs

Kirkham, Pete (UK)
In reply to this post by Ludovic Maître
>The state classes implement the (similar) interface StatefullValue - if you
come up with a better name, please go ahead :-)

Either StatefulValue or StateFullValue depending what you mean (having state or tell me the full state)?


Pete
Reply | Threaded
Open this post in threaded view
|

Re: JCR-RMI 0.16.4.1 diffs

Jukka Zitting-5
In reply to this post by Felix Meschberger-4
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
Reply | Threaded
Open this post in threaded view
|

Re: JCR-RMI 0.16.4.1 diffs

Felix Meschberger-4
Hi,

Thanks for checking in and your comments.

On second thought, I have to admit, that they are very valid, so I
propose to adapt also according to your initial Value implementation:

(1) Rename StatefullValue to StatefulValue (thanks to Pete for
reporting  :-) and StatefullValueAdapter to StatefulValueAdapter.

(2) Have StatefulValue extend Value and provding no additional API. I
prefer StatefulValue over ValueState because the latter sounds "the
state of a value" whereas I want to convey "Value with a state".

(3) Revert back to make all StatefulValue instances being Serializable
and thus removing the readObject/writeObject stuff. Would it be bad to
have StatefulValue to be Serializable ? I do not think that this would
collide with extensibility; to the opposite, as it would guarantee that
at the biggest extent possible, the same implementation is used before
and after serilialization whereas the current writeObject implementation
cannot guarantee this.

What do you think ?

>> (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

I do not understand this :-)

I took this implementation to factor our common behaviour and to provide
specialized overwriting behaviour.

Regards
Felix

Reply | Threaded
Open this post in threaded view
|

Re: JCR-RMI 0.16.4.1 diffs

Jukka Zitting-5
Hi,

Felix wrote:
> (1) Rename StatefullValue to StatefulValue (thanks to Pete for
> reporting  :-) and StatefullValueAdapter to StatefulValueAdapter.
>
> (2) Have StatefulValue extend Value and providing no additional API. I
> prefer StatefulValue over ValueState because the latter sounds "the
> state of a value" whereas I want to convey "Value with a state".

OK, sounds good.

However, I'd still prefer the name ValueState though, as these classes
represent Values in a specific and permanent state. In essence they are
"states of a value".

In fact I think that StatefulValue would probably be a more appropriate
name for the current SerialValue class, as it essentially is a "value
with a state" that can change over time.

> (3) Revert back to make all StatefulValue instances being Serializable
> and thus removing the readObject/writeObject stuff. Would it be bad to
> have StatefulValue to be Serializable ? I do not think that this would
> collide with extensibility; to the opposite, as it would guarantee that
> at the biggest extent possible, the same implementation is used before
> and after serialization whereas the current writeObject implementation
> cannot guarantee this.

Brilliant! I didn't think of that possibility. I'll make the change in a
moment.

This change actually gives a better reason for the existence of the
StatefulValueAdapter class as the readObject/writeObject methods should
be placed there.

BaseNonStreamValue class:

> > 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
>
> I do not understand this :-)
>
> I took this implementation to factor our common behaviour and to provide
> specialized overwriting behaviour.

:-) It's a matter of taste really.

The reasoning is that I consider the ValueFormatException thrown by
DateValue.getBoolean() conceptually different from the one thrown by
DoubleValue.getBoolean() because they are specified in different cells
of the property type conversion matrix in the JCR spec. Without the base
class each value class would actually implement the full semantics of a
single row of the type conversion matrix. Although the base class
reduces the amount of mostly similar code, it spreads the implementation
semantics over a wider range of classes.

In contrast I actually think that using the base class for throwing
IllegalStateExceptions from getStream() makes sense conceptually.

But this is mostly just splitting design hair anyhow as the actual
behaviour of the classes is identical in any case. :-) I'll leave the
base class as it is.

BR,

Jukka Zitting


Reply | Threaded
Open this post in threaded view
|

Re: JCR-RMI 0.16.4.1 diffs

Felix Meschberger-4
Hi,

>However, I'd still prefer the name ValueState though, as these classes
>represent Values in a specific and permanent state. In essence they are
>"states of a value".
>
>In fact I think that StatefulValue would probably be a more appropriate
>name for the current SerialValue class, as it essentially is a "value
>with a state" that can change over time.
>  
>
Ok, got it and I have to agree.

Regards
Felix Meschberger