Add optional callback functions when opening database #182

noordawod · 2019-08-27T13:13:35Z

This is backward-compatible.

…tible).

dkaera · 2019-08-27T13:38:44Z

Please, add floor_generator/lib/generated/i18n.dart (126) to .gitignore . It's an autogenerated file.

noordawod · 2019-08-27T13:42:40Z

Please, add floor_generator/lib/generated/i18n.dart (126) to .gitignore . It's an autogenerated file.

Done.

noordawod · 2019-08-27T13:48:14Z

Travis seems to be down or something...

noordawod · 2019-08-28T05:47:45Z

The command "./tool/travis.sh dartfmt" exited with 1.

Travis is not feeling well :/ but the code now passes tests if you do it offline: pub run test

dkaera · 2019-08-28T06:12:59Z

The command "./tool/travis.sh dartfmt" exited with 1.

Travis is not feeling well :/ but the code now passes tests if you do it offline: pub run test

As I see in CI log, you need to run flutter format ./floor_generator/ or navigate to floor_generator directory and run flutter format . from Terminal to apply Dart format for floor_generator module.

noordawod · 2019-08-28T06:32:42Z

The command "./tool/travis.sh dartfmt" exited with 1.
Travis is not feeling well :/ but the code now passes tests if you do it offline: pub run test

As I see in CI log, you need to run flutter format ./floor_generator/ or navigate to floor_generator directory and run flutter format . from Terminal to apply Dart format for floor_generator module.

Ugh, thanks for the tip. Retrying...

noordawod · 2019-08-28T11:15:46Z

Travis stopped?

travis-ci/travis-ci#10204

codecov · 2019-08-28T21:55:23Z

Codecov Report

Merging #182 into develop will increase coverage by 0.24%.
The diff coverage is 100%.

@@             Coverage Diff             @@
##           develop     #182      +/-   ##
===========================================
+ Coverage    77.52%   77.76%   +0.24%     
===========================================
  Files           49       49              
  Lines         1366     1381      +15     
===========================================
+ Hits          1059     1074      +15     
  Misses         307      307

Impacted Files	Coverage Δ
floor_generator/lib/writer/database_writer.dart	`79.41% <100%> (+1.28%)`	⬆️
..._generator/lib/writer/database_builder_writer.dart	`100% <100%> (ø)`	⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update cc331e4...ef0a589. Read the comment docs.

floor_generator/lib/writer/database_writer.dart

vitusortner · 2019-08-28T22:25:59Z

Hi @noordawod thanks for your contribution! What's the reasoning for allowing these additional functions? I see some problems with opening the possibility of hooking into the migration (onUpgrade) and table creation (onCreate) process.

Co-Authored-By: Vitus <[email protected]>

noordawod · 2019-08-29T07:14:31Z

Hi @noordawod thanks for your contribution! What's the reasoning for allowing these additional functions? I see some problems with opening the possibility of hooking into the migration (onUpgrade) and table creation (onCreate) process.

Hi @vitusortner.

My use case: I needed to know when the database is first created (so show an onboarding screen), then when an upgrade is going to happen (show a progress indicator), and when the database is up to speed -- and then rush the user to the main screen. I see many people wanting this use case, but maybe it's just me :)

About the callbacks: exposing the database instance could potentially be a problem if it isn't exposed in the first place, but I believe it is. If you do think this may cause trouble, then maybe just allow callbacks without the database instance? (new typedefs)

Let me know what you think.

PS: I wonder what problems do you see with this?

noordawod · 2019-09-01T08:50:41Z

@vitusortner

Would you like having callbacks without access to the SQLite database? I can do this modification if it fits better with the architecture you want.

vitusortner · 2019-09-01T10:11:04Z

I gave it a second thought and now think, that there is completely valid use cases for exposing the database.

Implementation

I would prefer passing the callbacks via the generated DatabaseBuilder (similar to how migrations are forwarded) instead of using the DatabaseBuilder.build function. An example usage is shown in the following snippet.

The callback parameters onCreate, onOpen, onUpgrade should be named arguments.

final callback = Callback(
   onCreate: (database, version) {},
   onOpen: (database) {},
   onUpgrade: (database, startVersion, endVersion) {},
);

final database = await $FloorTestDatabase
    .addCallback(callback)
    .build();

Callback could be as simple as the following and should be located at floor/lib/src/callback.dart.

class Callback {
  final FutureOr<void> Function(Database database, int version) onCreate;
  final FutureOr<void> Function(Database database) onOpen;
  final FutureOr<void> Function(
    Database database,
    int startVersion,
    int endVersion,
  ) onUpgrade;

  Callback({this.onCreate, this.onOpen, this.onUpgrade});
}

Using FutureOr opens up the possibility of either using Dart's asynchronous capabilities in case they are required or to just execute synchronous code.

What made you decide on executing the passed functions before the migrations and table creations happen?

noordawod · 2019-09-01T10:38:07Z

@vitusortner

This should work nicely too! What's your eta for this?

The decision stems from old habbits (from the JavaScript/jQuery world): the handler may return a false to cancel an event (click for example). But in our use case, it could be beneficial that these callbacks happen after Floor have done its work. Up to you.

vitusortner · 2019-09-01T10:43:28Z

If you'd find time for the implementation, I'd welcome your contribution! Based on the changes you've done, you know which places of the code base to touch.

Let's try with putting the callbacks after the initialization has happened.

noordawod · 2019-09-01T11:01:08Z

On it. I'll push to this pr.

noordawod · 2019-09-01T12:08:19Z

@vitusortner
Working nicely in my project!

floor/lib/src/callback.dart

floor_generator/lib/writer/database_builder_writer.dart

floor_generator/lib/writer/database_writer.dart

.gitignore

noordawod

Changes deployed.

vitusortner · 2019-09-01T13:44:29Z

Looking good so far. I'll add some documentation to the project's README files on the new callbacks and merge the PR after that. Thanks for your assistance!

noordawod · 2019-09-01T15:53:56Z

Sounds good! Maybe also add a test for the new functionality?

vitusortner · 2019-09-01T17:52:38Z

Yes, it would be beneficial to verify that each callback is invoked when it should be. Unluckily, I currently see no simple way of doing that.

noordawod · 2019-09-02T07:57:49Z

Coming to pub.dev anytime soon? :)

vitusortner · 2019-09-02T20:37:46Z

Is now released.

Add optional callback functions when opening database (backward-compa…

1dcf7cb

…tible).

noordawod changed the title ~~Add optional callback functions when opening database (backward-compatible).~~ Add optional callback functions when opening database Aug 27, 2019

Removed a file which slipped through to the repo.

4a5c4f9

Removed auto-generated files usually added by the i18n plugin.

548e4d4

noordawod added 2 commits August 28, 2019 07:29

Trigger Travis build system.

85e4459

Fix tests again (local "pub run test" works).

63a374f

Formatting Dart code.

7cb5e60

vitusortner reviewed Aug 28, 2019

View reviewed changes

floor_generator/lib/writer/database_writer.dart Outdated Show resolved Hide resolved

Update floor_generator/lib/writer/database_writer.dart

bdd27c1

Co-Authored-By: Vitus <[email protected]>

noordawod and others added 3 commits August 29, 2019 09:22

Dartfmt for changed file.

afc0617

Merge branch 'develop' into feature/open-callbacks

825bcd0

Merge branch 'develop' into feature/open-callbacks

f8d35cd

Use a Callback class to add callback support to the database builder.

6f0f6a3

vitusortner requested changes Sep 1, 2019

View reviewed changes

Fix comments reported.

d7bb715

noordawod commented Sep 1, 2019

View reviewed changes

Describe Callback's usage in README

ef0a589

vitusortner self-requested a review September 1, 2019 17:53

vitusortner approved these changes Sep 1, 2019

View reviewed changes

vitusortner merged commit a6473d1 into pinchbv:develop Sep 1, 2019

noordawod deleted the feature/open-callbacks branch September 1, 2019 23:07

vitusortner mentioned this pull request Sep 2, 2019

@update fails on compound key #185

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Add optional callback functions when opening database #182

Add optional callback functions when opening database #182

noordawod commented Aug 27, 2019 •

edited

Loading

dkaera commented Aug 27, 2019

noordawod commented Aug 27, 2019

noordawod commented Aug 27, 2019

noordawod commented Aug 28, 2019

dkaera commented Aug 28, 2019

noordawod commented Aug 28, 2019

noordawod commented Aug 28, 2019

codecov bot commented Aug 28, 2019 •

edited

Loading

vitusortner commented Aug 28, 2019

noordawod commented Aug 29, 2019 •

edited

Loading

noordawod commented Sep 1, 2019

vitusortner commented Sep 1, 2019

noordawod commented Sep 1, 2019 •

edited

Loading

vitusortner commented Sep 1, 2019

noordawod commented Sep 1, 2019

noordawod commented Sep 1, 2019

noordawod left a comment

vitusortner commented Sep 1, 2019

noordawod commented Sep 1, 2019

vitusortner commented Sep 1, 2019

noordawod commented Sep 2, 2019

vitusortner commented Sep 2, 2019

Add optional callback functions when opening database #182

Add optional callback functions when opening database #182

Conversation

noordawod commented Aug 27, 2019 • edited Loading

dkaera commented Aug 27, 2019

noordawod commented Aug 27, 2019

noordawod commented Aug 27, 2019

noordawod commented Aug 28, 2019

dkaera commented Aug 28, 2019

noordawod commented Aug 28, 2019

noordawod commented Aug 28, 2019

codecov bot commented Aug 28, 2019 • edited Loading

Codecov Report

vitusortner commented Aug 28, 2019

noordawod commented Aug 29, 2019 • edited Loading

noordawod commented Sep 1, 2019

vitusortner commented Sep 1, 2019

Implementation

noordawod commented Sep 1, 2019 • edited Loading

vitusortner commented Sep 1, 2019

noordawod commented Sep 1, 2019

noordawod commented Sep 1, 2019

noordawod left a comment

Choose a reason for hiding this comment

vitusortner commented Sep 1, 2019

noordawod commented Sep 1, 2019

vitusortner commented Sep 1, 2019

noordawod commented Sep 2, 2019

vitusortner commented Sep 2, 2019

noordawod commented Aug 27, 2019 •

edited

Loading

codecov bot commented Aug 28, 2019 •

edited

Loading

noordawod commented Aug 29, 2019 •

edited

Loading

noordawod commented Sep 1, 2019 •

edited

Loading