Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Can't primaryKey be String type? #313

Closed
heneyin opened this issue Apr 15, 2020 · 12 comments · Fixed by #318
Closed

Can't primaryKey be String type? #313

heneyin opened this issue Apr 15, 2020 · 12 comments · Fixed by #318
Labels
question Further information is requested

Comments

@heneyin
Copy link

heneyin commented Apr 15, 2020

When I create a entity like this:

class User {
  @PrimaryKey()
  final String id;
}

After flutter packages pub run build_runner build, the dao mapper of generated in *.g.dart file is :

id: row['id'] as double,

I need to change it manually every build as:

id: row['id'] as String,

Otherwise it won't work when operate db :

type 'String' is not a subtype of type 'double' in type cast

Is this a bug?

Version:

dependencies:
  floor: ^0.12.0

dev_dependencies:
  floor_generator: ^0.12.0
  build_runner: ^1.7.3
@heneyin
Copy link
Author

heneyin commented Apr 20, 2020

🤷‍♀️

@heneyin heneyin closed this as completed Apr 20, 2020
@vitusortner
Copy link
Collaborator

Has the problem been solved already?

@heneyin heneyin reopened this Apr 22, 2020
@heneyin
Copy link
Author

heneyin commented Apr 22, 2020

Has the problem been solved already?

Not yet, I solved this by manually modifying the *.g.database after each build.

@TheVinhLuong
Copy link

TheVinhLuong commented Apr 23, 2020

@Henvealf I have a string as a primary key too and everything works just fine.

@mqus
Copy link
Collaborator

mqus commented Apr 23, 2020

Where did you get this version from? Afaict there is only a 0.12.0. But I'll take a look and report back.

@TheVinhLuong
Copy link

@mqus my bad, it is 0.12.0.

@mqus
Copy link
Collaborator

mqus commented Apr 23, 2020

I just adjusted the integration tests (id of Dog to be of type String) and could not observe your bug. everything is generated as expected:
In TestDatabase::open

      onCreate: (database, version) async {
        await database.execute(
            'CREATE TABLE IF NOT EXISTS `person` (`id` INTEGER, `custom_name` TEXT NOT NULL, PRIMARY KEY (`id`))');
        await database.execute(
            'CREATE TABLE IF NOT EXISTS `dog` (`id` TEXT, `name` TEXT, `nick_name` TEXT, `owner_id` INTEGER, FOREIGN KEY (`owner_id`) REFERENCES `person` (`id`) ON UPDATE NO ACTION ON DELETE CASCADE, PRIMARY KEY (`id`))');
        await database.execute(
            'CREATE INDEX `index_person_custom_name` ON `person` (`custom_name`)');

        await callback?.onCreate?.call(database, version);
      },

in _$DogDao

  static final _dogMapper = (Map<String, dynamic> row) => Dog(
      row['id'] as String,
      row['name'] as String,
      row['nick_name'] as String,
      row['owner_id'] as int);

And the tests ran through successfully after also changing the calls to the constructor in the tests.

I'll assume that you already tried to remove the generated file and regenerate it from scratch, so can you share a bit more of your code, maybe a reproducable example?

@heneyin
Copy link
Author

heneyin commented Apr 25, 2020

@mqus

entities.dart:

import 'package:equatable/equatable.dart';
import 'package:floor/floor.dart';
import 'package:uuid/uuid.dart';

class CommonField extends Equatable{

  @ColumnInfo(name: 'create_time', nullable: false)
  final String createTime;

  @ColumnInfo(name: 'update_time')
  final String updateTime;

  @ColumnInfo(nullable: false)
  final bool avail;

  CommonField(
      this.updateTime,
      this.avail, {
        createTime,
      }) : this.createTime = createTime ?? DateTime.now().toString();

  @override
  List<Object> get props => [];
}

@Entity(tableName: 'groups')
class Group extends CommonField{

  final String name;

  @PrimaryKey()
  final String id;

  final String content;

  Group(
    this.name, {
    id,
    this.content = '',
    String createTime,
    String updateTime,
    bool avail = true
  }) : this.id = id ?? Uuid().v4(),
       super(updateTime, avail);


  @override
  String toString() {
    return 'Group{name: $name, id: $id, content: $content}';
  }

  @override
  List<Object> get props => [id, name, content];
}

dao.dart

@dao
abstract class GroupDao {

  static const _QUERY_ALL = 'xxxxxx';

  @Query(_QUERY_ALL)
  Future<List<Group>> findAllGroups();
}

database.dart:

@Database(version: 1, entities: [Group])
abstract class AppDatabase extends FloorDatabase {
  GroupDao get groupDao;
}

after flutter packages pub run build_runner build, the part of database.g.dart:

class _$GroupDao extends GroupDao {
  _$GroupDao(this.database, this.changeListener)
      : _queryAdapter = QueryAdapter(database);

  final sqflite.DatabaseExecutor database;

  final StreamController<String> changeListener;

  final QueryAdapter _queryAdapter;

  static final _groupsMapper = (Map<String, dynamic> row) => Group(
      row['name'] as String,
      id: row['id'] as double,
      content: row['content'] as String,
      createTime: row['create_time'] as String,
      updateTime: row['update_time'] as String,
      avail: (row['avail'] as int) != 0);

  @override
  Future<List<Group>> findAllGroups() async {
    return _queryAdapter.queryList('xxxxxx', mapper: _groupsMapper);
  }
}

You can see the double in database.g.dart.

@mqus
Copy link
Collaborator

mqus commented Apr 25, 2020

Thank you very much! I could reproduce it now.
Regarding the solution: the dart analyzer tool pointed me to your constructor (Missing parameter type for id).
If I write

Group(
    this.name, {
    String id,
    this.content = '',
    String createTime,
    String updateTime,
    bool avail = true
  }) : this.id = id ?? Uuid().v4(),
       super(updateTime, avail);

(note the additional String before id), then everything gets generated just fine. Why it assumes that it should be double is something that I 'll investigate now have explained below.

Btw, the analyzer also complained about a missing parameter type of createTime in the constructor of CommonField.
I get notified about those things by Android studio automatically, but you can also run the analyzer manually by executing

flutter analyze

I hope I could help 😉

@mqus
Copy link
Collaborator

mqus commented Apr 25, 2020

Ah right, 'double' is the fallback string, so even if the type was null for any constructor argument (not just the primary key), it would fall back to using 'double'.
https://github.com/vitusortner/floor/blob/cfa25d3b01b69a7ce281c9043604b8657264e03c/floor_generator/lib/processor/queryable_processor.dart#L77-L93

We should probably generate an error there if the type does not get matched. But this code is currently subject to change anyway due to #318 . What do you think, @vitusortner ?

@mqus mqus added the question Further information is requested label May 5, 2020
@vitusortner
Copy link
Collaborator

Such cases won't be possible anymore with the type converters implementation because we'll be making sure that used values and types are database-compatible. If they're not database-compatible out of the box, we'll be looking for matching type converters. If no converter can be found, an error will be thrown.

@mqus
Copy link
Collaborator

mqus commented May 27, 2020

This case was a special case where no type was set for a variable, so we should definitely look out for it (or test for it). But in either case this is an easy fix, if it occurs again.

I will be closing this issue, as @Henvealf hasn't mentioned any other issues. If you still have this problem please reopen this issue or open a new one.

@mqus mqus closed this as completed May 27, 2020
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
question Further information is requested
Development

Successfully merging a pull request may close this issue.

4 participants