drift icon indicating copy to clipboard operation
drift copied to clipboard

.moor Queries in Daos should be isolated

Open JCKodel opened this issue 5 years ago • 3 comments

Don't know if I'm using Daos properly, but...

Consider the following code:

database.dart

@UseMoor(
  include: {"users.moor", "purchases.moor"},
  daos: [UsersDao, PurchasesDao],
)
class Database extends _$Database {
  // ...
}

users_dao.dart

@UseDao(
  include: {"users.moor"},
)
class UsersDao extends DatabaseAccessor<Database> with $_UsersDaoMixin {
  // ...
}

purchases_dao.dart

@UseDao(
  include: {"purchases.moor"},
)
class PurchasesDao extends DatabaseAccessor<Database> with $_PurchasesDaoMixin {
  // ...
}

users.moor

CREATE TABLE Users(
  -- ...
);

getById: SELECT * FROM Users WHERE "id" = :id;

This will generate a method in database.dart AND the same method in users_dao.dart. I understand the need for tables and fields to be duplicated here, but methods should be available only for Daos, if they are using, because:

  1. The whole point of Daos is to isolate domains of my app (so Users don't know Purchases, but Purchases know Users (because it imports "users.moor")

  2. There should not be conflicts in generated methods (if purchases.moor also have a getById, database.dart will try to write 2 methods with the same name), avoidable, but still, I would prefer to use db.usersDao.getById() instead of db.usersDao.getUserById() and not to be worry about conflicts when there will be dozens of Daos in a highly complex application.

One solution I could think of was to make getById private (by writing _getById: SELECT ... in .moor files).

The problem with this approach is that duplicates are still possible and it gives me a warning from lint (unused_element - The declaration '_getById' isn't referenced) in the database.g.dart.

So, my requests are:

  1. Easy: Since analyser ignore doesn't work on VSCode, could you please add a // ignore-for-file: unused_element into all generated files? (notice: Daos have no ignore_for_file directive). It would be great if we could add our own ignores in @UseDao(ignores: ["unused_element"])

  2. Hard: It would be too hard to generate all code inside daos, instead of database? I need to duplicate include: {"script.moor"} in both database.dart and users_dao.dart because I want the method to be available only in dao (because it will be private) and because Dao references tables from the database.dart (so importing in database.dart is mandatory).

JCKodel avatar Apr 09 '20 16:04 JCKodel

What I prefer to do for this is to split moor files containing tables and moor files containing queries. So usually I end up with a folder structure like this:

/path/to/database/
├── tables/
│   ├── all.moor (would only import users.moor and purchases.moor)
│   ├── users.moor
│   └── purchases.moor (can import users.moor)
├── queries/
│   ├── user_queries.moor (imports ../tables/user.moor)
│   └── purchase_queries.moor (imports ../tables/purchases.moor)
├── daos
│   ├── users_dao.dart (includes ../queries/user_queries.moor)
│   └── purchases_dao.dart (includes ../queries/purchasse_queries.moor)
├── (usually I have another folder for type converters etc.)
└── database.dart (includes tables/all.moor)

I think this is an acceptable solution to archive modular code without duplication: Each query will only be generated in the matching dao, so there are no naming conflicts. Changing where code will be generated can quickly become a breaking change that I'd prefer to avoid. Do you think the approach of splitting table / query files works for you?

simolus3 avatar Apr 09 '20 18:04 simolus3

I think this is an acceptable solution to archive modular code without duplication: Each query will only be generated in the matching dao, so there are no naming conflicts. Changing where code will be generated can quickly become a breaking change that I'd prefer to avoid. Do you think the approach of splitting table / query files works for you?

It would, if there isn't a bug on using multiple moors.

For instance:

CREATE TABLE CapillaryScheduleSteps (
  "id" CHAR(36) NOT NULL PRIMARY KEY,
  "userId" CHAR(28) NOT NULL REFERENCES Users("id"),
  "capillaryScheduleId" CHAR(36) NOT NULL REFERENCES CapillarySchedules("id"),
  "annotation" VARCHAR(MAX) NOT NULL,
  "alarm" DATETIME NOT NULL,
  "type" INT MAPPED BY `EnumConverters.capillaryScheduleType` NOT NULL,
  "done" INT MAPPED BY `EnumConverters.capillaryScheduleStepDoneStatus` NOT NULL,
  "created" DATETIME NOT NULL,
  "updated" DATETIME NOT NULL,
  "deleted" BOOLEAN NOT NULL
);

I have three tables: CapillarySchedules, CapillaryScheduleSteps and CapillaryScheduleStepPhotos.

When all tables are inside capillary_schedule.moor, it works fine. When I create one .moor for each table, the line "type" INT MAPPED BY EnumConverters.capillaryScheduleType NOT NULL, gives an error in generator:

[INFO] Running build...
[SEVERE] moor_generator:preparing_builder on lib/data/sql/capillary_schedule_steps.moor:

Bad state: No element
[WARNING] moor_generator:moor_generator on lib/data/capillary_schedules_dao.dart:
There were some errors while running moor_generator on package:meucronogramacapilar/data/capillary_schedules_dao.dart:
[WARNING] moor_generator:moor_generator on lib/data/capillary_schedules_dao.dart:
Could not parse file: AssetNotFoundException: meucronogramacapilar|lib/data/sql/capillary_schedule_steps.dart_in_moor
[SEVERE] moor_generator:moor_generator on lib/data/capillary_schedules_dao.dart:
Error running MoorGenerator
NoSuchMethodError: The getter 'resolvedImports' was called on null.
Receiver: null
Tried calling: resolvedImports
[WARNING] moor_generator:moor_generator on lib/data/database.dart:
There were some errors while running moor_generator on package:meucronogramacapilar/data/database.dart:
[WARNING] moor_generator:moor_generator on lib/data/database.dart:
Could not parse file: AssetNotFoundException: meucronogramacapilar|lib/data/sql/capillary_schedule_steps.dart_in_moor
[SEVERE] moor_generator:moor_generator on lib/data/database.dart:
Error running MoorGenerator

So I merged all correlated tables in the same .moor file =\

JCKodel avatar Apr 09 '20 18:04 JCKodel

That looks like a bug worth looking into, thanks for bringing it up! Can you try to re-run the build with the -v flag to get verbose output? I'm interested in what moor_generator:preparing_builder is complaining about here.

I just realized that Dart converter imports in moor files aren't transitive. So you'd have to import the Dart file defining EnumConverters into lib/data/sql/capillary_schedule_steps.moor if you haven't done that. I'll look at ways to fix that.

simolus3 avatar Apr 09 '20 20:04 simolus3

This problem will be solved with modular code generation (#490). With the default generation, I don't think it can be solved in a reliable way since everything transitively reachable from any drift file is supposed to be put in the database or a DAO.

So I think the rest of this issue will be addressed by #490 once that is ready.

simolus3 avatar Nov 20 '22 16:11 simolus3