chemistry-dev mailing list archives

Site index · List index
Message view « Date » · « Thread »
Top « Date » · « Thread »
From Mike Hatfield <mike.hatfi...@alfresco.com>
Subject Re: Added Query Escaping
Date Fri, 18 Jul 2014 09:15:52 GMT
Hi Lukas

Thanks for adding this - it's definitely something we needed in ObjectiveCMIS!

Regarding the implementation, I have a few comments:

1 - My main concern is whether using '?' as token in "SELECT ?, ? FROM ? WHERE ? > ? AND
IN_FOLDER(?) OR ? IN (?)" is too fragile?

We have a similar token replacement scenario in our mobile app whilst parsing user activity
data, but in that case we use a numbered indexing scheme, e.g. "{1} renamed wiki page from
{2} to {0}". This has several advantages from my point of view: it means the string can be
reformatted without manual reordering of parameter passing and also makes the parameters within
the string easier to identify (instead of counting '?'s). Tokens are easily found using rangeOfString:
NSRange indexRange = [[inStr string] rangeOfString:[NSString stringWithFormat:@"{%@}", @(index)]];

It would also be possible to write the query generator to allow parameter re-use within the
string without having to escape the same parameter again.


2 - I notice convert: is allocating an NSDateFormatter each time it's called. Apple recommend
that NSDateFormatters are cached and reused, see [1]
[1] https://developer.apple.com/library/ios/documentation/Cocoa/Conceptual/DataFormatting/Articles/dfDateFormatting10_4.html#//apple_ref/doc/uid/TP40002369-SW10


3 - It's a shame that new code in ObjectiveCMIS isn't being written using modern syntax, for
example:
    [self.parametersDictionary setObject:[CMISQueryStatement escapeString:type withSurroundingQuotes:NO]
forKey:[NSNumber numberWithInteger:parameterIndex]];
becomes
    self.parametersDictionary[@(parameterIndex)] = [CMISQueryStatement escapeString:type withSurroundingQuotes:NO];


4 - Finally, a minor point; I think some line breaks got lost in the formatting for the setStringContainsAtIndex:string:
documentation:
 * Summary (input --> first level escaping --> second level escaping and
 * output): * --> * --> * ? --> ? --> ? - --> - --> - \ --> \\ -->
\\\\ (for
 * any other character following other than * ? -) \* --> \* --> \\* \? -->
 * \? --> \\? \- --> \- --> \\- ' --> \' --> \\\' " --> \" --> \\\"


Thanks,
Mike


On 17 Jul 2014, at 16:59, Gross, Lukas <lukas.gross@sap.com<mailto:lukas.gross@sap.com>>
wrote:

Hi,

I added CMISQueryStatement class to enable users to escape their queries using kind of a prepared
statement approach.
Please let me know if you have any objections…
I plan to trigger the .4 release soon, so please have a look :)

Best regards,
Lukas


Mime
  • Unnamed multipart/alternative (inline, None, 0 bytes)
View raw message