mikkopaderes / ember-cloud-firestore-adapter

Unofficial Ember Data Adapter and Serializer for Cloud Firestore
MIT License
69 stars 17 forks source link

Add support for polymorphic relationships #234

Open pixelhandler opened 2 years ago

pixelhandler commented 2 years ago

With use of polymorphic: true in a (Ember Data) model relationship we need support in the serializer

Here is a solution we put in our app:

import CloudFirestoreSerializer from 'ember-cloud-firestore-adapter/serializers/cloud-firestore-modular';
import { doc } from 'ember-cloud-firestore-adapter/firebase/firestore';
import DS from 'ember-data';
import { inject as service } from '@ember/service';
import { isNone } from '@ember/utils';
import buildCollectionName from 'ember-cloud-firestore-adapter/-private/build-collection-name';
import { CollectionReference, DocumentReference } from 'firebase/firestore';
import firebase from 'firebase/compat/app';
import { singularize } from 'ember-inflector';
import { dasherize } from '@ember/string';

interface Links {
  [key: string]: string;
}

interface ResourceHash {
  id: string;
  links: Links;
  [key: string]: string | Links | CollectionReference;
}

interface RelationshipDefinition {
  key: string;
  type: string;
  options: {
    buildReference(db: firebase.firestore.Firestore): CollectionReference;
    polymorphic: boolean;
  };
}

export default class Application extends CloudFirestoreSerializer {
  @service firebase: any;
  @service store: any;

  // support methods defined on JSONSerializer
  [x: string]: any;

  // Copied from ember-data 3.22.1 and combined with ember-cloud-firestore-adapter
  // with condition to skip in favor of serializePolymorphicType
  serializeBelongsTo(
    snapshot: DS.Snapshot,
    json: { [key: string]: string | null | DocumentReference },
    relationship: RelationshipDefinition,
  ): void {
    const key = relationship.key;
    if (!this._canSerialize(key)) return;

    if (relationship.options.polymorphic) {
      this.serializePolymorphicType(snapshot, json, relationship);
    } else {
      const db = this.firebase.firestore();
      const belongsToId = snapshot.belongsTo(key, { id: true });
      if (isNone(belongsToId)) {
        json[key] = null;
      } else if (relationship.options.buildReference) {
        json[key] = doc(relationship.options.buildReference(db), belongsToId);
      } else {
        const collectionName: string = buildCollectionName(relationship.type);
        const path = `${collectionName}/${belongsToId}`;
        json[key] = db.doc(path);
      }
    }
  }

  serializePolymorphicType(
    snapshot: DS.Snapshot,
    json: { [key: string]: string | null | DocumentReference },
    relationship: RelationshipDefinition,
  ): void {
    const db = this.firebase.firestore();
    let key = relationship.key;
    let belongsTo = snapshot.belongsTo(key);

    if (isNone(belongsTo)) {
      json[key] = null;
    } else {
      const collectionName = buildCollectionName(belongsTo.modelName as string);
      const belongsToId = snapshot.belongsTo(key, { id: true });
      const path = `${collectionName}/${belongsToId}`;
      json[key] = db.doc(path);
    }
  }

  // Firestore adapter does not extract polymorphic types
  // https://github.com/mikkopaderes/ember-cloud-firestore-adapter/blob/master/addon/serializers/cloud-firestore.ts#L50
  // NOTE: `extractRelationship(relationshipModelName: string, resourceHash: ResourceHash): {}` may also return null, same for polymorphic
  // 
  // @ts-ignore
  extractPolymorphicRelationship(relationshipModelName: string, relationshipHash: ResourceHash /*, _relationshipOptions*/): {} | null {
    if (isNone(relationshipHash)) {
      return null;
    }

    const [collectionName, belongsToId] = (relationshipHash.path as string).split('/');
    relationshipModelName = dasherize(singularize(collectionName));

    return { id: belongsToId, type: relationshipModelName };
  }
}

// DO NOT DELETE: this is how TypeScript knows how to look up your serializers.
declare module 'ember-data/types/registries/serializer' {
  export default interface SerializerRegistry {
    'application': Application;
  }
}
charlesfries commented 2 years ago

Nice work here. Can you post the _canSerialize method as well?

pixelhandler commented 2 years ago

@charlesfries here is the link to the _canSerialize method on JSONSerializer of ember-data, https://github.com/emberjs/data/blob/master/packages/serializer/addon/json.js#L896-L909

charlesfries commented 2 years ago

Thanks @pixelhandler. I am super appreciative of this as I've been trying to get polymorphism working in this addon for a while, and you have taken it significantly further than I had. I'm just struggling to get your code working. The issue for me seems to be that the polymorphic base class is never replaced by the more specific class we get from the collection name in the Firestore reference value, which I believe should be happening in the extractPolymorphicRelationship method. Can you take a glance at the code below and see if I am doing anything obviously wrong?

I've made the following modifications to the ember-cloud-firestore-adapter dummy app to test this out. First I added your serializer code, and then made these models;

// app/models/owner.js
// ***this is the base class for the polymorphic relationship***

import Model from '@ember-data/model';

export default class OwnerModel extends Model {}
// app/models/user.js

import OwnerModel from './owner';

export default class UserModel extends OwnerModel {
  ...
}
// app/models/group.js

import OwnerModel from './owner';

export default class GroupModel extends OwnerModel {
  ...
}
// app/models/event.js

import Model, { belongsTo } from '@ember-data/model';

export default class EventModel extends Model {
  @belongsTo('owner', { polymorphic: true }) owner;
}

Then, in Cloud Firestore, create a record under events/event_a and add an owner attribute with reference value users/user_a.

Now fetch the event:

// app/routes/application.js

import Route from '@ember/routing/route';

export default class ApplicationRoute extends Route {
  @service store;

  async model() {
    const event = await this.store.findRecord('event', 'event_a');
    const user = await event.owner;
    console.log(user);
    console.log(user.name);
  }
}

I expect to see a UserModel object and user_a (the value of the UserModel name attr) in the console. However, I only see OwnerModel and undefined as the output. Are you seeing the same thing?

mikkopaderes commented 2 years ago

Thanks for bringing this up. I've never used the polymorphic attribute for ember-data but I could see how this is useful.

At the moment, I'm unable to work much on the project because my focus is somewhere else, and if ever I start picking up work on this again, my priority is to address #229 on which I have some WIP already in this branch.

Would be happy to take a PR for this if anyone wants to try.

charlesfries commented 2 years ago

Think I figured it out. Just needed to explicitly defer to extractPolymorphicRelationship in the existing extractRelationships method. Here is a draft PR #236

mikkopaderes commented 2 years ago

Copy-paste from original comment

I've been looking into this and my conclusion here is I don't want to implement a default way to handle polymorphism. What I'm looking into is if I can provide some overridable API in the serializer that can be used to setup polymorphism. But to be honest, I don't think that it's a good idea to use polymorphism even if the adapter is capable of doing it especially for large scale apps, let me explain.

In order for polymorphism to work, your data needs to have some indicator on what type of model this would be. It can look like this:

{
  "id": "farm_1",
  "animals": [
    { "id": "pet_1", "type": "sheep" },
    { "id": "pet_2", "type": "cow" },
    { "id": "pet_3", "type": "cow" },
    { "id": "pet_4", "type": "chicken" },
  ]
}

The example above is for a has-many polymorphism. Imagine if you have that as a Document in Firestore, the animals field could have an infinite amount of data. If you fetch the farm_1 document, you would have to download the whole animals field as well—you wouldn't have the ability to limit/paginate it. Not only will this be slow for your users, it will also cost you more especially if you have a listener setup on that document.

Granted that some may force their way into using polymorphism especially if they can guarantee that the has-many relationship wouldn't grow in size, this is something that I will try to look into. As mentioned above, the approach that I'd like to take is to provide APIs that developers will override in order to setup polymorphism based on how they structure the data in their Firestore project.

EDIT:

Actually, it seems that Ember Data is capable for determining async polymorphic. Meaning, you can determine the polymorphic type after doing the actual fetching. This should provide a better and more natural integration with Firestore as I build the APIs.

knownasilya commented 1 year ago

Should #236 be revisited because of your edit comment?

mikkopaderes commented 1 year ago

I'll be honest that my last comment here has been more than a year already. I can't recall if something prevented me from working on the feature and I should've commented it if there was any. I don't have a timeline on when I can revisit this.