chemistry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Gavin Cornwell <gavin.cornw...@alfresco.com>
Subject Re: Browser binding re-factoring
Date Fri, 02 May 2014 15:33:51 GMT
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