Attached please find revised patch that added a comment to the JNIObject
class to
indicate that it is not part of the public API.
[[[
JavaHL: New method for creating java objects linked to their C++ counterpart
[ in subversion/bindings/javahl/native ]
* SVNBase.cpp,
SVNBase.h
(createCppBoundObject): New method for creating java objects linked to
their
C++ counterpart
[ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]
* JNIObject.java: Base class for JNI linked java objects
]]]
Thank you,
Vladimir
On Mon, Jun 11, 2012 at 8:39 AM, Vladimir Berezniker <v...@hitechman.com>wrote:
>
>
> On Mon, Jun 11, 2012 at 6:20 AM, Hyrum K Wright <hyrum.wri...@wandisco.com
> > wrote:
>
>> On Thu, Jun 7, 2012 at 4:03 AM, Vladimir Berezniker <v...@hitechman.com>
>> wrote:
>> > Hi All,
>> >
>> > The intention if this patch is to introduce reusable logic for creating
>> java
>> > objects from within C++. This keeps object creation logic fully within
>> C++
>> > while leaving to java the decision as to when they will be destroyed. It
>> > will be used by RA code to allocate container object for items like
>> SVNRa,
>> > Editor, Directory and File.
>> >
>> > Thank you,
>> >
>> > Vladimir
>> >
>> > [[[
>> > JavaHL: New method for creating java objects linked to their C++
>> counterpart
>> >
>> > [ in subversion/bindings/javahl/native ]
>> >
>> > * SVNBase.cpp, SVNBase.h
>> > (createCppBoundObject): New method for creating java objects linked to
>> > their
>> > C++ counterpart
>> >
>> > [ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]
>> >
>> > * JNIObject.java: Base class for JNI linked java objects
>> > ]]]
>>
>> Looks good. Is there a way to mark the new Java class as private?
>>
>
> The original plan is to put RA java classes in a ra/ sub package, so
> package visible
> would not work in such case. It is an abstract class, even if someone
> instantiates
> it will be harmless, as it will not create any C++ instances. Would
> putting a good
> javadoc explaining it is not part of the public API address your concerns?
>
>
>>
>> How do you plan to use this functionality?
>
>
> As compared to what SVNClient class does, where caller creates the java
> object which
> in turn calls native method call to create a matching native object. With
> RA layer,
> consumer calls a factory method:
>
> /**
> * Crates RA session for a given url with provided context
> * @param url to connect to
> * @param uuid of the remote repository, can be null if uuid check is not
> desired
> * @param config configuration to use for the session.
> * @return RA session
> */
> public static native ISVNRa createRaSession(String url, String uuid,
> ISVNRaConfig config);
> }
>
> that method is responsible for instantiation of the related C++ and Java
> classes.
> I think it is cleaner because:
>
> - It better encapsulates implementation class: User only sees factory
> and ISVNRa interface
> - Instantiation logic resides in a single language
> - It avoids chicken and the egg with not year having cppAddr when java
> object is constructed: Allows enforcement that parameter is supplied via
> constructor argument
>
> Relatedly, do you have a
>
>> high-level plan for this series of patches?
>
>
> Let me get the up to date version published. But high level summary for
> now:
>
> 1. Have the following 3 patches reviewed, so that minimal version of SVNRa
> implementation
> can be committed to the javahl-ra branch and merged with the existing code
> base.
>
> - [PATCH] JavaHL: Add SVN_JNI_STRING macro to reduce amount of code
> necessary to declare JNIStringHolder and check for exceptions
> - [PATCH] JavaHL: New method for creating java objects linked to their
> C++ counterpart
> - [PATCH] JavaHL: Factor out common context to be shared between
> SVNClient and SVNRa classes
>
> 2. Commit additional RA function calls that do not require additional
> enhancements to the
> common code to the branch. These are mostly pass-through calls to the C
> functions.
>
> 3. Submit another ~6 patches for review om shared code that will support
> RA Editor code
>
> 4. Commit revised RA Editor implementation to the branch
>
> 5. Present proposal on making RA layer use Runtime exceptions rather than
> checked ones
>
>
> (Or, perhaps it was in
>> your first set of mail, and I've just overlooked it...)
>>
>> -Hyrum
>>
>>
> Thank you for you continuous feedback, it is much appreciated,
>
> Vladimir
>
>
>>
>> --
>>
>> uberSVN: Apache Subversion Made Easy
>> http://www.uberSVN.com/
>>
>
>
Index: subversion/bindings/javahl/native/SVNBase.cpp
===================================================================
--- subversion/bindings/javahl/native/SVNBase.cpp (revision 1347593)
+++ subversion/bindings/javahl/native/SVNBase.cpp (working copy)
@@ -97,3 +97,29 @@
}
}
}
+
+jobject SVNBase::createCppBoundObject(const char *clazzName)
+{
+ JNIEnv *env = JNIUtil::getEnv();
+
+ // Create java session object
+ jclass clazz = env->FindClass(clazzName);
+ if (JNIUtil::isJavaExceptionThrown())
+ return NULL;
+
+ static jmethodID ctor = 0;
+ if (ctor == 0)
+ {
+ ctor = env->GetMethodID(clazz, "<init>", "(J)V");
+ if (JNIUtil::isJavaExceptionThrown())
+ return NULL;
+ }
+
+ jlong cppAddr = this->getCppAddr();
+
+ jobject jself = env->NewObject(clazz, ctor, cppAddr);
+ if (JNIUtil::isJavaExceptionThrown())
+ return NULL;
+
+ return jself;
+}
Index: subversion/bindings/javahl/native/SVNBase.h
===================================================================
--- subversion/bindings/javahl/native/SVNBase.h (revision 1347593)
+++ subversion/bindings/javahl/native/SVNBase.h (working copy)
@@ -82,6 +82,11 @@
*/
void dispose(jobject jthis, jfieldID *fid, const char *className);
+ /**
+ * Instantiates java object attached to this base object
+ */
+ jobject createCppBoundObject(const char *clazzName);
+
private:
/**
* If the value pointed to by @a fid is zero, find the @c jfieldID
Index:
subversion/bindings/javahl/src/org/apache/subversion/javahl/JNIObject.java
===================================================================
--- subversion/bindings/javahl/src/org/apache/subversion/javahl/JNIObject.java
(revision 0)
+++ subversion/bindings/javahl/src/org/apache/subversion/javahl/JNIObject.java
(working copy)
@@ -0,0 +1,43 @@
+/**
+ * @copyright
+ * ====================================================================
+ * Licensed to the Apache Software Foundation (ASF) under one
+ * or more contributor license agreements. See the NOTICE file
+ * distributed with this work for additional information
+ * regarding copyright ownership. The ASF licenses this file
+ * to you under the Apache License, Version 2.0 (the
+ * "License"); you may not use this file except in compliance
+ * with the License. You may obtain a copy of the License at
+ *
+ * http://www.apache.org/licenses/LICENSE-2.0
+ *
+ * Unless required by applicable law or agreed to in writing,
+ * software distributed under the License is distributed on an
+ * "AS IS" BASIS, WITHOUT WARRANTIES OR CONDITIONS OF ANY
+ * KIND, either express or implied. See the License for the
+ * specific language governing permissions and limitations
+ * under the License.
+ * ====================================================================
+ * @endcopyright
+ */
+
+package org.apache.subversion.javahl;
+
+/**
+ * This class is used internally by the JavaHL implementation and not
+ * considered part part of the public API.
+ */
+public abstract class JNIObject
+{
+ /**
+ * slot for the address of the native peer.
+ * The JNI code controls this field. If it is set to 0 then
+ * underlying JNI object has been freed
+ */
+ protected long cppAddr;
+
+ protected JNIObject(long cppAddr)
+ {
+ this.cppAddr = cppAddr;
+ }
+}
