Merge lp:~kalikiana/u1db-qt/wonderiousFields into lp:u1db-qt

[Cris Dywan]

Reverse query logic to check non-matching and internally convert between query syntaxes

- bug 1215898 example works
- bug 1214538 was already fixed before it seems
- bug 1284194 works

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:116
http://jenkins.qa.ubuntu.com/job/u1db-qt-ci/41/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-precise-amd64-ci/41
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-saucy-amd64-ci/43
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-trusty-amd64-ci/21
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-trusty-armhf-ci/21

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/u1db-qt-ci/41/rebuild

review: Approve (continuous-integration)

[Nekhelesh Ramananthan (nik90)]

On testing, I found the following results,
1. bug #1214538 is fixed
2. bug #1284194 doesn't work with 2 Queries. With 1 Query, the filtering of elements with "show" works as expected.

[PS Jenkins bot (ps-jenkins)]

PASSED: Continuous integration, rev:119
http://jenkins.qa.ubuntu.com/job/u1db-qt-ci/43/
Executed test runs:
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-precise-amd64-ci/43
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-saucy-amd64-ci/45
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-trusty-amd64-ci/23
    SUCCESS: http://jenkins.qa.ubuntu.com/job/u1db-qt-trusty-armhf-ci/23

Click here to trigger a rebuild:
http://s-jenkins.ubuntu-ci:8080/job/u1db-qt-ci/43/rebuild

review: Approve (continuous-integration)

[Benjamin Zeller (zeller-benjamin)]

3 Mentioned examples work, ready to merge

review: Approve

=== modified file 'src/query.cpp'
--- src/query.cpp	2014-02-19 09:39:07 +0000
+++ src/query.cpp	2014-03-07 17:54:50 +0000
@@ -122,6 +122,34 @@
 void Query::generateQueryResults()
 {
     QList<QVariantMap> results(m_index->getAllResults());
+
+    /* Convert "*" or 123 or "aa" into  a list */
+    /* Also convert ["aa", 123] into [{foo:"aa", bar:123}] */
+    QVariantList queryList(m_query.toList());
+    if (queryList.empty()) {
+        // * is the default if query is empty
+        if (!m_query.isValid())
+            queryList.append(QVariant(QString("*")));
+        else
+            queryList.append(m_query);
+    }
+    if (queryList.at(0).type() != QVariant::Map) {
+        QVariantList oldQueryList(queryList);
+        QListIterator<QVariant> j(oldQueryList);
+        QListIterator<QString> k(m_index->getExpression());
+        while(j.hasNext() && k.hasNext()) {
+            QVariant j_value = j.next();
+            QString k_value = k.next();
+            QVariantMap valueMap;
+            // Strip hierarchical components
+            if (k_value.contains("."))
+                valueMap.insert(k_value.split(".").last(), j_value);
+            else
+                valueMap.insert(k_value, j_value);
+            queryList.append(QVariant(valueMap));
+        }
+    }
+
     Q_FOREACH (QVariantMap mapIdResult, results) {
         QString docId((mapIdResult["docId"]).toString());
         QVariant result_variant(mapIdResult["result"]);
@@ -135,10 +163,9 @@
 
             j.next();
 
-            bool tmp_match = queryField(j.key(), j.value());
-
-            if(tmp_match == false){
+            if (!iterateQueryList(queryList, j.key(), j.value())) {
                 match = false;
+                break;
             }
 
         }
@@ -158,7 +185,6 @@
 
     Q_EMIT documentsChanged(m_documents);
     Q_EMIT resultsChanged(m_results);
-
 }
 
 /*!
@@ -175,87 +201,41 @@
 
 /*!
     \internal
-    Query a single field.
- */
-bool Query::queryField(QString field, QVariant value){
-
-    bool match = false;
-
-    QVariant query = getQuery();
-    // * is the default if query is empty
-    if (!query.isValid())
-        query = QVariant(QString("*"));
-    QString typeName = query.typeName();
-
-    if(typeName == "QString")
-    {
-        QString query_string = query.toString();
-        match = queryString(query_string, value);
-    }
-    else if(typeName == "int")
-    {
-        QString query_string = query.toString();
-        match = queryString(query_string, value);
-    }
-    else if(typeName == "QVariantList")
-    {
-        match = iterateQueryList(query, field, value);
-    }
-    else
-    {
-        m_query = "";
-        qWarning("u1db: Unexpected type %s for query", qPrintable(typeName));
-    }
-
-    return match;
-
-}
-
-/*!
-    \internal
     Loop through the query assuming it's a list.
+
+    For example:
+    queryList: { type: 'show' }
+    field: 'type'
+    value: 'show'
  */
-bool Query::iterateQueryList(QVariant query, QString field, QVariant value)
+bool Query::iterateQueryList(QVariantList queryList, QString field, QVariant value)
 {
-
-    bool match = false;
-
-    QList<QVariant> query_list = query.toList();
-    QListIterator<QVariant> j(query_list);
+    QListIterator<QVariant> j(queryList);
 
     while (j.hasNext()) {
-
         QVariant j_value = j.next();
-
-        QString typeName = j_value.typeName();
-
-        if(typeName == "QVariantMap")
-        {
-            match = queryMap(j_value.toMap(), value.toString(), field);
-
-            if(match == true){
-                break;
-            }
-
-        }
-        else if(typeName == "QString"){
-
-            match = queryString(j_value.toString(), value);
-
-            if(match == true){
-                break;
-            }
-
-        }
-        else
-        {
-            m_query = "";
-            qWarning("u1db: Unexpected type %s for query", qPrintable(typeName));
-        }
-
+        QVariantMap valueMap(j_value.toMap());
+        if (!queryMap(valueMap, value.toString(), field))
+            return false;
     }
 
-    return match;
+    return true;
+}
+
+/*!
+    \internal
+    Verify that query is an identical or wild card match.
+ */
+bool Query::queryMatchesValue(QString query, QString value)
+{
+    if (query == "*")
+        return true;
+    if (query == value)
+        return true;
+    if (!query.contains ("*"))
+       return false;
+    QString prefix(query.split("*")[0]);
+    return value.startsWith(prefix, Qt::CaseSensitive);
 }
 
 /*!
@@ -274,36 +254,20 @@
         return false;
     }
 
-    bool match = false;
-
-        if(query == "*"){
-            return true;
-        }
-        else if(query == value){
-            return true;
-        }
-        else if(query.contains("*")){
-            QStringList k_string_list = query.split("*");
-            QString k_string = k_string_list[0];
-            match = value.toString().startsWith(k_string,Qt::CaseSensitive);
-
-            return match;
-
-        }
-
-
-    return match;
+    return queryMatchesValue(query, value.toString());
 }
 
 /*!
     \internal
     Loop through the given map of keys and queries.
+
+    For example:
+    map: { type: 'show' }
+    value: { 'show' }
+    field: 'type'
  */
 bool Query::queryMap(QVariantMap map, QString value, QString field)
 {
-
-    bool match = false;
-
     QMapIterator<QString,QVariant> k(map);
 
     while(k.hasNext()){
@@ -314,24 +278,12 @@
         QString query = k_variant.toString();
 
         if(field == k_key){
-
-            if(query == "*"){
-                return true;
-            }
-            else if(query == value){
-                return true;
-            }
-            else if(query.contains("*")){
-                QStringList k_string_list = query.split("*");
-                QString k_string = k_string_list[0];
-                match = value.startsWith(k_string,Qt::CaseSensitive);
-                return match;
-            }
-
+            if (!queryMatchesValue(query, value))
+                return false;
         }
     }
 
-    return match;
+    return true;
 }
 
 /*!
 
=== modified file 'src/query.h'
--- src/query.h	2014-01-31 19:20:07 +0000
+++ src/query.h	2014-03-07 17:54:50 +0000
@@ -68,8 +68,10 @@
 
     void onDataInvalidated();
 
+    bool debug();
     void generateQueryResults();
-    bool iterateQueryList(QVariant query, QString field, QVariant value);
+    bool iterateQueryList(QVariantList list, QString field, QVariant value);
+    bool queryMatchesValue(QString query, QString value);
     bool queryString(QString query, QVariant value);
     bool queryMap(QVariantMap map, QString value, QString field);
     bool queryField(QString field, QVariant value);
 
=== modified file 'tests/tst_query.qml'
--- tests/tst_query.qml	2014-02-19 09:39:07 +0000
+++ tests/tst_query.qml	2014-03-07 17:54:50 +0000
@@ -26,7 +26,6 @@
 
     U1db.Database {
         id: gents
-        path: 'aDatabaseU'
     }
 
     U1db.Document {
@@ -47,6 +46,18 @@
         contents: { 'misc': { 'software': 'linux', 'sports': [ 'basketball', 'hockey' ] }, 'date': '2014-01-01' , 'gents': [ { 'name': 'Ivanka', 'phone': 00321 }, ] }
     }
 
+    U1db.Document {
+        database: gents
+        docId: 'F'
+        contents: { 'details': { 'name': 'spy', 'type': 'hide', 'colour': 'blue' } }
+    }
+
+    U1db.Document {
+        database: gents
+        docId: 'G'
+        contents: { 'details': { 'name': 'kid', 'type': 'show', 'colour': 'green' } }
+    }
+
     U1db.Index {
         id: byPhone
         database: gents
@@ -100,6 +111,12 @@
     U1db.Query {
         id: i12345Phone
         index: byPhone
+        query: [ { 'phone': 12345 } ]
+    }
+
+    U1db.Query {
+        id: i12345PhoneSimple
+        index: byPhone
         query: 12345
     }
 
@@ -112,6 +129,12 @@
     U1db.Query {
         id: ivankaAllNamePhone
         index: byNamePhone
+        query: [ { name: 'Ivanka', phone: '*' } ]
+    }
+
+    U1db.Query {
+        id: ivankaAllNamePhoneSimple
+        index: byNamePhone
         query: ['Ivanka', '*']
     }
 
@@ -122,17 +145,96 @@
     }
 
     U1db.Query {
-        id: wrongQuery
-        index: byNamePhone
-        query: [{ 'name': 'Ivanka', 'phone': '*' }]
-    }
-
-    U1db.Query {
         id: toplevelQuery
         index: byDate
         query: [{ 'date': '2014*', 'sports': 'basketball', 'software': 'linux' }]
     }
 
+    U1db.Query {
+        id: toplevelQuerySimple
+        index: byDate
+        query: [ '2014*', 'basketball', 'linux' ]
+    }
+
+    U1db.Query {
+        id: queryOne
+        index: U1db.Index {
+            database: gents
+            name: 'one'
+            expression: [ 'details.type' ]
+        }
+        query: [ 'show' ]
+    }
+
+    U1db.Query {
+        id: queryBothSimple
+        index: U1db.Index {
+            database: gents
+            name: 'bothSimple'
+            expression: [ 'details.type', 'details.colour' ]
+        }
+        query: [ 'show', '*' ]
+    }
+
+    U1db.Query {
+        id: queryBoth
+        index: U1db.Index {
+            database: gents
+            name: 'both'
+            expression: [ 'details.type', 'details.colour' ]
+        }
+        query: [ { type: 'show', colour: '*' } ]
+    }
+
+    U1db.Database {
+        id: tokusatsu
+    }
+
+    U1db.Document {
+        database: tokusatsu
+        docId: 'ooo'
+        contents: { 'series': 'ooo', 'type': 'rider' }
+    }
+
+    U1db.Document {
+        database: tokusatsu
+        docId: 'gokaiger'
+        contents: { 'series': 'gokaiger', 'type': 'sentai' }
+    }
+
+    U1db.Document {
+        id: tokusatsuDocumentWizard
+        docId: 'wizard'
+        contents: { 'series': 'wizard', 'type': 'rider',
+                    'transformations': ['Flame Style','Water Style'] }
+    }
+
+    U1db.Document {
+        id: tokusatsuDocumentDino
+        docId: 'dino'
+        contents: { 'series': 'zyuranger', 'scarf': false, 'type': 'sentai',
+                    'beasts': ['T-Rex', 'Mastodon'] }
+    }
+
+    U1db.Index {
+        id: bySeries
+        database: tokusatsu
+        name: 'by-series'
+        expression: ['series', 'type']
+    }
+
+    U1db.Query {
+        id: allHeroesWithType
+        index: bySeries
+        query: [{ 'series': '*' }, { 'type': '*' }]
+    }
+
+    U1db.Query {
+        id: allHeroesSeriesOnly
+        index: bySeries
+        query: [{ 'series': '*' }]
+    }
+
     SignalSpy {
         id: spyDocumentsChanged
         target: defaultPhone
@@ -154,7 +256,11 @@
         return A
     }
 
-    function compare (a, b) {
+    function compareFail(a, b, msg) {
+        compare(a, b, msg, true)
+    }
+
+    function compare(a, b, msg, willFail) {
         /* Override built-in compare to:
            Match different JSON for identical values (number hash versus list)
            Produce readable output for all JSON values
@@ -163,8 +269,14 @@
             return
         var A = prettyJson(a), B = prettyJson(b)
         if (A != B) {
-            fail('%1 != %2 (%3 != %4)'.arg(A).arg(B).arg(JSON.stringify(a)).arg(JSON.stringify(b)))
+            if (willFail) {
+                console.log('Expected failure: %1%2 != %3'.arg(msg ? msg + ': ' : '').arg(A).arg(B))
+                return
+            }
+            fail('%5%1 != %2 (%3 != %4)'.arg(A).arg(B).arg(JSON.stringify(a)).arg(JSON.stringify(b)).arg(msg ? msg + ': ' : ''))
         }
+        if (willFail)
+            fail('Expected to fail, but passed: %5%1 != %2 (%3 != %4)'.arg(A).arg(B).arg(JSON.stringify(a)).arg(JSON.stringify(b)).arg(msg ? msg + ': ' : ''))
     }
 
     function workaroundQueryAndWait (buggyQuery) {
@@ -173,21 +285,12 @@
         spyDocumentsChanged.wait();
     }
 
-    function test_0_wrongUse () {
-        workaroundQueryAndWait(wrongQuery)
-        ignoreWarning('u1db: Unexpected type QVariantMap for query')
-        wrongQuery.query = { 'name': 'Ivanka' }
-        ignoreWarning('u1db: Unexpected type QObject* for query')
-        wrongQuery.query = defaultPhone
-    }
-
     function test_1_defaults () {
         // We should get all documents
         workaroundQueryAndWait(defaultPhone)
         compare(defaultPhone.documents, ['1', '_', 'a'], 'uno')
         compare(defaultPhone.results.length, 3, 'dos')
         compare(defaultPhone.results.length, defaultPhone.documents.length, 'puntos')
-        // FIXME: compare(defaultPhone.results, [], 'dos')
         // These queries are functionally equivalent
         compare(defaultPhone.documents, allPhone.documents, 'tres')
         compare(defaultPhone.documents, allPhoneList.documents, 'quatro')
@@ -205,18 +308,21 @@
         compare(s12345Phone.documents, ['1'], 'uno')
         // It's okay to mix strings and numerical values
         compare(s12345Phone.documents, i12345Phone.documents, 'dos')
+        compare(i12345PhoneSimple.documents, i12345Phone.documents, 'tres')
     }
 
     function test_3_wildcards () {
         // Trailing string wildcard
         compare(s1wildcardPhone.documents, ['1'], 'uno')
         // Last given field can use wildcards
-        // FIXME: compare(ivankaAllNamePhone.documents, ['_', 'a'], 'dos')
+        compare(ivankaAllNamePhoneSimple.documents, ['_', 'a'], 'dos')
+        compare(ivankaAllNamePhone.documents, ['_', 'a'], 'tres')
         // These queries are functionally equivalent
         workaroundQueryAndWait(ivankaAllNamePhoneKeywords)
         workaroundQueryAndWait(ivankaAllNamePhone)
-        // FIXME: compare(ivankaAllNamePhone.documents, ivankaAllNamePhoneKeywords.documents, 'tres')
+        compare(ivankaAllNamePhone.documents, ivankaAllNamePhoneKeywords.documents, 'tres')
         compare(toplevelQuery.documents, ['_'])
+        compare(toplevelQuerySimple.documents, ['_'], 'cinco')
     }
 
     function test_4_delete () {
@@ -226,5 +332,29 @@
         compare(defaultPhone.documents, ['1', 'a'], 'dos')
     }
 
+    function test_5_fields () {
+        compare(queryOne.documents, ['G'], 'one field')
+        compare(queryBoth.documents, ['G'], 'two fields')
+        compare(queryBothSimple.documents, ['G'], 'two fields simple')
+    }
+
+    function test_6_definition () {
+        workaroundQueryAndWait(allHeroesWithType)
+        compare(allHeroesWithType.documents, ['gokaiger', 'ooo'], 'ichi')
+        workaroundQueryAndWait(allHeroesSeriesOnly)
+        compare(allHeroesSeriesOnly.documents, ['gokaiger', 'ooo'], 'ni')
+        compare(allHeroesWithType.documents, allHeroesSeriesOnly.documents, 'doube-check')
+        // Add a document with extra fields
+        tokusatsu.putDoc(tokusatsuDocumentWizard.contents, tokusatsuDocumentWizard.docId)
+        workaroundQueryAndWait(allHeroesWithType)
+        compare(allHeroesWithType.documents, ['gokaiger', 'ooo', 'wizard'], 'san')
+        workaroundQueryAndWait(allHeroesSeriesOnly)
+        compare(allHeroesWithType.documents, allHeroesSeriesOnly.documents, 'chi')
+        // Add a document with mixed custom fields
+        tokusatsu.putDoc(tokusatsuDocumentDino.contents, tokusatsuDocumentDino.docId)
+        workaroundQueryAndWait(allHeroesWithType)
+        compare(allHeroesWithType.documents, ['dino', 'gokaiger', 'ooo', 'wizard'], 'go')
+        compare(allHeroesWithType.documents, allHeroesSeriesOnly.documents, 'roku')
+    }
 } }