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

On 14 June 2012 13:12, Vladimir Berezniker <vmpn_at_hitechman.com> wrote:
>
>
>
> On Thu, Jun 14, 2012 at 4:37 AM, Hyrum K Wright <hyrum.wright_at_wandisco.com> wrote:
>>
>> On Thu, Jun 14, 2012 at 10:16 AM, Mat Booth <mat.booth_at_wandisco.com> wrote:
>> > 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/ ?
>>
>
> Are there any particular features of the RA JavaHL layer that you would
> be interested in trying out?
>

To begin with, just the interrogation methods, like get_log,
get_file_revs, get_file, get_dir, etc. Did you have a plan of which
parts to implement first?

I look forward to giving a try when it starts reaching a usable state.
Maybe I will even be able to report bugs for you :-)

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