add escapeStringForLIKE to mozIStorageStatement

RESOLVED FIXED in mozilla1.9alpha8

Status

()

Toolkit
Storage
RESOLVED FIXED
11 years ago
11 years ago

People

(Reporter: (not reading, please use seth@sspitzer.org instead), Assigned: (not reading, please use seth@sspitzer.org instead))

Tracking

Trunk
mozilla1.9alpha8
Points:
---
Dependency tree / graph
Bug Flags:
in-litmus +

Firefox Tracking Flags

(Not tracked)

Details

Attachments

(1 attachment, 4 obsolete attachments)

add EscapeForLIKE to mozIStorageStatement

attaching a patch and a test case. 

sdwilsh wrote that he'd like to make this return a UTF8String, but I'm not sure that is ideal.

I'd prefer to keep it an AString, to avoid conversions.  I've fixed my C++ in history autocomplete from:

  rv = mDBAutoCompleteQuery->BindUTF8StringParameter(2, NS_ConvertUTF16toUTF8(escapedSearchString));
  NS_ENSURE_SUCCESS(rv, rv);

to

  rv = mDBAutoCompleteQuery->BindUTF8StringParameter(2, escapedSearchString);
  NS_ENSURE_SUCCESS(rv, rv);
Attachment #276137 - Flags: review? → review?(sdwilsh)
So, I'm thinking that letting the consumer specify what they want the escape character to be would be a very good thing.

Also, for consistency with the interface, let's call it |escapeStringForLIKE|.

I'm also wondering if, for C++ consumers, we want to have an |escapeUTF8StringForLIKE| so they do not have to convert.

Lastly, can you indicate in the javadoc header that consumers will have to add the ESCAPE {their character} in as well?
Created attachment 276169 [details] [diff] [review]
patch

shawn, I did not add the UTF8 version, as the current consumer in C++ doesn't need it.

Sample consumer code:

  nsString escapedSearchString;
  rv = mDBAutoCompleteQuery->EscapeStringForLIKE(mCurrentSearchString, PRUnichar('/'), escapedSearchString);
  NS_ENSURE_SUCCESS(rv, rv);

  // prepend and append with % for "contains"
  rv = mDBAutoCompleteQuery->BindStringParameter(2, NS_LITERAL_STRING("%") + escapedSearchString + NS_LITERAL_STRING("%"));
  NS_ENSURE_SUCCESS(rv, rv);
Attachment #276137 - Attachment is obsolete: true
Attachment #276169 - Flags: review?
Attachment #276137 - Flags: review?(sdwilsh)
Attachment #276169 - Flags: review? → review?(sdwilsh)
Created attachment 276171 [details] [diff] [review]
improve comments in idl
Attachment #276169 - Attachment is obsolete: true
Attachment #276171 - Flags: review?(sdwilsh)
Attachment #276169 - Flags: review?(sdwilsh)
Comment on attachment 276171 [details] [diff] [review]
improve comments in idl

>+/* AString escapeStringForLIKE(in AString aValue, in char aEscapeChar); */
>+NS_IMETHODIMP
>+mozStorageStatement::EscapeStringForLIKE(const nsAString & aValue, const PRUnichar aEscapeChar, nsAString &aEscapedValue)
nit: 80 char line wrapping
nit: s/aEscapedValue/escapedString/

>+{
>+    const PRUnichar MATCH_ALL('%');
>+    const PRUnichar MATCH_ONE('_');
>+
>+    aEscapedValue.Truncate(0);
>+
>+    PRInt32 i;
>+    for (i = 0; i < aValue.Length(); i++) {
nit: declare i inside the for loop

>+      if (aValue[i] == aEscapeChar || aValue[i] == MATCH_ALL || aValue[i] == MATCH_ONE)
nit: 80 char line wrapping

>+        aEscapedValue += aEscapeChar;
nit: indentation is 4 spaces in this file, not 2

r=sdwilsh, with nits fixed.
Attachment #276171 - Flags: review?(sdwilsh) → review+
Blocks: 389491
No longer depends on: 389491
Flags: in-litmus+
Target Milestone: --- → mozilla1.9 M8
Created attachment 276195 [details] [diff] [review]
patch as checked in
Attachment #276171 - Attachment is obsolete: true
Attachment #276195 - Flags: review+
Created attachment 276196 [details] [diff] [review]
patch as checked in
Attachment #276195 - Attachment is obsolete: true
Attachment #276196 - Flags: review+
fixed.

Checking in public/mozIStorageStatement.idl;
/cvsroot/mozilla/storage/public/mozIStorageStatement.idl,v  <--  mozIStorageStat
ement.idl
new revision: 1.9; previous revision: 1.8
done
Checking in src/mozStorageStatement.cpp;
/cvsroot/mozilla/storage/src/mozStorageStatement.cpp,v  <--  mozStorageStatement
.cpp
new revision: 1.25; previous revision: 1.24
done
RCS file: /cvsroot/mozilla/storage/test/unit/test_like_escape.js,v
done
Checking in test/unit/test_like_escape.js;
/cvsroot/mozilla/storage/test/unit/test_like_escape.js,v  <--  test_like_escape.
js
initial revision: 1.1
done
Status: NEW → RESOLVED
Last Resolved: 11 years ago
Resolution: --- → FIXED
Summary: add EscapeForLIKE to mozIStorageStatement → add escapeStringForLIKE to mozIStorageStatement

Updated

11 years ago
Keywords: dev-doc-needed

Updated

11 years ago
Blocks: 399535
Removing doc needed keyword in favor of a new bug that covers the fact that in general we actually need proper documentation for Storage; that bug (bug 399535) is set as blocked by this one.
Keywords: dev-doc-needed
You need to log in before you can comment on or make changes to this bug.