Re: [PATCH] JavaHL: New method for creating java objects linked to their C++ counterpart

On 12 June 2012 03:11, Vladimir Berezniker <vmpn_at_hitechman.com> wrote:
>
>
> On Mon, Jun 11, 2012 at 8:39 AM, Vladimir Berezniker <vmpn_at_hitechman.com>
> wrote:
>>
>>
>>
>> On Mon, Jun 11, 2012 at 6:20 AM, Hyrum K Wright
>> <hyrum.wright_at_wandisco.com> wrote:
>>>
>>> On Thu, Jun 7, 2012 at 4:03 AM, Vladimir Berezniker <vmpn_at_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?
>>
>>
>
> More detailed plan, full version is in the BRANCH-README on javahl-ra
> branch:
>
>    * Prepare existing code for merging of SVNRa editor       [IN PROGRESS]
>
>       01. JavaHL: Changed return value from the
>           java svn_stream_t read function to be compatible
>           with the txdelta_next_window function              [trunk_at_1342720]
>
>       02. JavaHL: Explicitly pass jobject jthis when
>           processing dispose() call rather  than stashing a
>           reference in the SVNBase class where it can be
>           misused later                                      [trunk_at_1342810]
>
>       03. JavaHL: Add SVN_JNI_STRING macro to reduce amount
>           of code necessary to declare JNIStringHolder and
>           check for exceptions                               [IN REVIEW]
>
>       04. JavaHL: New method for creating java objects
>           linked to their C++ counterpart                    [IN REVIEW]
>
>       05. JavaHL: Factored out common context for later use
>           by SVNRa class                                     [IN REVIEW]
>
>       06. JavaHL: Minimal implementation of SVNRa            [TODO]
>
>       07. JavaHL: Merge existing SVNRepoAccess into SVNRA    [TODO]
>
>
>    * Implement other ra functions not requiring addition code
>      refactoring                                             [TODO]
>
>
>    * Apply editor support patches                            [TODO]
>       xx. JavaHL: Support returning non const, empty rather
>           than NULL hash as required by
>           (svn_ra_get_commit_editor3)
>           apr_hash_t *revprop_table parameter                [br._at_1343456
> TBR]
>
>       xx. JavaHL: Support keeping global reference to the
>           callback java object as required by the RA API due
>           to callback being used across method calls         [TODO]
>
>       xx. JavaHL: Added support for creating of svn_string_t
>           from JNIByteArray                                  [TODO]
>
>       xx. JavaHL: Added SVN_JNI_BYTE_ARRAY macro to reduce
>           amount of duplicate code dealing with jbyteArray
>           wrapper and checking for exceptions                [TODO]
>
>       xx. JavaHL: Factor out common java string map
>           processing into StringsTable class from
>           svn_string_t specific processing in the
>           RevpropTable class                                 [TODO]
>
>

Hi Vladimir,

When you start implementing JavaHL RA, what would be the best way for
me to try it out? Will the work be on this branch first:
http://svn.apache.org/repos/asf/subversion/branches/javahl-ra/ ?

Regards,
Mat

-- 
Mat Booth
Software Engineer
WANdisco, Inc.
http://www.wandisco.com