Improved Parameter mapping for query methods (#509)

* (refactor) streamline code generation, prepare for parameter insertion the parameter order had to be altered somewhat, therefore the tests had to be adapted a small bit * (refactor) pull out Query value object to be able to extend it more easily * (feature) Allow for parameter reuse and fix tests * (feature) Improve error messages and test coverage around query parameters * Apply suggestions from code review Co-authored-by: Vitus <[email protected]> * (refactor) address review issues - inline processParameters - improve error message for list<>notlist matching errors * (refactor) fix tests Co-authored-by: Vitus <[email protected]>
pinchbv · Mar 17, 2021 · b0601d8 · b0601d8
1 parent 7cc7b70
commit b0601d8
Show file tree

Hide file tree

Showing 17 changed files with 793 additions and 188 deletions.
diff --git a/floor/test/integration/dao/name_dao.dart b/floor/test/integration/dao/name_dao.dart
@@ -17,6 +17,10 @@ abstract class NameDao {
   @Query('SELECT * FROM names WHERE name LIKE :suffix ORDER BY name ASC')
   Future<List<Name>> findNamesLike(String suffix);
 
+  @Query(
+      'SELECT * FROM names WHERE name LIKE :suffix AND name LIKE :prefix ORDER BY name ASC')
+  Future<List<Name>> findNamesMatchingBoth(String prefix, String suffix);
+
   @Query('SELECT * FROM multiline_query_names WHERE name = :name')
   Future<MultilineQueryName?> findMultilineQueryName(String name);
 }
diff --git a/floor/test/integration/dao/person_dao.dart b/floor/test/integration/dao/person_dao.dart
@@ -26,6 +26,11 @@ abstract class PersonDao {
   @Query('SELECT * FROM person WHERE custom_name IN (:names)')
   Future<List<Person>> findPersonsWithNames(List<String> names);
 
+  @Query(
+      'SELECT * FROM person WHERE custom_name IN (:names) AND id>=:reference OR custom_name IN (:moreNames) AND id<=:reference')
+  Future<List<Person>> findPersonsWithNamesComplex(
+      int reference, List<String> names, List<String> moreNames);
+
   @Query('SELECT * FROM person WHERE custom_name LIKE :name')
   Future<List<Person>> findPersonsWithNamesLike(String name);
 

diff --git a/floor/test/integration/database_test.dart b/floor/test/integration/database_test.dart
@@ -322,6 +322,35 @@ void main() {
 
           expect(actual, equals([person1, person2]));
         });
+
+        test('Find persons with names (complex query)', () async {
+          final person1 = Person(1, 'Sylvie');
+          final person2 = Person(2, 'Simon');
+          final person3 = Person(3, 'Paul');
+          final person4 = Person(4, 'Albert');
+          final person5 = Person(5, 'Louis');
+          final person6 = Person(6, 'Chris');
+          final person7 = Person(7, 'Maria');
+          await personDao.insertPersons(
+              [person1, person2, person3, person4, person5, person6, person7]);
+          final names1 = [
+            person1.name,
+            person3.name,
+            person5.name,
+            person7.name
+          ];
+          final names2 = [
+            person2.name,
+            person4.name,
+            person6.name,
+            person7.name
+          ];
+
+          final actual =
+              await personDao.findPersonsWithNamesComplex(4, names1, names2);
+
+          expect(actual, equals([person5, person7, person4, person2]));
+        });
       });
 
       group('LIKE operator', () {

diff --git a/floor/test/integration/view/view_test.dart b/floor/test/integration/view/view_test.dart
@@ -70,6 +70,19 @@ void main() {
         expect(actual, equals(expected));
       });
 
+      test('query view with double LIKE (reordered query params)', () async {
+        final persons = [Person(1, 'Leo'), Person(2, 'Frank')];
+        await personDao.insertPersons(persons);
+
+        final dog = Dog(1, 'Romeo', 'Rome', 1);
+        await dogDao.insertDog(dog);
+
+        final actual = await nameDao.findNamesMatchingBoth('L%', '%eo');
+
+        final expected = [Name('Leo')];
+        expect(actual, equals(expected));
+      });
+
       test('query view with all values', () async {
         final persons = [Person(1, 'Leo'), Person(2, 'Frank')];
         await personDao.insertPersons(persons);

diff --git a/floor_generator/lib/misc/extension/string_extension.dart b/floor_generator/lib/misc/extension/string_extension.dart
@@ -16,4 +16,40 @@ extension StringExtension on String {
         return this[0].toLowerCase() + substring(1);
     }
   }
+
+  /// Returns a copy of this string having its first letter uppercased, or the
+  /// original string, if it's empty or already starts with a upper case letter.
+  ///
+  /// ```dart
+  /// print('abcd'.capitalize()) // Abcd
+  /// print('Abcd'.capitalize()) // Abcd
+  /// ```
+  String capitalize() {
+    switch (length) {
+      case 0:
+        return this;
+      case 1:
+        return toUpperCase();
+      default:
+        return this[0].toUpperCase() + substring(1);
+    }
+  }
+}
+
+extension NullableStringExtension on String? {
+  /// Converts this string to a literal for
+  /// embedding it into source code strings.
+  ///
+  /// ```dart
+  /// print(null.toLiteral())   // null
+  /// print('Abcd'.toLiteral()) // 'Abcd'
+  /// ```
+  String toLiteral() {
+    if (this == null) {
+      return 'null';
+    } else {
+      //TODO escape correctly
+      return "'$this'";
+    }
+  }
 }
diff --git a/floor_generator/lib/processor/error/query_method_processor_error.dart b/floor_generator/lib/processor/error/query_method_processor_error.dart
@@ -17,14 +17,6 @@ class QueryMethodProcessorError {
     );
   }
 
-  InvalidGenerationSourceError get queryArgumentsAndMethodParametersDoNotMatch {
-    return InvalidGenerationSourceError(
-      'SQL query arguments and method parameters have to match.',
-      todo: 'Make sure to supply one parameter per SQL query argument.',
-      element: _methodElement,
-    );
-  }
-
   InvalidGenerationSourceError get doesNotReturnFutureNorStream {
     return InvalidGenerationSourceError(
       'All queries have to return a Future or Stream.',
@@ -33,18 +25,6 @@ class QueryMethodProcessorError {
     );
   }
 
-  ProcessorError queryMethodParameterIsNullable(
-    final ParameterElement parameterElement,
-  ) {
-    return ProcessorError(
-      message: 'Query method parameters have to be non-nullable.',
-      todo: 'Define ${parameterElement.displayName} as non-nullable.'
-          '\nIf you want to assert null, change your query to use the `IS NULL`/'
-          '`IS NOT NULL` operator without passing a nullable parameter.',
-      element: _methodElement,
-    );
-  }
-
   ProcessorError get doesNotReturnNullableStream {
     return ProcessorError(
       message: 'Queries returning streams of single elements might emit null.',

diff --git a/floor_generator/lib/processor/error/query_processor_error.dart b/floor_generator/lib/processor/error/query_processor_error.dart
@@ -0,0 +1,69 @@
+// ignore_for_file: import_of_legacy_library_into_null_safe
+import 'package:analyzer/dart/element/element.dart';
+import 'package:floor_generator/processor/error/processor_error.dart';
+
+class QueryProcessorError {
+  final MethodElement _methodElement;
+
+  QueryProcessorError(final MethodElement methodElement)
+      : _methodElement = methodElement;
+
+  ProcessorError unusedQueryMethodParameter(
+    final ParameterElement parameterElement,
+  ) {
+    return ProcessorError(
+      message: 'Query method parameters have to be used.',
+      todo: 'Use ${parameterElement.displayName} in the query or remove it.',
+      element: parameterElement,
+    );
+  }
+
+  ProcessorError unknownQueryVariable(
+    final String variableName,
+  ) {
+    return ProcessorError(
+      message:
+          'Query variable `$variableName` has to exist as a method parameter.',
+      todo:
+          'Provide $variableName as a method parameter or remove it from the query.',
+      element: _methodElement,
+    );
+  }
+
+  ProcessorError queryMethodParameterIsListButVariableIsNot(
+    final String varName,
+  ) {
+    final name = varName.substring(1);
+    return ProcessorError(
+      message:
+          'The parameter $name should be referenced like a list (`x IN ($varName)`)',
+      todo: 'Change the type of $name to not be a List<> or'
+          'reference it with ` IN ($varName)` (including the parentheses).',
+      element: _methodElement,
+    );
+  }
+
+  ProcessorError queryMethodParameterIsNormalButVariableIsList(
+    final String varName,
+  ) {
+    final name = varName.substring(1);
+    return ProcessorError(
+      message: 'The parameter $name should be referenced without `IN`',
+      todo: 'Change the type of $name to be a List<> or'
+          'reference it without `IN`, e.g. `IS $varName`.',
+      element: _methodElement,
+    );
+  }
+
+  ProcessorError queryMethodParameterIsNullable(
+    final ParameterElement parameterElement,
+  ) {
+    return ProcessorError(
+      message: 'Query method parameters have to be non-nullable.',
+      todo: 'Define ${parameterElement.displayName} as non-nullable.'
+          '\nIf you want to assert null, change your query to use the `IS NULL`/'
+          '`IS NOT NULL` operator without passing a nullable parameter.',
+      element: parameterElement,
+    );
+  }
+}
diff --git a/floor_generator/lib/processor/query_method_processor.dart b/floor_generator/lib/processor/query_method_processor.dart
@@ -10,6 +10,7 @@ import 'package:floor_generator/misc/extension/type_converter_element_extension.
 import 'package:floor_generator/misc/type_utils.dart';
 import 'package:floor_generator/processor/error/query_method_processor_error.dart';
 import 'package:floor_generator/processor/processor.dart';
+import 'package:floor_generator/processor/query_processor.dart';
 import 'package:floor_generator/value_object/query_method.dart';
 import 'package:floor_generator/value_object/queryable.dart';
 import 'package:floor_generator/value_object/type_converter.dart';
@@ -36,7 +37,9 @@ class QueryMethodProcessor extends Processor<QueryMethod> {
     final parameters = _methodElement.parameters;
     final rawReturnType = _methodElement.returnType;
 
-    final query = _getQuery();
+    final query = QueryProcessor(_methodElement, _getQuery()).process();
+
+    _getQuery();
     final returnsStream = rawReturnType.isStream;
 
     _assertReturnsFutureOrStream(rawReturnType, returnsStream);
@@ -90,29 +93,10 @@ class QueryMethodProcessor extends Processor<QueryMethod> {
         .getAnnotation(annotations.Query)
         .getField(AnnotationField.queryValue)
         ?.toStringValue()
-        ?.replaceAll('\n', ' ')
-        .replaceAll(RegExp(r'[ ]{2,}'), ' ')
-        .trim();
+        ?.trim();
 
     if (query == null || query.isEmpty) throw _processorError.noQueryDefined;
-
-    final substitutedQuery = query.replaceAll(RegExp(r':[.\w]+'), '?');
-    _assertQueryParameters(substitutedQuery, _methodElement.parameters);
-    return _replaceInClauseArguments(substitutedQuery);
-  }
-
-  String _replaceInClauseArguments(final String query) {
-    var index = 0;
-    return query.replaceAllMapped(
-      RegExp(r'( in\s*)\([?]\)', caseSensitive: false),
-      (match) {
-        final matched = match.input.substring(match.start, match.end);
-        final replaced =
-            matched.replaceFirst(RegExp(r'(\?)'), '\$valueList$index');
-        index++;
-        return replaced;
-      },
-    );
+    return query;
   }
 
   DartType _getFlattenedReturnType(
@@ -143,22 +127,6 @@ class QueryMethodProcessor extends Processor<QueryMethod> {
     }
   }
 
-  void _assertQueryParameters(
-    final String query,
-    final List<ParameterElement> parameterElements,
-  ) {
-    for (final parameter in parameterElements) {
-      if (parameter.type.isNullable) {
-        throw _processorError.queryMethodParameterIsNullable(parameter);
-      }
-    }
-
-    final queryParameterCount = RegExp(r'\?').allMatches(query).length;
-    if (queryParameterCount != parameterElements.length) {
-      throw _processorError.queryArgumentsAndMethodParametersDoNotMatch;
-    }
-  }
-
   void _assertReturnsNullableSingle(
     final bool returnsStream,
     final bool returnsList,