[ODM] Add a way to store the document ID in the model

[rrousselGit]

Users want a way to store the ID of a document within the deserialized class

Some things to consider:

• calling ref.set(...) or ref.update(...) should not allow changing the "id", since it isn't part of the document but instead some metadata

[arnoutvandervorst]

Same goes for the documentPath, which would make an update self-contained. When deserialising using json_serializable I augment all objects with id, path and parent. Since this is not handled by json_serializable I have an abstract BaseModel with only these properties (which are set to @jsonkey(ignore: true), all other Firestore based classes extend BaseModel.

This means changing the withConverter to:

fromFirestore: (doc, _) => fromFirestore(doc)..augment(doc),
toFirestore: (model, _) => toFirestore(model),

void augment(DocumentSnapshot<Map<String, dynamic>> doc) {
  id = doc.id;
  path = doc.reference.path;
  parent = doc.reference.parent;
}

[Lyokone]

Would something like that be acceptable for a PR : https://github.com/Lyokone/flutterfire/pull/1/files ?

I would add the ability to configure how the mapping is done.
Would love know if this approach is acceptable or if it's not something maintainable enough to make it to master ?

[rrousselGit]

@Lyokone no, there's a lot more to it than this

There's some disambiguation to do. For example what if the model contains a property called path? We don't want the document path to override the actual path property.
Meaning there's some error handling to do too: what if the json contains a path/id property but they were supposed to be injected by Firestore?

Mutating the map passed to fromJson is a no-go either, since that could have side-effects

We also don't want to generate update(path: ...)/update(id: ...)

And there's testing needed

[poirierlouis]

I propose to add an annotation, something like:

@JsonSerializable('galaxies')
class Galaxy {

  Galaxy({this.documentId, required this.name});

  @JsonDocumentId()
  final String? documentId;

  final String name;

}

Type would be nullable:

• when instantiating id is null by default.
• when reading from Firestore, id is obtained from document reference using annotation metadata and passed to fromJson map.

If possible, annotation could assert Type is nullable.

I think it would prevent ambiguation as mentionned above.
It would also allow to not generate update(documentId: ...).

Would it also have side-effects to mutate the map passed to fromJson while using this annotation?

Finally I guess this might be done along with json_serializable package.

[AgDude]

I would like to propose making the snapshot or reference available rather than only the id. This has the advantage that it can be used to access subcollections directly from the model instance.

I am new to dart, so not sure on the feasibility, but what about something like below, where the new annotation excludes the field from json serializable as well as identifies it for the odm generator.

@JsonSerializable('galaxies')
class Galaxy {

  Galaxy({this.documentId, required this.name});

  @docementSnapshot()
  final FirestoreDocumentSnapshot? snapshot;

  final String name;

}

[rrousselGit]

Attaching the snapshot is counter-productive IMO.

If that solution is fine with you, then you could directly pass the Firestore snapshot instead of passing only the document content.

The purpose of this issue I believe is for code that is not firebase aware

[poirierlouis]

@rrousselGit Could my proposal be acceptable ?

[Jjagg]

I implemented this for my project using an annotation:

// user.dart

@JsonSerializable()
class User {
  User({
    required this.name,
  });

  factory User.fromJson(Map<String, dynamic> json) => _$UserFromJson(json);

  @Inject(FirestoreValue.id)
  String? _id;
  String? get id => _id;

  final String name;
}

// user.g.dart

static User fromFirestore(
  DocumentSnapshot<Map<String, Object?>> snapshot,
  SnapshotOptions? options,
) {
  final data = User.fromJson(snapshot.data()!);
  data._id = snapshot.id;
  return data;
}

Injected fields are filtered out of queryable fields, so users can use public mutable fields if they prefer.
I also added support for setters, but that's mostly because I was experimenting with the analyzer API; I don't think it is very useful to support setters for injection.

My branch is here: https://github.com/Jjagg/flutterfire/tree/odm-inject

I added logic to inject:

• id
• path
• parent document id (for subcollections)

With some experimentation this is the way I thought worked best for injecting stuff.

[larssn]

I suggest not deviating the API from the official one. Otherwise we could find ourselves in a future scenario where the official API (iOS/Android) is changed in a way that makes it incompatible with this version.

IMHO this plugin should mainly exist as a wrapper around the official API.

[Jjagg]

Any feedback on my comment?
The implementation is fairly straightforward. If my approach is seen as a good solution I could try to set up a PR for it :)