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?
>
>
>
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@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@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.@1343456TBR]

      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]



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

Reply via email to