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" <[email protected]> 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 <[email protected]> >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 <[email protected]> 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" <[email protected]> >>> 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 >>> >> >
