From e4a4386abe727134e20a77934dfb15ec8f41eab4 Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Tue, 20 Jun 2017 00:54:33 -0700 Subject: [PATCH 1/6] Firebase database query and persistence improvements --- packages/firebase_database/CHANGELOG.md | 7 + .../database/FirebaseDatabasePlugin.java | 127 +++++++++++++++--- .../firebase_database/example/lib/main.dart | 2 + .../ios/Classes/FirebaseDatabasePlugin.m | 29 +++- .../lib/src/database_reference.dart | 4 +- .../lib/src/firebase_database.dart | 51 ++++--- packages/firebase_database/lib/src/query.dart | 28 ++-- packages/firebase_database/pubspec.yaml | 3 +- .../test/firebase_database_test.dart | 121 +++++++++-------- 9 files changed, 262 insertions(+), 110 deletions(-) diff --git a/packages/firebase_database/CHANGELOG.md b/packages/firebase_database/CHANGELOG.md index 38867a464897..7b8caa2fb300 100644 --- a/packages/firebase_database/CHANGELOG.md +++ b/packages/firebase_database/CHANGELOG.md @@ -1,3 +1,10 @@ +## 0.0.8 + +* Added missing offline persistence and query functionality on Android +* Fixed startAt query behavior on iOS +* Persistence methods no longer throw errors on failure, return false instead +* Updates to docs and tests + ## 0.0.7 * Fixed offline persistence on iOS diff --git a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java index 51f000c3f505..b151ce02be61 100644 --- a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java +++ b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java @@ -8,8 +8,10 @@ import com.google.firebase.database.ChildEventListener; import com.google.firebase.database.DataSnapshot; import com.google.firebase.database.DatabaseError; +import com.google.firebase.database.DatabaseException; import com.google.firebase.database.DatabaseReference; import com.google.firebase.database.FirebaseDatabase; +import com.google.firebase.database.Query; import com.google.firebase.database.ValueEventListener; import io.flutter.plugin.common.MethodCall; import io.flutter.plugin.common.MethodChannel; @@ -31,7 +33,7 @@ public class FirebaseDatabasePlugin implements MethodCallHandler { // Handles are ints used as indexes into the sparse array of active observers private int nextHandle = 0; - private final SparseArray observers = new SparseArray(); + private final SparseArray observers = new SparseArray<>(); public static void registerWith(PluginRegistry.Registrar registrar) { final MethodChannel channel = @@ -43,14 +45,68 @@ private FirebaseDatabasePlugin(MethodChannel channel) { this.channel = channel; } - private static DatabaseReference getReference(Map arguments) { - @SuppressWarnings("unchecked") + private DatabaseReference getReference(Map arguments) { String path = (String) arguments.get("path"); DatabaseReference reference = FirebaseDatabase.getInstance().getReference(); if (path != null) reference = reference.child(path); return reference; } + private Query getQuery(Map arguments) { + Query query = getReference(arguments); + @SuppressWarnings("unchecked") + Map parameters = (Map) arguments.get("parameters"); + Object orderBy = parameters.get("orderBy"); + if ("child".equals(orderBy)) { + query = query.orderByChild((String) parameters.get("orderByChildKey")); + } else if ("key".equals(orderBy)) { + query = query.orderByKey(); + } else if ("value".equals(orderBy)) { + query = query.orderByValue(); + } else if ("priority".equals(orderBy)) { + query = query.orderByPriority(); + } + if (parameters.containsKey("startAt")) { + Object startAt = parameters.get("startAt"); + String startAtKey = (String) parameters.get("startAtKey"); + if (startAt instanceof Boolean) { + query = query.startAt((Boolean) startAt, startAtKey); + } else if (startAt instanceof String) { + query = query.startAt((String) startAt, startAtKey); + } else { + query = query.startAt((Double) startAt, startAtKey); + } + } + if (parameters.containsKey("endAt")) { + Object endAt = parameters.get("endAt"); + String endAtKey = (String) parameters.get("endAtKey"); + if (endAt instanceof Boolean) { + query = query.endAt((Boolean) endAt, endAtKey); + } else if (endAt instanceof String) { + query = query.endAt((String) endAt, endAtKey); + } else { + query = query.endAt((Double) endAt, endAtKey); + } + } + if (arguments.containsKey("equalTo")) { + Object equalTo = arguments.get("equalTo"); + if (equalTo instanceof Boolean) { + query = query.equalTo((Boolean) equalTo); + } else if (equalTo instanceof String) { + query = query.equalTo((String) equalTo); + } else { + query = query.equalTo((Double) equalTo); + } + } + if (arguments.containsKey("limitToFirst")) { + query = query.limitToFirst((int) arguments.get("limitToFirst")); + } + if (arguments.containsKey("limitToLast")) { + query = query.limitToLast((int) arguments.get("limitToLast")); + } + return query; + } + private class DefaultCompletionListener implements DatabaseReference.CompletionListener { private final Result result; @@ -79,8 +135,8 @@ private class EventObserver implements ChildEventListener, ValueEventListener { private void sendEvent(String eventType, DataSnapshot snapshot, String previousChildName) { if (eventType.equals(requestedEventType)) { - Map arguments = new HashMap(); - Map snapshotMap = new HashMap(); + Map arguments = new HashMap<>(); + Map snapshotMap = new HashMap<>(); snapshotMap.put("key", snapshot.getKey()); snapshotMap.put("value", snapshot.getValue()); arguments.put("handle", handle); @@ -119,34 +175,54 @@ public void onDataChange(DataSnapshot snapshot) { } } - @SuppressWarnings("unchecked") @Override public void onMethodCall(MethodCall call, final Result result) { - Map arguments = (Map) call.arguments; + Map arguments = call.arguments(); switch (call.method) { case "FirebaseDatabase#goOnline": - FirebaseDatabase.getInstance().goOnline(); - break; + { + FirebaseDatabase.getInstance().goOnline(); + result.success(null); + break; + } case "FirebaseDatabase#goOffline": - FirebaseDatabase.getInstance().goOffline(); - break; + { + FirebaseDatabase.getInstance().goOffline(); + result.success(null); + break; + } case "FirebaseDatabase#purgeOutstandingWrites": - FirebaseDatabase.getInstance().purgeOutstandingWrites(); - break; + { + FirebaseDatabase.getInstance().purgeOutstandingWrites(); + result.success(null); + break; + } case "FirebaseDatabase#setPersistenceEnabled": { - boolean isEnabled = (boolean) arguments.get("enabled"); - FirebaseDatabase.getInstance().setPersistenceEnabled(isEnabled); + Boolean isEnabled = (Boolean) arguments.get("enabled"); + try { + FirebaseDatabase.getInstance().setPersistenceEnabled(isEnabled); + result.success(true); + } catch (DatabaseException e) { + // Database is already in use, e.g. after hot reload/restart. + result.success(false); + } break; } case "FirebaseDatabase#setPersistenceCacheSizeBytes": { - long cacheSize = (long) arguments.get("cacheSize"); - FirebaseDatabase.getInstance().setPersistenceCacheSizeBytes(cacheSize); + Long cacheSize = (Long) arguments.get("cacheSize"); + try { + FirebaseDatabase.getInstance().setPersistenceCacheSizeBytes(cacheSize); + result.success(true); + } catch (DatabaseException e) { + // Database is already in use, e.g. after hot reload/restart. + result.success(false); + } break; } @@ -171,6 +247,13 @@ public void onMethodCall(MethodCall call, final Result result) { break; } + case "Query#keepSynced": + { + boolean value = (Boolean) arguments.get("value"); + getQuery(arguments).keepSynced(value); + break; + } + case "Query#observe": { String eventType = (String) arguments.get("eventType"); @@ -178,9 +261,9 @@ public void onMethodCall(MethodCall call, final Result result) { EventObserver observer = new EventObserver(eventType, handle); observers.put(handle, observer); if (eventType.equals(EVENT_TYPE_VALUE)) { - getReference(arguments).addValueEventListener(observer); + getQuery(arguments).addValueEventListener(observer); } else { - getReference(arguments).addChildEventListener(observer); + getQuery(arguments).addChildEventListener(observer); } result.success(handle); break; @@ -188,14 +271,14 @@ public void onMethodCall(MethodCall call, final Result result) { case "Query#removeObserver": { - DatabaseReference reference = getReference(arguments); + Query query = getQuery(arguments); int handle = (Integer) arguments.get("handle"); EventObserver observer = observers.get(handle); if (observer != null) { if (observer.requestedEventType.equals(EVENT_TYPE_VALUE)) { - reference.removeEventListener((ValueEventListener) observer); + query.removeEventListener((ValueEventListener) observer); } else { - reference.removeEventListener((ChildEventListener) observer); + query.removeEventListener((ChildEventListener) observer); } observers.delete(handle); result.success(null); diff --git a/packages/firebase_database/example/lib/main.dart b/packages/firebase_database/example/lib/main.dart index 66185a88eeb9..4a7d5fb19dbe 100755 --- a/packages/firebase_database/example/lib/main.dart +++ b/packages/firebase_database/example/lib/main.dart @@ -44,6 +44,8 @@ class _MyHomePageState extends State { @override void initState() { super.initState(); + FirebaseDatabase.instance.setPersistenceEnabled(true); + _counterRef.keepSynced(true); _counterSubscription = _counterRef.onValue.listen((Event event) { setState(() { _counter = event.snapshot.value ?? 0; diff --git a/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m b/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m index 582d8905809e..c84790368479 100644 --- a/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m +++ b/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m @@ -40,7 +40,7 @@ - (FlutterError *)flutterError { } id startAt = parameters[@"startAt"]; if (startAt) { - query = [query queryStartingAtValue:startAt childKey:parameters[@"endAtKey"]]; + query = [query queryStartingAtValue:startAt childKey:parameters[@"startAtKey"]]; } id endAt = parameters[@"endAt"]; if (endAt) { @@ -109,16 +109,39 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result }; if ([@"FirebaseDatabase#goOnline" isEqualToString:call.method]) { [[FIRDatabase database] goOnline]; + result(nil); } else if ([@"FirebaseDatabase#goOffline" isEqualToString:call.method]) { [[FIRDatabase database] goOffline]; + result(nil); } else if ([@"FirebaseDatabase#purgeOutstandingWrites" isEqualToString:call.method]) { [[FIRDatabase database] purgeOutstandingWrites]; + result(nil); } else if ([@"FirebaseDatabase#setPersistenceEnabled" isEqualToString:call.method]) { NSNumber *value = call.arguments[@"enabled"]; - [FIRDatabase database].persistenceEnabled = value.boolValue; + @try { + [FIRDatabase database].persistenceEnabled = value.boolValue; + result([NSNumber numberWithBool:YES]); + } @catch (NSException *exception) { + if ([@"FIRDatabaseAlreadyInUse" isEqualToString:exception.name]) { + // Database is already in use, e.g. after hot reload/restart. + result([NSNumber numberWithBool:NO]); + } else { + @throw; + } + } } else if ([@"FirebaseDatabase#setPersistenceCacheSizeBytes" isEqualToString:call.method]) { NSNumber *value = call.arguments[@"cacheSize"]; - [FIRDatabase database].persistenceCacheSizeBytes = value.unsignedIntegerValue; + @try { + [FIRDatabase database].persistenceCacheSizeBytes = value.unsignedIntegerValue; + result([NSNumber numberWithBool:YES]); + } @catch (NSException *exception) { + if ([@"FIRDatabaseAlreadyInUse" isEqualToString:exception.name]) { + // Database is already in use, e.g. after hot reload/restart. + result([NSNumber numberWithBool:NO]); + } else { + @throw; + } + } } else if ([@"DatabaseReference#set" isEqualToString:call.method]) { [getReference(call.arguments) setValue:call.arguments[@"value"] andPriority:call.arguments[@"priority"] diff --git a/packages/firebase_database/lib/src/database_reference.dart b/packages/firebase_database/lib/src/database_reference.dart index 75ed7a807a2a..2a3ece39117c 100644 --- a/packages/firebase_database/lib/src/database_reference.dart +++ b/packages/firebase_database/lib/src/database_reference.dart @@ -97,10 +97,10 @@ class DatabaseReference extends Query { /// priority (including no priority), they are sorted by key. Numeric keys /// come first (sorted numerically), followed by the remaining keys (sorted /// lexicographically). - + /// /// Note that priorities are parsed and ordered as IEEE 754 double-precision /// floating-point numbers. Keys are always stored as strings and are treated - /// as numbers only when they can be parsed as a 32-bit integer + /// as numbers only when they can be parsed as a 32-bit integer. Future setPriority(dynamic priority) async { return _database._channel.invokeMethod( 'DatabaseReference#setPriority', diff --git a/packages/firebase_database/lib/src/firebase_database.dart b/packages/firebase_database/lib/src/firebase_database.dart index d2ce9307b6a2..864da3e60c06 100644 --- a/packages/firebase_database/lib/src/firebase_database.dart +++ b/packages/firebase_database/lib/src/firebase_database.dart @@ -32,52 +32,65 @@ class FirebaseDatabase { /// Gets a DatabaseReference for the root of your Firebase Database. DatabaseReference reference() => new DatabaseReference._(this, []); + /// Attempts to sets the database persistence to [enabled]. + /// + /// This property must be set before calling methods on database references + /// and only needs to be called once per application. The returned `Future` + /// will complete with `true` if the operation was successful or `false` if + /// the persistence could not be set (because database references have + /// already been created). + /// /// The Firebase Database client will cache synchronized data and keep track /// of all writes you’ve initiated while your application is running. It /// seamlessly handles intermittent network connections and re-sends write /// operations when the network connection is restored. /// /// However by default your write operations and cached data are only stored - /// in-memory and will be lost when your app restarts. By setting this value - /// to YES, the data will be persisted to on-device (disk) storage and will + /// in-memory and will be lost when your app restarts. By setting [enabled] + /// to `true`, the data will be persisted to on-device (disk) storage and will /// thus be available again when the app is restarted (even when there is no - /// network connectivity at that time). Note that this property must be set - /// before creating your first Database reference and only needs to be called - /// once per application. - Future setPersistenceEnabled(bool enabled) { + /// network connectivity at that time). + Future setPersistenceEnabled(bool enabled) { return _channel.invokeMethod( - "FirebaseDatabase#setPersistenceEnabled", - {'enabled': enabled}, + 'FirebaseDatabase#setPersistenceEnabled', + {'enabled': enabled}, ); } + /// Attempts to set the size of the persistence cache. + /// /// By default the Firebase Database client will use up to 10MB of disk space /// to cache data. If the cache grows beyond this size, the client will start /// removing data that hasn’t been recently used. If you find that your /// application caches too little or too much data, call this method to change - /// the cache size. This property must be set before creating your first - /// FIRDatabaseReference and only needs to be called once per application. + /// the cache size. + /// + /// This property must be set before calling methods on database references + /// and only needs to be called once per application. The returned `Future` + /// will complete with `true` if the operation was successful or `false` if + /// the value could not be set (because database references have already been + /// created). /// /// Note that the specified cache size is only an approximation and the size /// on disk may temporarily exceed it at times. Cache sizes smaller than 1 MB /// or greater than 100 MB are not supported. - Future setPersistenceCacheSizeBytes(int cacheSize) { + Future setPersistenceCacheSizeBytes(int cacheSize) { return _channel.invokeMethod( - "FirebaseDatabase#setPersistenceCacheSizeBytes", - {'cacheSize': cacheSize}, + 'FirebaseDatabase#setPersistenceCacheSizeBytes', + {'cacheSize': cacheSize}, ); } /// Resumes our connection to the Firebase Database backend after a previous - /// goOffline call. + /// [goOffline] call. Future goOnline() { - return _channel.invokeMethod("FirebaseDatabase#goOnline"); + return _channel.invokeMethod('FirebaseDatabase#goOnline'); } - /// Shuts down our connection to the Firebase Database backend until goOnline - /// is called. + /// Shuts down our connection to the Firebase Database backend until + /// [goOnline] is called. Future goOffline() { - return _channel.invokeMethod("FirebaseDatabase#goOffline"); + return _channel.invokeMethod('FirebaseDatabase#goOffline'); } /// The Firebase Database client automatically queues writes and sends them to @@ -91,6 +104,6 @@ class FirebaseDatabase { /// affected event listeners, and the client will not (re-)send them to the /// Firebase Database backend. Future purgeOutstandingWrites() { - return _channel.invokeMethod("FirebaseDatabase#purgeOutstandingWrites"); + return _channel.invokeMethod('FirebaseDatabase#purgeOutstandingWrites'); } } diff --git a/packages/firebase_database/lib/src/query.dart b/packages/firebase_database/lib/src/query.dart index 94eb0d28149b..4e1e9be3ffa2 100644 --- a/packages/firebase_database/lib/src/query.dart +++ b/packages/firebase_database/lib/src/query.dart @@ -93,7 +93,9 @@ class Query { /// than or equal to the given key. Query startAt(dynamic value, { String key }) { assert(!_parameters.containsKey('startAt')); - return _copyWithParameters({ 'startAt': value, 'startAtKey': key}); + return _copyWithParameters( + { 'startAt': value, 'startAtKey': key }, + ); } /// Create a query constrained to only return child nodes with a value less @@ -102,7 +104,9 @@ class Query { /// than or equal to the given key. Query endAt(dynamic value, { String key }) { assert(!_parameters.containsKey('endAt')); - return _copyWithParameters({ 'endAt': value, 'endAtKey': key}); + return _copyWithParameters( + { 'endAt': value, 'endAtKey': key }, + ); } /// Create a query constrained to only return child nodes with the given @@ -111,7 +115,9 @@ class Query { /// If a key is provided, there is at most one such child as names are unique. Query equalTo(dynamic value, { String key }) { assert(!_parameters.containsKey('equalTo')); - return _copyWithParameters({ 'equalTo': value, 'equalToKey': key }); + return _copyWithParameters( + { 'equalTo': value, 'equalToKey': key }, + ); } /// Create a query with limit and anchor it to the start of the window. @@ -128,16 +134,20 @@ class Query { /// Generate a view of the data sorted by values of a particular child key. /// - /// Intended to be used in combination with startAt(), endAt(), or equalTo(). + /// Intended to be used in combination with [startAt()], [endAt()], or + /// [equalTo()]. Query orderByChild(String key) { assert(key != null); assert(!_parameters.containsKey('orderBy')); - return _copyWithParameters({ 'orderBy': 'child', 'orderByChildKey': key }); + return _copyWithParameters( + { 'orderBy': 'child', 'orderByChildKey': key }, + ); } /// Generate a view of the data sorted by key. /// - /// Intended to be used in combination with startAt(), endAt(), or equalTo(). + /// Intended to be used in combination with [startAt()], [endAt()], or + /// [equalTo()]. Query orderByKey() { assert(!_parameters.containsKey('orderBy')); return _copyWithParameters({ 'orderBy': 'key' }); @@ -145,7 +155,8 @@ class Query { /// Generate a view of the data sorted by value. /// - /// Intended to be used in combination with startAt(), endAt(), or equalTo(). + /// Intended to be used in combination with [startAt()], [endAt()], or + /// [equalTo()]. Query orderByValue() { assert(!_parameters.containsKey('orderBy')); return _copyWithParameters({ 'orderBy': 'value' }); @@ -153,7 +164,8 @@ class Query { /// Generate a view of the data sorted by priority. /// - /// Intended to be used in combination with startAt(), endAt(), or equalTo(). + /// Intended to be used in combination with [startAt()], [endAt()], or + /// [equalTo()]. Query orderByPriority() { assert(!_parameters.containsKey('orderBy')); return _copyWithParameters({ 'orderBy': 'priority' }); diff --git a/packages/firebase_database/pubspec.yaml b/packages/firebase_database/pubspec.yaml index 593f34becdf9..dad3c9aafc47 100755 --- a/packages/firebase_database/pubspec.yaml +++ b/packages/firebase_database/pubspec.yaml @@ -3,7 +3,7 @@ name: firebase_database description: Firebase Database plugin for Flutter. author: Flutter Team homepage: https://github.com/flutter/plugins/tree/master/packages/firebase_database -version: 0.0.7 +version: 0.0.8 flutter: plugin: @@ -16,7 +16,6 @@ dependencies: meta: "^1.0.5" dev_dependencies: - mockito: ^2.0.2 test: 0.12.21 environment: diff --git a/packages/firebase_database/test/firebase_database_test.dart b/packages/firebase_database/test/firebase_database_test.dart index 6a5d5a7856fb..2eac0a47819f 100755 --- a/packages/firebase_database/test/firebase_database_test.dart +++ b/packages/firebase_database/test/firebase_database_test.dart @@ -4,7 +4,6 @@ import 'dart:async'; -import 'package:mockito/mockito.dart'; import 'package:test/test.dart'; import 'package:flutter/services.dart'; @@ -24,64 +23,79 @@ void main() { setUp(() async { channel.setMockMethodCallHandler((MethodCall methodCall) async { log.add(methodCall); - if (methodCall.method == 'Query#observe') { - return mockHandleId++; + switch (methodCall.method) { + case 'Query#observe': + return mockHandleId++; + case 'FirebaseDatabase#setPersistenceEnabled': + return true; + case 'FirebaseDatabase#setPersistenceCacheSizeBytes': + return true; + default: + return null; } - return null; }); log.clear(); }); test('setPersistenceEnabled', () async { - await database.setPersistenceEnabled(false); - await database.setPersistenceEnabled(true); + expect(await database.setPersistenceEnabled(false), true); + expect(await database.setPersistenceEnabled(true), true); expect( - log, - equals([ - new MethodCall('FirebaseDatabase#setPersistenceEnabled', - {'enabled': false}), - new MethodCall('FirebaseDatabase#setPersistenceEnabled', - {'enabled': true}), - ])); + log, + equals([ + new MethodCall( + 'FirebaseDatabase#setPersistenceEnabled', + {'enabled': false}, + ), + new MethodCall( + 'FirebaseDatabase#setPersistenceEnabled', + {'enabled': true}, + ), + ]), + ); }); test('setPersistentCacheSizeBytes', () async { - await database.setPersistenceCacheSizeBytes(42); + expect(await database.setPersistenceCacheSizeBytes(42), true); expect( - log, - equals([ - new MethodCall( - 'FirebaseDatabase#setPersistenceCacheSizeBytes', - {'cacheSize': 42}, - ), - ])); + log, + equals([ + new MethodCall( + 'FirebaseDatabase#setPersistenceCacheSizeBytes', + {'cacheSize': 42}, + ), + ]), + ); }); test('goOnline', () async { await database.goOnline(); expect( - log, - equals([ - const MethodCall('FirebaseDatabase#goOnline'), - ])); + log, + equals([ + const MethodCall('FirebaseDatabase#goOnline'), + ]), + ); }); test('goOffline', () async { await database.goOffline(); expect( - log, - equals([ - const MethodCall('FirebaseDatabase#goOffline'), - ])); + log, + equals([ + const MethodCall('FirebaseDatabase#goOffline'), + ]), + ); }); test('purgeOutstandingWrites', () async { await database.purgeOutstandingWrites(); expect( - log, - equals([ - const MethodCall('FirebaseDatabase#purgeOutstandingWrites'), - ])); + log, + equals([ + const MethodCall('FirebaseDatabase#purgeOutstandingWrites'), + ]), + ); }); group('$DatabaseReference', () { @@ -91,25 +105,26 @@ void main() { await database.reference().child('foo').set(value); await database.reference().child('bar').set(value, priority: priority); expect( - log, - equals([ - new MethodCall( - 'DatabaseReference#set', - { - 'path': 'foo', - 'value': value, - 'priority': null - }, - ), - new MethodCall( - 'DatabaseReference#set', - { - 'path': 'bar', - 'value': value, - 'priority': priority - }, - ), - ])); + log, + equals([ + new MethodCall( + 'DatabaseReference#set', + { + 'path': 'foo', + 'value': value, + 'priority': null + }, + ), + new MethodCall( + 'DatabaseReference#set', + { + 'path': 'bar', + 'value': value, + 'priority': priority + }, + ), + ]), + ); }); test('setPriority', () async { @@ -163,5 +178,3 @@ void main() { }); }); } - -class MockPlatformChannel extends Mock implements MethodChannel {} From 72448427eace344e6ad14f5f4f0d9003a55efdd8 Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Tue, 20 Jun 2017 07:59:41 -0700 Subject: [PATCH 2/6] Simple example of a limited query in the example app --- packages/firebase_database/example/lib/main.dart | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/packages/firebase_database/example/lib/main.dart b/packages/firebase_database/example/lib/main.dart index 4a7d5fb19dbe..ef664808cb9f 100755 --- a/packages/firebase_database/example/lib/main.dart +++ b/packages/firebase_database/example/lib/main.dart @@ -51,7 +51,9 @@ class _MyHomePageState extends State { _counter = event.snapshot.value ?? 0; }); }); - _messagesSubscription = _messagesRef.onChildAdded.listen((Event event) { + _messagesSubscription = _messagesRef + .limitToLast(10) + .onChildAdded.listen((Event event) { print('Child added: ${event.snapshot.value}'); }); } From e799f30e8717fff735fd0dd4015ccdd7daa898b5 Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Tue, 20 Jun 2017 08:37:44 -0700 Subject: [PATCH 3/6] Review feedback --- .../lib/src/firebase_database.dart | 4 ++-- packages/firebase_database/lib/src/query.dart | 16 ++++++++-------- 2 files changed, 10 insertions(+), 10 deletions(-) diff --git a/packages/firebase_database/lib/src/firebase_database.dart b/packages/firebase_database/lib/src/firebase_database.dart index 864da3e60c06..2efa52bb5670 100644 --- a/packages/firebase_database/lib/src/firebase_database.dart +++ b/packages/firebase_database/lib/src/firebase_database.dart @@ -35,7 +35,7 @@ class FirebaseDatabase { /// Attempts to sets the database persistence to [enabled]. /// /// This property must be set before calling methods on database references - /// and only needs to be called once per application. The returned `Future` + /// and only needs to be called once per application. The returned [Future] /// will complete with `true` if the operation was successful or `false` if /// the persistence could not be set (because database references have /// already been created). @@ -66,7 +66,7 @@ class FirebaseDatabase { /// the cache size. /// /// This property must be set before calling methods on database references - /// and only needs to be called once per application. The returned `Future` + /// and only needs to be called once per application. The returned [Future] /// will complete with `true` if the operation was successful or `false` if /// the value could not be set (because database references have already been /// created). diff --git a/packages/firebase_database/lib/src/query.dart b/packages/firebase_database/lib/src/query.dart index 4e1e9be3ffa2..393c21f50b20 100644 --- a/packages/firebase_database/lib/src/query.dart +++ b/packages/firebase_database/lib/src/query.dart @@ -134,8 +134,8 @@ class Query { /// Generate a view of the data sorted by values of a particular child key. /// - /// Intended to be used in combination with [startAt()], [endAt()], or - /// [equalTo()]. + /// Intended to be used in combination with [startAt], [endAt], or + /// [equalTo]. Query orderByChild(String key) { assert(key != null); assert(!_parameters.containsKey('orderBy')); @@ -146,8 +146,8 @@ class Query { /// Generate a view of the data sorted by key. /// - /// Intended to be used in combination with [startAt()], [endAt()], or - /// [equalTo()]. + /// Intended to be used in combination with [startAt], [endAt], or + /// [equalTo]. Query orderByKey() { assert(!_parameters.containsKey('orderBy')); return _copyWithParameters({ 'orderBy': 'key' }); @@ -155,8 +155,8 @@ class Query { /// Generate a view of the data sorted by value. /// - /// Intended to be used in combination with [startAt()], [endAt()], or - /// [equalTo()]. + /// Intended to be used in combination with [startAt], [endAt], or + /// [equalTo]. Query orderByValue() { assert(!_parameters.containsKey('orderBy')); return _copyWithParameters({ 'orderBy': 'value' }); @@ -164,8 +164,8 @@ class Query { /// Generate a view of the data sorted by priority. /// - /// Intended to be used in combination with [startAt()], [endAt()], or - /// [equalTo()]. + /// Intended to be used in combination with [startAt], [endAt], or + /// [equalTo]. Query orderByPriority() { assert(!_parameters.containsKey('orderBy')); return _copyWithParameters({ 'orderBy': 'priority' }); From 2c85344312ba704612db00337092cad91f207aaa Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Tue, 20 Jun 2017 08:44:38 -0700 Subject: [PATCH 4/6] Bug fixes to Android implementation --- .../database/FirebaseDatabasePlugin.java | 30 ++++++++++++------- packages/firebase_database/lib/src/query.dart | 3 ++ 2 files changed, 22 insertions(+), 11 deletions(-) diff --git a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java index b151ce02be61..6414049c3d9b 100644 --- a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java +++ b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java @@ -56,6 +56,8 @@ private Query getQuery(Map arguments) { Query query = getReference(arguments); @SuppressWarnings("unchecked") Map parameters = (Map) arguments.get("parameters"); + if (parameters == null) + return query; Object orderBy = parameters.get("orderBy"); if ("child".equals(orderBy)) { query = query.orderByChild((String) parameters.get("orderByChildKey")); @@ -73,8 +75,10 @@ private Query getQuery(Map arguments) { query = query.startAt((Boolean) startAt, startAtKey); } else if (startAt instanceof String) { query = query.startAt((String) startAt, startAtKey); - } else { - query = query.startAt((Double) startAt, startAtKey); + } else if (startAt instanceof Double) { + query = query.endAt((Double) startAt); + } else if (startAt instanceof Integer) { + query = query.startAt((int) startAt); } } if (parameters.containsKey("endAt")) { @@ -84,25 +88,29 @@ private Query getQuery(Map arguments) { query = query.endAt((Boolean) endAt, endAtKey); } else if (endAt instanceof String) { query = query.endAt((String) endAt, endAtKey); - } else { - query = query.endAt((Double) endAt, endAtKey); + } else if (endAt instanceof Double) { + query = query.endAt((Double) endAt); + } else if (endAt instanceof Integer) { + query = query.endAt((int) endAt); } } - if (arguments.containsKey("equalTo")) { - Object equalTo = arguments.get("equalTo"); + if (parameters.containsKey("equalTo")) { + Object equalTo = parameters.get("equalTo"); if (equalTo instanceof Boolean) { query = query.equalTo((Boolean) equalTo); } else if (equalTo instanceof String) { query = query.equalTo((String) equalTo); - } else { + } else if (equalTo instanceof Double) { query = query.equalTo((Double) equalTo); + } else if (equalTo instanceof Integer) { + query = query.equalTo((int) equalTo); } } - if (arguments.containsKey("limitToFirst")) { - query = query.limitToFirst((int) arguments.get("limitToFirst")); + if (parameters.containsKey("limitToFirst")) { + query = query.limitToFirst((int) parameters.get("limitToFirst")); } - if (arguments.containsKey("limitToLast")) { - query = query.limitToLast((int) arguments.get("limitToLast")); + if (parameters.containsKey("limitToLast")) { + query = query.limitToLast((int) parameters.get("limitToLast")); } return query; } diff --git a/packages/firebase_database/lib/src/query.dart b/packages/firebase_database/lib/src/query.dart index 393c21f50b20..9ee9284b98b1 100644 --- a/packages/firebase_database/lib/src/query.dart +++ b/packages/firebase_database/lib/src/query.dart @@ -93,6 +93,7 @@ class Query { /// than or equal to the given key. Query startAt(dynamic value, { String key }) { assert(!_parameters.containsKey('startAt')); + assert(value is String || value is bool || value is double || value is int); return _copyWithParameters( { 'startAt': value, 'startAtKey': key }, ); @@ -104,6 +105,7 @@ class Query { /// than or equal to the given key. Query endAt(dynamic value, { String key }) { assert(!_parameters.containsKey('endAt')); + assert(value is String || value is bool || value is double || value is int); return _copyWithParameters( { 'endAt': value, 'endAtKey': key }, ); @@ -115,6 +117,7 @@ class Query { /// If a key is provided, there is at most one such child as names are unique. Query equalTo(dynamic value, { String key }) { assert(!_parameters.containsKey('equalTo')); + assert(value is String || value is bool || value is double || value is int); return _copyWithParameters( { 'equalTo': value, 'equalToKey': key }, ); From 54994eb2d7d5e83c3de9659661d574c95eaf03b7 Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Tue, 20 Jun 2017 09:13:33 -0700 Subject: [PATCH 5/6] Reformat --- .../plugins/firebase/database/FirebaseDatabasePlugin.java | 3 +-- packages/firebase_database/example/lib/main.dart | 5 ++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java index 6414049c3d9b..01e2acd2ea7d 100644 --- a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java +++ b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java @@ -56,8 +56,7 @@ private Query getQuery(Map arguments) { Query query = getReference(arguments); @SuppressWarnings("unchecked") Map parameters = (Map) arguments.get("parameters"); - if (parameters == null) - return query; + if (parameters == null) return query; Object orderBy = parameters.get("orderBy"); if ("child".equals(orderBy)) { query = query.orderByChild((String) parameters.get("orderByChildKey")); diff --git a/packages/firebase_database/example/lib/main.dart b/packages/firebase_database/example/lib/main.dart index ef664808cb9f..8278f16da0e5 100755 --- a/packages/firebase_database/example/lib/main.dart +++ b/packages/firebase_database/example/lib/main.dart @@ -51,9 +51,8 @@ class _MyHomePageState extends State { _counter = event.snapshot.value ?? 0; }); }); - _messagesSubscription = _messagesRef - .limitToLast(10) - .onChildAdded.listen((Event event) { + _messagesSubscription = + _messagesRef.limitToLast(10).onChildAdded.listen((Event event) { print('Child added: ${event.snapshot.value}'); }); } From c1c321b3c3fcf98d55ce522c52ea1a640981aee9 Mon Sep 17 00:00:00 2001 From: Collin Jackson Date: Tue, 20 Jun 2017 09:30:36 -0700 Subject: [PATCH 6/6] mravn code review feedback --- .../firebase/database/FirebaseDatabasePlugin.java | 10 +++++++--- packages/firebase_database/example/lib/main.dart | 1 + .../ios/Classes/FirebaseDatabasePlugin.m | 4 ++-- .../lib/src/firebase_database.dart | 4 ++-- .../test/firebase_database_test.dart | 15 +++------------ 5 files changed, 15 insertions(+), 19 deletions(-) diff --git a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java index 01e2acd2ea7d..e7ff45988b24 100644 --- a/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java +++ b/packages/firebase_database/android/src/main/java/io/flutter/plugins/firebase/database/FirebaseDatabasePlugin.java @@ -184,7 +184,6 @@ public void onDataChange(DataSnapshot snapshot) { @Override public void onMethodCall(MethodCall call, final Result result) { - Map arguments = call.arguments(); switch (call.method) { case "FirebaseDatabase#goOnline": { @@ -209,7 +208,7 @@ public void onMethodCall(MethodCall call, final Result result) { case "FirebaseDatabase#setPersistenceEnabled": { - Boolean isEnabled = (Boolean) arguments.get("enabled"); + Boolean isEnabled = (Boolean) call.arguments; try { FirebaseDatabase.getInstance().setPersistenceEnabled(isEnabled); result.success(true); @@ -222,7 +221,7 @@ public void onMethodCall(MethodCall call, final Result result) { case "FirebaseDatabase#setPersistenceCacheSizeBytes": { - Long cacheSize = (Long) arguments.get("cacheSize"); + long cacheSize = (Integer) call.arguments; try { FirebaseDatabase.getInstance().setPersistenceCacheSizeBytes(cacheSize); result.success(true); @@ -235,6 +234,7 @@ public void onMethodCall(MethodCall call, final Result result) { case "DatabaseReference#set": { + Map arguments = call.arguments(); Object value = arguments.get("value"); Object priority = arguments.get("priority"); DatabaseReference reference = getReference(arguments); @@ -248,6 +248,7 @@ public void onMethodCall(MethodCall call, final Result result) { case "DatabaseReference#setPriority": { + Map arguments = call.arguments(); Object priority = arguments.get("priority"); DatabaseReference reference = getReference(arguments); reference.setPriority(priority, new DefaultCompletionListener(result)); @@ -256,6 +257,7 @@ public void onMethodCall(MethodCall call, final Result result) { case "Query#keepSynced": { + Map arguments = call.arguments(); boolean value = (Boolean) arguments.get("value"); getQuery(arguments).keepSynced(value); break; @@ -263,6 +265,7 @@ public void onMethodCall(MethodCall call, final Result result) { case "Query#observe": { + Map arguments = call.arguments(); String eventType = (String) arguments.get("eventType"); int handle = nextHandle++; EventObserver observer = new EventObserver(eventType, handle); @@ -278,6 +281,7 @@ public void onMethodCall(MethodCall call, final Result result) { case "Query#removeObserver": { + Map arguments = call.arguments(); Query query = getQuery(arguments); int handle = (Integer) arguments.get("handle"); EventObserver observer = observers.get(handle); diff --git a/packages/firebase_database/example/lib/main.dart b/packages/firebase_database/example/lib/main.dart index 8278f16da0e5..74e14af52d63 100755 --- a/packages/firebase_database/example/lib/main.dart +++ b/packages/firebase_database/example/lib/main.dart @@ -45,6 +45,7 @@ class _MyHomePageState extends State { void initState() { super.initState(); FirebaseDatabase.instance.setPersistenceEnabled(true); + FirebaseDatabase.instance.setPersistenceCacheSizeBytes(10000000); _counterRef.keepSynced(true); _counterSubscription = _counterRef.onValue.listen((Event event) { setState(() { diff --git a/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m b/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m index c84790368479..4e40b8c93bab 100644 --- a/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m +++ b/packages/firebase_database/ios/Classes/FirebaseDatabasePlugin.m @@ -117,7 +117,7 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result [[FIRDatabase database] purgeOutstandingWrites]; result(nil); } else if ([@"FirebaseDatabase#setPersistenceEnabled" isEqualToString:call.method]) { - NSNumber *value = call.arguments[@"enabled"]; + NSNumber *value = call.arguments; @try { [FIRDatabase database].persistenceEnabled = value.boolValue; result([NSNumber numberWithBool:YES]); @@ -130,7 +130,7 @@ - (void)handleMethodCall:(FlutterMethodCall *)call result:(FlutterResult)result } } } else if ([@"FirebaseDatabase#setPersistenceCacheSizeBytes" isEqualToString:call.method]) { - NSNumber *value = call.arguments[@"cacheSize"]; + NSNumber *value = call.arguments; @try { [FIRDatabase database].persistenceCacheSizeBytes = value.unsignedIntegerValue; result([NSNumber numberWithBool:YES]); diff --git a/packages/firebase_database/lib/src/firebase_database.dart b/packages/firebase_database/lib/src/firebase_database.dart index 2efa52bb5670..678f0dd59227 100644 --- a/packages/firebase_database/lib/src/firebase_database.dart +++ b/packages/firebase_database/lib/src/firebase_database.dart @@ -53,7 +53,7 @@ class FirebaseDatabase { Future setPersistenceEnabled(bool enabled) { return _channel.invokeMethod( 'FirebaseDatabase#setPersistenceEnabled', - {'enabled': enabled}, + enabled, ); } @@ -77,7 +77,7 @@ class FirebaseDatabase { Future setPersistenceCacheSizeBytes(int cacheSize) { return _channel.invokeMethod( 'FirebaseDatabase#setPersistenceCacheSizeBytes', - {'cacheSize': cacheSize}, + cacheSize, ); } diff --git a/packages/firebase_database/test/firebase_database_test.dart b/packages/firebase_database/test/firebase_database_test.dart index 2eac0a47819f..f4397b353942 100755 --- a/packages/firebase_database/test/firebase_database_test.dart +++ b/packages/firebase_database/test/firebase_database_test.dart @@ -43,14 +43,8 @@ void main() { expect( log, equals([ - new MethodCall( - 'FirebaseDatabase#setPersistenceEnabled', - {'enabled': false}, - ), - new MethodCall( - 'FirebaseDatabase#setPersistenceEnabled', - {'enabled': true}, - ), + const MethodCall('FirebaseDatabase#setPersistenceEnabled', false), + const MethodCall('FirebaseDatabase#setPersistenceEnabled', true), ]), ); }); @@ -60,10 +54,7 @@ void main() { expect( log, equals([ - new MethodCall( - 'FirebaseDatabase#setPersistenceCacheSizeBytes', - {'cacheSize': 42}, - ), + const MethodCall('FirebaseDatabase#setPersistenceCacheSizeBytes', 42), ]), ); });