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