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

Store additional MultaccContact fields in database #72

Merged
merged 1 commit into from Mar 12, 2020
Merged

Conversation

@TortoiseWrath
Copy link
Member

TortoiseWrath commented Mar 12, 2020

Closes #70.

In this commit, only display name, avatar, and birthday are stored
alongside MultaccItems. Additional fields should be added as they
become necessary.

The following changes were made to facilitate this:

  • Add Hive adapters for MultaccContact and MultaccItem
  • Modify MultaccItem.fromDB to take only the JSON value as a parameter
    • Modify dummy contacts json to include key
  • Create an empty default constructor for MultaccContact
  • Name the existing MultaccContact(Contact) constructor as fromContact
  • Remove the existing addContact method from the database interface and
    rename addMultaccContact to addContact
  • Add names to the dummy contacts
  • DatabaseInterface.getContact now returns null if the contact does
    not exist (previously it returned an empty list of items)

Other changes while I'm here:

  • Add constants in item.dart for '_t' and '_id' magic keys
  • Add toString to MultaccItem, which calls jsonEncode on it
  • Add DatabaseInterface.getAllContacts
  • Rename hive box from 'box' to 'contacts'
  • Contacts box can now only contain contacts
  • Move Hive box info from main.dart into DatabaseInterface
    • Makes adding additional boxes easier
    • Closes #67
  • Rename DatabaseInterface instance in main from dbi to db

This uses a generated Hive adapter for MultaccContact. In order to
generate the adapter, we have to apply annotations to the fields we want
to store, which means MultaccContact has to override the fields from
Contact we want to store. Overriding fields is generally frowned upon,
so if this is a concern the adapter can be replaced with a manually
written one in the future.

We cannot generate a Hive adapter for MultaccItem because it has several
implementations with their own fields that have to be stored. We could
generate adapters for each MultaccItem implementation, but then we would
need code when storing the items to detect their type and code when
reading them from the database to handle each type differently. For the
same reason, the adapter I have written continues to use our existing
JSON serialization for MultaccItem.

I created a file, type_ids.dart, which contains constants for the type
IDs for Hive adapters. This is because these must be globally unique,
and keeping track of them in a single file will be easier than having
them each in the file with the adapter.

As MultaccItem.fromDB no longer takes parameters other than a single
json-formatted string, it is conceivable to make it the fromJson method.
This would necessitate renaming the existing fromJson method to fromMap,
paralleling toMap; this is the method that would be separately implemen-
ted for each item type.

Closes #70.

In this commit, only display name, avatar, and birthday are stored
alongside MultaccItems. Additional fields should be added as they
become necessary.

The following changes were made to facilitate this:
* Add Hive adapters for MultaccContact and MultaccItem
* Modify MultaccItem.fromDB to take only the JSON value as a parameter
    * Modify dummy contacts json to include key
* Create an empty default constructor for MultaccContact
* Name the existing MultaccContact(Contact) constructor as fromContact
* Remove the existing addContact method from the database interface and
  rename addMultaccContact to addContact
* Add names to the dummy contacts
* DatabaseInterface.getContact now returns null if the contact does
  not exist (previously it returned an empty list of items)

Other changes while I'm here:
* Add constants in item.dart for '_t' and '_id' magic keys
* Add toString to MultaccItem, which calls jsonEncode on it
* Add DatabaseInterface.getAllContacts
* Rename hive box from 'box' to 'contacts'
* Contacts box can now only contain contacts
* Move Hive box info from main.dart into DatabaseInterface
    * Makes adding additional boxes easier
    * Closes #67
* Rename DatabaseInterface instance in main from dbi to db

This uses a generated Hive adapter for MultaccContact. In order to
generate the adapter, we have to apply annotations to the fields we want
to store, which  means MultaccContact has to override the fields from
Contact we want to store. Overriding fields is generally frowned upon,
so if this is a concern the adapter can be replaced with a manually
written one in the future.

We cannot generate a Hive adapter for MultaccItem because it has several
implementations with their own fields that have to be stored. We *could*
generate adapters for each MultaccItem implementation, but then we would
need code when storing the items to detect their type and code when
reading them from the database to handle each type differently. For the
same reason, the adapter I have written continues to use our existing
JSON serialization for MultaccItem.

I created a file, type_ids.dart, which contains constants for the type
IDs for Hive adapters. This is because these must be globally unique,
and keeping track of them in a single file will be easier than having
them each in the file with the adapter.

As MultaccItem.fromDB no longer takes parameters other than a single
json-formatted string, it is conceivable to make it the fromJson method.
This would necessitate renaming the existing fromJson method to fromMap,
paralleling toMap; this is the method that would be separately implemen-
ted for each item type.
@TortoiseWrath TortoiseWrath added this to the sprint-2 milestone Mar 12, 2020
@TortoiseWrath TortoiseWrath requested a review from mayank99 Mar 12, 2020
@multacc multacc deleted a comment from todo bot Mar 12, 2020
@HiveField(3)
Uint8List avatar;
Comment on lines +29 to +30

This comment has been minimized.

Copy link
@TortoiseWrath

TortoiseWrath Mar 12, 2020

Author Member

I figure we may as well try this and delete it if the performance is no good

import 'package:enum_to_string/enum_to_string.dart';

const ITEM_TYPE_KEY = '_t';
const ITEM_KEY_KEY = '_id';

This comment has been minimized.

Copy link
@mayank99

mayank99 Mar 12, 2020

Contributor

Kiki

This comment has been minimized.

Copy link
@TortoiseWrath

TortoiseWrath Mar 12, 2020

Author Member

const KEY_KEY_KEY = 'ITEM_KEY_KEY';

}

void write(BinaryWriter writer, MultaccItem item) {
writer.writeString(item.toString());

This comment has been minimized.

Copy link
@mayank99

mayank99 Mar 12, 2020

Contributor

Is string the best way to store this? Hive supports binary objects

This comment has been minimized.

Copy link
@TortoiseWrath

TortoiseWrath Mar 12, 2020

Author Member

it's ... already json?

This comment has been minimized.

Copy link
@TortoiseWrath

TortoiseWrath Mar 12, 2020

Author Member

from slack:

"I'm talking about binary objects without json"

but how

We cannot generate a Hive adapter for MultaccItem because it has several implementations with their own fields that have to be stored. We could generate adapters for each MultaccItem implementation, but then we would need code when storing the items to detect their type and code when reading them from the database to handle each type differently. For the same reason, the adapter I have written continues to use our existing JSON serialization for MultaccItem.

so afaik the only reasonable way to convert a MultaccItem to a binary object that Hive can handle would be to write a complete binary serialization method for MultaccItem, and then we would have two serialization methods on it for no particular reason

@mayank99 mayank99 merged commit e3e093b into master Mar 12, 2020
1 check passed
1 check passed
build
Details
@mayank99 mayank99 deleted the contact-db branch Mar 12, 2020
TortoiseWrath added a commit that referenced this pull request Mar 12, 2020
Forgot to do this in #72. Need to register the adapters when
initializing hive. Failure to do so makes it no worky
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Projects
None yet
Linked issues

Successfully merging this pull request may close these issues.

2 participants
You can’t perform that action at this time.