On Thu, Jun 14, 2012 at 10:16 AM, Mat Booth <mat.bo...@wandisco.com> wrote: > On 12 June 2012 03:11, Vladimir Berezniker <v...@hitechman.com> wrote: >> >> >> 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.@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/ ?
I think that's the goal. We're encouraging him to put as much "common functionality" type stuff on trunk, though. -Hyrum -- uberSVN: Apache Subversion Made Easy http://www.uberSVN.com/