Re: [PATCH] JavaHL: Factor out common context to be shared between SVNClient and SVNRa classes

Attached please find the revised patch for the RaSharedContext (temporary
name
pending your feedback) with whitespace fixed factored out. The whitespace
changes
will be applied in the subsequent patch.

As I cannot get svn to generate patches that deal with copies of existing
files,
before applying below the following svn copy commands have to be issued:

svn cp
subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java
subversion/bindings/javahl/src/org/apache/subversion/javahl/RaSharedContext.java
svn cp subversion/bindings/javahl/native/ClientContext.h
subversion/bindings/javahl/native/RaSharedContext.h
svn cp subversion/bindings/javahl/native/ClientContext.cpp
subversion/bindings/javahl/native/RaSharedContext.cpp

[[[
JavaHL: Factor out common context to be shared between SVNClient and SVNRa
classes

[ in subversion/bindings/javahl/src/org/tigris/subversion/javahl/ ]

* RaSharedContext.java,
  SVNClient.java
  (ClientContext): Move to progress listener into RaSharedContext

[ in subversion/bindings/javahl/native ]

* RaSharedContext.cpp,
  RaSharedContext.h,
  ClientContext.cpp,
  ClientContext.h
  (username, password, getConfigDirectory, setConfigDirectory, setPrompt,
   cancelOperation, progress): Move from ClientContext to RaSharedContext

* RaSharedContext.cpp,
  RaSharedContext.h
  (attachJavaObject): New function to hold common logic of attaching to the
    java CommonContext class used for callbacks
  (getConfigData, getAuthBaton): Split getContext into separate
configuration
    data setup and authentication data setup to better reflect their
different life cycles
  (getClientName): New function providing client name to be used in
callbacks

* ClientContext.cpp,
  ClientContext.h
  (ClientContext, getContext): Use the factored out RaSharedContext member
    variables and functions
]]]

Thank you,

Vladimir

On Mon, Jun 11, 2012 at 9:53 AM, Vladimir Berezniker <vmpn_at_hitechman.com>wrote:

>
>
> On Mon, Jun 11, 2012 at 9:09 AM, Hyrum K Wright <hyrum.wright_at_wandisco.com
> > wrote:
>
>> On Mon, Jun 11, 2012 at 2:58 PM, Vladimir Berezniker <vmpn_at_hitechman.com>
>> wrote:
>> >
>> >
>> > On Mon, Jun 11, 2012 at 6:56 AM, Hyrum K Wright <
>> hyrum.wright_at_wandisco.com>
>> > wrote:
>> >>
>> >> On Thu, Jun 7, 2012 at 5:09 AM, Vladimir Berezniker <
>> vmpn_at_hitechman.com>
>> >> wrote:
>> >> > Greetings,
>> >> >
>> >> > This patch signifies a point post which I can merge the existing
>> logic
>> >> > on
>> >> > the
>> >> > javahl-ra branch with starting parts of my proposed enhancements. So
>> I
>> >> > hope
>> >> > people still have some patience left to review them.
>> >> >
>> >> > This patch moves parts of client context that are shared with ra
>> context
>> >> > into
>> >> > common classes. As I cannot get svn to generate patches that deal
>> with
>> >> > copies
>> >> > of existing file, before applying below the following svn copy
>> command
>> >> > have
>> >> > to
>> >> > be issued:
>> >> >
>> >> > svn cp
>> >> >
>> >> >
>> subversion/bindings/javahl/src/org/apache/subversion/javahl/SVNClient.java
>> >> >
>> >> >
>> subversion/bindings/javahl/src/org/apache/subversion/javahl/CommonContext.java
>> >> > svn cp subversion/bindings/javahl/native/ClientContext.h
>> >> > subversion/bindings/javahl/native/CommonContext.h
>> >> > svn cp subversion/bindings/javahl/native/ClientContext.cpp
>> >> > subversion/bindings/javahl/native/CommonContext.cpp
>> >> >
>> >> >
>> >> > Also please note that java part of this patch is already applied in
>> >> > r1343452
>> >> > & r1347345
>> >> > on javahl-ra branch, due to my earlier mistake.
>> >>
>> >> If this work is specific to the things happening on the branch, I
>> >> think you're fine committing it directly there.
>> >
>> >
>> > Similar to the other patches so far, while the logic is used by RA
>> > layer. The re factoring is applicable to the trunk on its own. Though
>> > not as useful as it does not take care of the additional flexibility on
>> > its own. It does lean more towards RA specific enhancements than the
>> > other patches.
>> >
>> >>
>> >>
>> >> One question, though. The ClientContext object directly mirrors the
>> >> svn_client_context_t struct in the C layer (the analogs are probably
>> >> obvious). To my knowledge, we don't have a similar struct for the C
>> >> ra layer. I'm just a little confused why we need to break this clear
>> >> correlation with the C layer in Java, when we don't need to in C.
>> >> I'll need to think on this a bit...
>> >
>> >
>> > The refactoring represents the common elements between
>> > the svn_client_ctx_t,
>> > svn_ra_callbacks2_t and svn_commit_callback2_t needed by the
>> > (svn_ra_get_commit_editor3) function. This overlap is demonstrated by
>> the
>> > existing code in the (svn_client__open_ra_session_internal) function
>> that
>> > goes
>> > between the client context and ra session. Specifically client and ra
>> layers
>> > share
>> > the following objects:
>> >
>> > * svn_auth_baton_t
>> > * apr_hash_t (config) needed to create svn_auth_baton_t
>> > * svn_ra_progress_notify_func_t
>> >
>> > Hope this helps,
>>
>> It helps a lot! Should we then call the new object something a bit
>> more descriptive? Maybe CommitContext?
>>
>
> How about RASharedContext? It is RA layer data that is used across client
> and
> ra functions.
>
> Also, I have not correctly described the role of the svn_commit_callback2_t.
> While
> its JNI implementation is shared between client and ra layers. It is not
> part of the
> svn_client_ctx_t nor svn_ra_callbacks2_t nor their JNI counterparts.
> Rather, it is
> passed directly to the svn_client_commit6 and svn_ra_get_commit_editor3
> functions.
>
> Sorry for the confusion, my brain was still thinking of patch series,
> where one of the
> patches factors this out to common code, when I mentioned it.
>
> Vladimir
>
>
>
>
>>
>> -Hyrum
>>
>>
>>
>> --
>>
>> uberSVN: Apache Subversion Made Easy
>> http://www.uberSVN.com/
>>
>
>