On Thu, Jun 14, 2012 at 4:37 AM, Hyrum K Wright <hyrum.wri...@wandisco.com>wrote:
> 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/ ? > > Are there any particular features of the RA JavaHL layer that you would be interested in trying out? > I think that's the goal. We're encouraging him to put as much "common > functionality" type stuff on trunk, though. > Yep, that is the approach I am following. To give more context to the way this has been progressing: I submit few (0 to ~3) patches to make it easier to review while developing few steps ahead on my local PC so that if/when the patches are signed off I have the next batch ready, without getting too much ahead; as I keep realizing better ways to design the code due to the received feedback. I try to keep the plan reflective of my local progress, so where you see more detail that means I have at least partial code locally, and where the plan has less detail that means the code to back that step is not there yet. Regards, Vladimir
