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

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