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 implementation
Date Mon, 21 Apr 2014 20:19:11 GMT
Hi Peter,

Apologies, my comment about CMISBrowserUtil was a little unfair, it is still fairly simple
right now, my concern was the complexity we'll introduce if it has to start doing remote calls
to get type definitions. 

Yes, each binding implementation would have to do it's own type caching. Personally, I think
the caching is currently in the wrong place, CMISSession should just call the binding each
time and the caching should be done by the binding itself (just as OpenCMIS does).

Regards,

Gavin

Sent from my iPad


> On 18 Apr 2014, at 11:51, "Sutter, Peter" <peter.sutter@sap.com> wrote:
> 
> Hi Gavin,
> 
> I saw the CMISBrowserUtil similar to the JSONConverter in OpenCMIS. Can
> you point me to an example where this class currently does more than
> simple JSON to CMIS object conversion?
> 
> Regarding your proposal moving the type definition retrieval method to the
> base class and putting the type cache into the bintoSession, this would
> mean that each binding implementation has to manage the type cache on it’s
> own, right? Because currently this is done “centrally” in the CMISSession
> class for all binding implementations which we have to remove then.
> 
> Best regards,
> Peter
> 
> 
> 
>> On 4/17/14, 11:08 PM, "Gavin Cornwell" <gavin.cornwell@alfresco.com> wrote:
>> 
>> Hi Peter,
>> 
>> I have applied the patch you attached to CMIS-787.
>> 
>> I also applied the patch from Mike attached to CMIS-784. The default test
>> server for the browser binding branch is now an Alfresco 4.2 Enterprise
>> server. I’ve left the other servers configured but disabled.
>> 
>> I now see what you mean about the retrieving type definitions from
>> CMISBrowserUtil. To be honest I saw this class as being a simple helper
>> class with simple JSON to CMIS object parsing methods, it’s already going
>> beyond that.
>> 
>> I think we should consider adding the common parsing code to
>> CMISBrowserBaseService (but defined as a Protected category like in the
>> AtomPub binding) as there’s a good chance it’s going to be required by
>> the other services as we implement them. The typeDefinition retrieval
>> method could also be added to the base class if that’s required to parse
>> succinct properties.
>> 
>> If we need to do type definition caching (it sounds like a good idea) we
>> should store that in the bindingSession.
>> 
>> What do you think?
>> 
>> I’m still looking through your other changes, I’ll send a separate email
>> with my thoughts soon.
>> 
>> Regards,
>> 
>> Gavin
>> 
>> 
>> 
>>> On 17 Apr 2014, at 19:38, Sutter, Peter <peter.sutter@sap.com> wrote:
>>> 
>>> Hi Mike, Hi Gavin,
>>> 
>>> I went for the category approach. Will also attach the diff of my
>>> changes
>>> to jira in a few seconds.
>>> 
>>> 
>>> The AtomPub binding does not need fetch the type definitions as they
>>> come
>>> along with the response..
>>> 
>>> Best regards,
>>> Peter
>>> 
>>> 
>>> On 4/17/14, 5:35 PM, "Gavin Cornwell" <gavin.cornwell@alfresco.com>
>>> wrote:
>>> 
>>>> Hi Peter,
>>>> 
>>>> Yes, NSNull is one of those annoyances of parsing JSON in ObjectiveC!
>>>> What approach did you decide to go with?
>>>> 
>>>> I haven¹t taken a look at your first set of changes yet (I will try to
>>>> tonight) but passing CMISSession around the binding implementation
>>>> doesn¹t sound like something we want to do to be honest. The
>>>> CMISSession
>>>> classes are supposed to sit on top of the binding implementation so
>>>> doing
>>>> this could introduce some interesting cyclic dependencies.
>>>> 
>>>> I¹m not sure what the answer is to be honest at the moment. If we need
>>>> access to the type cache maybe that¹s an indication the one we have is
>>>> in
>>>> the wrong place. The AtomPub binding must have faced a similar issue
>>>> how
>>>> has it dealt with the issue you¹re seeing?
>>>> 
>>>> I will try and take a look tonight and come back with something a
>>>> little
>>>> more concrete.
>>>> 
>>>> Regards,
>>>> 
>>>> Gavin
>>>> 
>>>> 
>>>> 
>>>> On 17 Apr 2014, at 13:48, Mike Hatfield <mike.hatfield@alfresco.com>
>>>> wrote:
>>>> 
>>>>> Hi Peter
>>>>> 
>>>>> This JSON behaviour presumably stems from the limitation on
>>>>> NSDictionary and NSArray that they cannot contain nil objects, so
>>>>> NSNull
>>>>> is used instead.
>>>>> 
>>>>> There seem to be two popular workarounds if the client code requires
>>>>> nil instead of NSNull:
>>>>> 1 - A category on NSDictionary, e.g. -
>>>>> (id)cmis_objectForKeyNotNull:(id)key { Š }
>>>>> 2 - A macro, e.g. NilOrObjectForKey(json, key) which would do the same
>>>>> thing.
>>>>> 
>>>>> Neither of those solutions would add too much overhead, although
>>>>> clearly we would no longer benefit from modern Objective-C syntax in
>>>>> those instances.
>>>>> 
>>>>> I'm not currently up-to-speed with the browser binding branch, so
>>>>> can't
>>>>> comment on your second issue.
>>>>> 
>>>>> Regards,
>>>>> Mike
>>>>> 
>>>>>> On 17 Apr 2014, at 13:17, Sutter, Peter <peter.sutter@sap.com>
wrote:
>>>>>> 
>>>>>> Hi there,
>>>>>> 
>>>>>> I¹m facing currently an issue with [NSJSONSerialization
>>>>>> JSONObjectWithData:options:error:] and would like to hear your
>>>>>> opinion.
>>>>>> The method returns for keys that do not have a value the NSNull
>>>>>> object.
>>>>>> 
>>>>>> E.g. when reading the repository info and the description is null
on
>>>>>> the server,
>>>>>> 
>>>>>> repoInfo.desc = repo[kCMISBrowserJSONRepositoryDescription];
>>>>>> 
>>>>>> 
>>>>>> repoInfo.desc has the NSNull object instead of nil. Is there a better
>>>>>> way than to wrap this "dictionary[<key>]² with a helper method?
>>>>>> 
>>>>>> Something like
>>>>>> 
>>>>>> + (id)valueOrNil:(id)value
>>>>>> 
>>>>>> {
>>>>>> 
>>>>>> return value == [NSNull null] ? nil : value;
>>>>>> 
>>>>>> }
>>>>>> 
>>>>>> But then we have to wrap all calls when accessing the value of the
>>>>>> dictionary with such a method and this would blow up the code.. or
>>>>>> do I
>>>>>> miss something here?
>>>>>> 
>>>>>> 
>>>>>> The other thing is I need to access the CMISSession within the
>>>>>> CMISBrowserUtil so that I can retrieve the type definitions (and
also
>>>>>> make use of the type cache) when converting the succinctProperties.
>>>>>> 
>>>>>> I could add the CMISSession as property to the
>>>>>> CMISBrowserBaseService.
>>>>>> From there I could pass the session to the CMISBrowserUtil. Any
>>>>>> objections or even a better approach as this seems to me like a dirty
>>>>>> workaround?
>>>>>> 
>>>>>> 
>>>>>> Best regards,
>>>>>> 
>>>>>> Peter
> 

Mime
View raw message