dappsnation / akita-ng-fire

Akita ❤️ Angular 🔥 Firebase
MIT License
131 stars 27 forks source link

FR: Expose entity metadata #178

Closed mcroker closed 3 years ago

mcroker commented 3 years ago

Is it possible to expose the object metadata through the akita entity store...

As I'm in danger of jumping to the solution, the challenge I have is starting a dynamic store for a subcollection before the parent collection is created fully...

Give a firestore structure of .. /{library}/books/{book}

I am:

I'm getting beaten by a race-condition... libraryService.selectAll() includes the new library, before firebase will accept the sync() collection query for the books SubCollection. It is failing with an insufficient permissions error.

Looking at the metadata property , I believe the issue that that the subCollection query is not accepted until the hasPendingWrites property on the doc metadata object returned by return this.db.collection(path, queryFn).stateChanges() is false. metadata: {hasPendingWrites: false, fromCache: false} I can currently find no way of detecting this when using syncCollection() - hence my request to expose the metadata.

GrandSchtroumpf commented 3 years ago

Hmmm that's typically the kind of situation where I think using akita-ng-fire is overkill. I guess you want to do something like displaying a list of library with their books. Personally I would go for a solution with a Pipe without using the store

<ng-container *ngFor="let library of libraries$ | async; trackBy: trackByID">
  <h2>{{ library.name }}</h2>
  <ul>
    <li *ngFor="let book of library.id | getBook | async; trackBy: trackByID">{{ book.title }}</li>
  </ul>
</ng-container>
@Pipe({ name: 'getBook' })
export class GetBookPipe implements PipeTransform {
  record: Record<string, Observable<Book[]>> = {};
  constructor(private service: BookService) {}
  transform(libraryId: string) {
    if (!this.record[libraryId]) {
      this.record[libraryId] = this.service.valueChanges({ params: { libraryId } });
    }
    return this.record[libraryId];
  }
}

Using multiple store might create side effects, and will make it super difficult to maintain in the long run. My advice: use akita for long term / global values like "notification", "auth", ... anything that doesn't require querying.

mcroker commented 3 years ago

Thanks for the response... library/{lib}/books/{books} is a good parallel to my data-model. Keeping the analogy, I actually go one further and have multiple child, and one grand-child subCollections.

   /library/{library}
        /book/{book}
            /issue_history/{record}
        /borrower/{borrower}

The PipeCode above is elegant (I need to research pipes a bit better)...

My only concern is that the volatile entity is actually the book sub-collection, not the library collection (book added to library far more often than new libraries are built, so it's really for {books} where I get the value of using a state manager. I was also hoping to use Akita persistance to allow the app some offline capability.

I worked around my first issue by waiting for the new library to have settled before creating the store/query/service for library_{newlib}_books.

    protected async serviceConstructor(libraryid: ID): Promise<Services<Book, StoreService, BookQuery>> {

        // Wait for the library to be in a fit state...
        await this.angularFireStore.doc(`/libary/${libraryid}`).snapshotChanges().pipe(
            filter(doc => !doc.payload.metadata.hasPendingWrites),
            take(1)
        ).toPromise();

        // Create the dynamic Akita store
        const store = new BookStore(libraryid);
        const query = new BookQuery(libraryid);
        const service = new BookService(libraryid, store);
        return { store, query, service };
    }

Using multiple store might create side effects... Yep hit my next one! I think Akita has a race-condition impacting perstance of multiple stores: https://github.com/datorama/akita/issues/602

fritzschoff commented 3 years ago

Well I guess I can implement a function that lets you operate on the meta data.

mcroker commented 3 years ago

That would work... as would including metadata on the CollectionState() interface.

My design is a called into question by finding the race-condition described above... I'm mulling over my options...

The BookStore factory solution works nicely, but unsurprisingly it's the create() and destroy(0 cases that are causing challenge (e.g. stopping a store syncing when it's parent is deleted)

fritzschoff commented 3 years ago

published a new version of 4.3.1@beta npm i akita-ng-fire@beta. Let me know your feedback. Here you also can see how you could use it. https://github.com/dappsnation/akita-ng-fire/pull/181/files#diff-142ed72bf40adaed9a9d4f5184e0a561d48c1868cfa30f951a10211dfcd2e56cR16

mcroker commented 3 years ago

Firstly THANK-YOU for such a swift response, once again proving the OSS community is faster and often more helpful than commercial software...

The fix didn't actually do the job. Not because of it's implementation, but because I missunderstood the problem. The problem I have is because there security rules rely on the content of the record, I can't even query it for metadata until firebase finishes it's back end... I'm not quire sure why it appears in the library collection selectAll() prior to this, but it definitely does.

In the interim I've implemented this (rather ugly, but effective) workaround, which catches the permissions error and implements an exponential retry. It seems on my high-bandwidth home network to resolve inside 100ms the vast majority of the time.

    syncWithRetry(delay: number = 100, ...args: string[]) {
        return this.sync(...args).pipe(
            catchError((error, obs) => {
                if (delay <= 3000 && error.name === 'FirebaseError' && error.code === 'permission-denied') {
                    console.log(`Permission denied ${args} - trying again in ${delay}ms`);
                    return timer(delay).pipe(take(1), switchMap(() => this.syncWithRetry(delay * 2, ...args)));
                }
                throw (error);
            })
        );
    }
fritzschoff commented 3 years ago

Sorry to hear that! I would close this issue for know, but feel free to open a new issue or post something new here so I can reopen this issue.