chemistry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From "Sutter, Peter" <peter.sut...@sap.com>
Subject Re: Browser binding re-factoring
Date Mon, 05 May 2014 06:55:08 GMT
Hi Gavin,

yes agree with your second proposal (supply start/end data with flag for
HttpUploadRequest).

I moved the media type constants there because I needed
kCMISMediaTypeOctetStream also for browser binding. And OpenCMIS library
has those constants also in the
org.apache.chemistry.opencmis.commons.impl.Constants class. So at least
the constant kCMISMediaTypeOctetStream should be there. Don’t mind if you
want to move the others back.

We have a similar method in our coding which is on a NSString category but
it’s also fine for me if we change it back to a private method on the
CMISURLUtil class. 


Best regards,
Peter

On 5/2/14, 5:33 PM, "Gavin Cornwell" <gavin.cornwell@alfresco.com> wrote:

>Hi,
>
>I’ve completed the agreed refactoring on the browser binding branch and
>also fixed the underlying issues causing the last remaining tests to
>fail. With the fixes in place all tests are now passing against both
>bindings. I’ve therefore changed the test config file to run the tests
>against both bindings by default.
>
>To fix the JSON parsing issue I’ve added a hack that should be by-passed
>if Apple ever fix the bug. We now remove the minValue and maxValue
>properties from the JSON if a parsing error occurs and re-try and parse
>operation.
>
>I took a look at CMISHttpUploadRequest, I agree we need to move the
>prepareXMLWithCMISProperties method out, we could introduce an elaborate
>mechanism where custom HttpUpload/DownloadRequest objects could be
>plugged into the network provider, but I think the easiest/most sensible
>thing to do is to just expose the underlying base64Encoding flag as a
>parameter and use the combinedStream stream feature you’ve added. Both
>bindings can then supply the start/end data and set the flag
>appropriately, what do you think?
>
>A couple of other questions following the changes provided in CMIS-794...
>
>Was there a particular reason why the atom pub specific media type
>constants (kCMISMediaTypeService etc.) got moved into the common
>CMISConstants file? I personally think these should be moved back to
>CMISAtomPubConstants as they’re only used by that binding, do you agree?
>
>Finally, was there a particular reason to make the replacePathWithPath
>method in CMISURLUtil a category? It’s only used in this class so just
>wondering why it’s not a simple private method?
>
>Have a great weekend.
>
>Regards,
>
>Gavin
>
>
>
>On 1 May 2014, at 16:35, Gavin Cornwell <gavin.cornwell@alfresco.com>
>wrote:
>
>> Hi Peter,
>> 
>> Excellent.
>> 
>> I have assigned CMIS-794 to myself, I’ll commit this tomorrow as well.
>> 
>> I will also take a look at the CMISHttpUploadRequest issue you mention.
>> 
>> Regards,
>> 
>> Gavin
>> 
>> 
>> 
>> On 1 May 2014, at 15:53, Sutter, Peter <peter.sutter@sap.com> wrote:
>> 
>>> Hi Gavin,
>>> 
>>> I just uploaded the current state of my implementation:
>>> https://issues.apache.org/jira/browse/CMIS-794
>>> 
>>> Now all methods of the interfaces should be implemented. I have also
>>> tested the browser binding implementation in our app and it works very
>>> nicely as far as I can see.
>>> 
>>> There is maybe one area where I¹m not really sure because the
>>> CMISHttpUploadRequest class contains atom pub specific coding
>>> (prepareXMLWithCMISProperties) which should probably be moved down to
>>>the
>>> respective atom class.
>>> And still there is the JSONSerializer bug with very large/small double
>>> values where we need to decide how we deal with it.
>>> 
>>> You can start with your part 2 refactoring, no problem. Will only do
>>>minor
>>> changes in the next working days, if any.
>>> 
>>> Best regards,
>>> Peter
>>> 
>>> 
>>> On 5/1/14, 11:03 AM, "Gavin Cornwell" <gavin.cornwell@alfresco.com>
>>> wrotsue:
>>> 
>>>> Hi Peter,
>>>> 
>>>> Great work on the browser binding so far, it¹s really coming together.
>>>> 
>>>> As you¹ve probably seen by now, I did part 1 of the re-factoring we
>>>> agreed upon previously (updating property names).
>>>> 
>>>> Part 2 is going to be a bit more disruptive as it¹s going to involve
>>>> renaming and moving quite a few classes (ensure all Atom classes are
>>>> named as such). So I thought it would be best to co-ordinate this with
>>>> you to avoid a merge headache for you and I.
>>>> 
>>>> I was planning on doing this tomorrow, do you think you¹ll have your
>>>> latest set of changes committed by then?
>>>> 
>>>> As a side note, I looked into the UriBuilder classes last night and
>>>> found, as you¹d hinted at, they are pretty specific to the AtomPub
>>>> binding so I¹m going to rename them as such and move the constants
>>>>from
>>>> the generic CMISBindingSession class to the CMISAtomPubConstants
>>>>class.
>>>> 
>>>> Following this, I¹ll be available to help out with the actual
>>>> implementation, if there are any areas you¹d like me to look at (that
>>>>are
>>>> not on your critical path) please let me know.
>>>> 
>>>> Regards,
>>>> 
>>>> Gavin
>>> 
>> 
>

Mime
View raw message