fix: fixed documentRef not having converter configured

- Fixed issue where DocumentReferences that were created through the firestoreDocumentAccessor would not have the converter set, causing issues with snapshot conversion/serialization

BREAKING CHANGES: converter is now required for firestoreContext collection config

apps/demo-api/src/app/function/guestbook/guestbookentry.function.spec.ts

@@ -42,6 +42,29 @@ demoApiFunctionContextFactory((f: DemoApiFunctionContextFixture) => {
 
           describe('guestbook entry exists', () => {
             demoGuestbookEntryContext({ f, u, g }, (ge) => {
+              describe('reading a guestbook entry', () => {
+                describe('with incomplete database data', () => {
+                  beforeEach(async () => {
+                    // clear the data from the entry
+                    await ge.document.accessor.update({ updatedAt: null, createdAt: null } as any);
+                  });
+
+                  it('should read default times', async () => {
+                    const rawData = (await ge.document.accessor.getWithConverter(null)).data();
+
+                    expect(rawData?.updatedAt).toBeNull();
+                    expect(rawData?.createdAt).toBeNull();
+
+                    const data = await ge.document.snapshotData();
+
+                    expect(data?.updatedAt).toBeDefined();
+                    expect(data?.createdAt).toBeDefined();
+                    expect(isDate(data?.updatedAt)).toBe(true);
+                    expect(isDate(data?.createdAt)).toBe(true);
+                  });
+                });
+              });
+
               it('should update guestbook entry.', async () => {
                 const userGuestbookEntry = ge.document;
 

components/demo-firebase/src/lib/models/guestbook/guestbook.ts

@@ -54,13 +54,14 @@ export const guestbookConverter = snapshotConverterFunctions<Guestbook>({
 });
 
 export function guestbookCollectionReference(context: FirestoreContext): CollectionReference<Guestbook> {
-  return context.collection(guestbookIdentity.collection).withConverter<Guestbook>(guestbookConverter);
+  return context.collection(guestbookIdentity.collection);
 }
 
 export type GuestbookFirestoreCollection = FirestoreCollection<Guestbook, GuestbookDocument>;
 
 export function guestbookFirestoreCollection(firestoreContext: FirestoreContext): GuestbookFirestoreCollection {
   return firestoreContext.firestoreCollection({
+    converter: guestbookConverter,
     modelIdentity: guestbookIdentity,
     itemsPerPage: 50,
     collection: guestbookCollectionReference(firestoreContext),
@@ -116,7 +117,7 @@ export const guestbookEntryConverter = snapshotConverterFunctions<GuestbookEntry
 
 export function guestbookEntryCollectionReferenceFactory(context: FirestoreContext): (guestbook: GuestbookDocument) => CollectionReference<GuestbookEntry> {
   return (guestbook: GuestbookDocument) => {
-    return context.subcollection(guestbook.documentRef, guestbookEntryIdentity.collection).withConverter<GuestbookEntry>(guestbookEntryConverter);
+    return context.subcollection(guestbook.documentRef, guestbookEntryIdentity.collection);
   };
 }
 
@@ -130,6 +131,7 @@ export function guestbookEntryFirestoreCollectionFactory(firestoreContext: Fires
 
   return (parent: GuestbookDocument) => {
     return firestoreContext.firestoreCollectionWithParent({
+      converter: guestbookEntryConverter,
       modelIdentity: guestbookEntryIdentity,
       itemsPerPage: 50,
       collection: factory(parent),
@@ -142,13 +144,14 @@ export function guestbookEntryFirestoreCollectionFactory(firestoreContext: Fires
 }
 
 export function guestbookEntryCollectionReference(context: FirestoreContext): CollectionGroup<GuestbookEntry> {
-  return context.collectionGroup(guestbookEntryIdentity.collection).withConverter<GuestbookEntry>(guestbookEntryConverter);
+  return context.collectionGroup(guestbookEntryIdentity.collection);
 }
 
 export type GuestbookEntryFirestoreCollectionGroup = FirestoreCollectionGroup<GuestbookEntry, GuestbookEntryDocument>;
 
 export function guestbookEntryFirestoreCollectionGroup(firestoreContext: FirestoreContext): GuestbookEntryFirestoreCollectionGroup {
   return firestoreContext.firestoreCollectionGroup({
+    converter: guestbookEntryConverter,
     modelIdentity: guestbookEntryIdentity,
     itemsPerPage: 50,
     accessorFactory: guestbookEntryAccessorFactory,

components/demo-firebase/src/lib/models/profile/profile.ts

@@ -48,14 +48,15 @@ export const profileConverter = snapshotConverterFunctions<Profile>({
 export const profileAccessorFactory = copyUserRelatedDataAccessorFactoryFunction<Profile>();
 
 export function profileCollectionReference(context: FirestoreContext): CollectionReference<Profile> {
-  return context.collection(profileIdentity.collection).withConverter<Profile>(profileConverter);
+  return context.collection(profileIdentity.collection);
 }
 
 export type ProfileFirestoreCollection = FirestoreCollection<Profile, ProfileDocument>;
 
 export function profileFirestoreCollection(firestoreContext: FirestoreContext): ProfileFirestoreCollection {
   return firestoreContext.firestoreCollection({
     modelIdentity: profileIdentity,
+    converter: profileConverter,
     itemsPerPage: 50,
     accessorFactory: profileAccessorFactory,
     collection: profileCollectionReference(firestoreContext),
@@ -97,7 +98,7 @@ export const profilePrivateDataConverter = snapshotConverterFunctions<ProfilePri
 
 export function profilePrivateDataCollectionReferenceFactory(context: FirestoreContext): (profile: ProfileDocument) => CollectionReference<ProfilePrivateData> {
   return (profile: ProfileDocument) => {
-    return context.subcollection(profile.documentRef, profilePrivateDataIdentity.collection).withConverter<ProfilePrivateData>(profilePrivateDataConverter);
+    return context.subcollection(profile.documentRef, profilePrivateDataIdentity.collection);
   };
 }
 
@@ -110,6 +111,7 @@ export function profilePrivateDataFirestoreCollectionFactory(firestoreContext: F
   return (parent: ProfileDocument) => {
     return firestoreContext.singleItemFirestoreCollection({
       modelIdentity: profilePrivateDataIdentity,
+      converter: profilePrivateDataConverter,
       itemsPerPage: 50,
       collection: factory(parent),
       makeDocument: (accessor, documentAccessor) => new ProfilePrivateDataDocument(accessor, documentAccessor),
@@ -121,14 +123,15 @@ export function profilePrivateDataFirestoreCollectionFactory(firestoreContext: F
 }
 
 export function profilePrivateDataCollectionReference(context: FirestoreContext): CollectionGroup<ProfilePrivateData> {
-  return context.collectionGroup(profilePrivateDataIdentity.collection).withConverter<ProfilePrivateData>(profilePrivateDataConverter);
+  return context.collectionGroup(profilePrivateDataIdentity.collection);
 }
 
 export type ProfilePrivateDataFirestoreCollectionGroup = FirestoreCollectionGroup<ProfilePrivateData, ProfilePrivateDataDocument>;
 
 export function profilePrivateDataFirestoreCollectionGroup(firestoreContext: FirestoreContext): ProfilePrivateDataFirestoreCollectionGroup {
   return firestoreContext.firestoreCollectionGroup({
     modelIdentity: profilePrivateDataIdentity,
+    converter: profilePrivateDataConverter,
     itemsPerPage: 50,
     queryLike: profilePrivateDataCollectionReference(firestoreContext),
     makeDocument: (accessor, documentAccessor) => new ProfilePrivateDataDocument(accessor, documentAccessor),

packages/firebase-server/src/lib/firestore/test.firestore.spec.ts

@@ -1,6 +1,6 @@
 import { DocumentReference, Transaction, Firestore } from '@google-cloud/firestore';
 import { DocumentSnapshot, makeFirestoreCollection } from '@dereekb/firebase';
-import { mockItemIdentity, MockItem, mockItemCollectionReference, MockItemDocument, MockItemFirestoreCollection } from '@dereekb/firebase/test';
+import { mockItemIdentity, MockItem, mockItemCollectionReference, MockItemDocument, MockItemFirestoreCollection, mockItemConverter } from '@dereekb/firebase/test';
 import { Maybe } from '@dereekb/util';
 import { adminTestWithMockItemCollection } from '@dereekb/firebase-server/test';
 import { googleCloudFirestoreDrivers } from './driver';
@@ -20,6 +20,7 @@ describe('FirestoreCollection', () => {
         const itemsPerPage = 50;
 
         firestoreCollection = makeFirestoreCollection({
+          converter: mockItemConverter,
           modelIdentity: mockItemIdentity,
           firestoreContext: f.parent.context,
           itemsPerPage,

packages/firebase/src/lib/client/firestore/test.firestore.spec.ts

@@ -1,5 +1,5 @@
 import { DocumentSnapshot, DocumentReference, runTransaction, Transaction, Firestore, writeBatch } from '@firebase/firestore';
-import { MockItem, mockItemCollectionReference, MockItemDocument, MockItemFirestoreCollection, mockItemFirestoreCollection, authorizedTestWithMockItemCollection, mockItemIdentity } from '@dereekb/firebase/test';
+import { MockItem, mockItemCollectionReference, MockItemDocument, MockItemFirestoreCollection, mockItemFirestoreCollection, authorizedTestWithMockItemCollection, mockItemIdentity, mockItemConverter } from '@dereekb/firebase/test';
 import { FirestoreDocumentContext, makeFirestoreCollection } from '../../common/firestore';
 import { transactionDocumentContext } from './driver.accessor.transaction';
 import { Maybe } from '@dereekb/util';
@@ -20,6 +20,7 @@ describe('FirestoreCollection', () => {
         const itemsPerPage = 50;
 
         firestoreCollection = makeFirestoreCollection({
+          converter: mockItemConverter,
           modelIdentity: mockItemIdentity,
           firestoreContext: f.parent.context,
           itemsPerPage,

packages/firebase/src/lib/common/firestore/accessor/accessor.wrap.ts

@@ -45,7 +45,7 @@ export abstract class AbstractFirestoreDocumentDataAccessorWrapper<T, D = Docume
   }
 
   update(data: UpdateData<D>, params?: FirestoreDocumentUpdateParams): Promise<void | WriteResult> {
-    return this.update(data, params);
+    return this.accessor.update(data, params);
   }
 }
 

packages/firebase/src/lib/common/firestore/accessor/document.ts

@@ -6,7 +6,7 @@ import { FirestoreAccessorDriverRef } from '../driver/accessor';
 import { FirestoreModelId, FirestoreModelIdRef, FirestoreModelKey, FirestoreModelKeyRef, FirestoreModelName } from './../collection/collection';
 import { DocumentReference, CollectionReference, Transaction, WriteBatch, DocumentSnapshot, SnapshotOptions, WriteResult } from '../types';
 import { createOrUpdateWithAccessorSet, dataFromSnapshotStream, FirestoreDocumentDataAccessor } from './accessor';
-import { CollectionReferenceRef, DocumentReferenceRef, FirestoreContextReference } from '../reference';
+import { CollectionReferenceRef, DocumentReferenceRef, FirestoreContextReference, FirestoreDataConverterRef } from '../reference';
 import { FirestoreDocumentContext } from './context';
 import { build, cachedGetter, Maybe } from '@dereekb/util';
 import { FirestoreModelNameRef, FirestoreModelIdentity, FirestoreModelIdentityRef } from '../collection/collection';
@@ -155,7 +155,7 @@ export interface LimitedFirestoreDocumentAccessorFactory<T, D extends FirestoreD
 /**
  * FirestoreDocumentAccessor configuration.
  */
-export interface LimitedFirestoreDocumentAccessorFactoryConfig<T, D extends FirestoreDocument<T> = FirestoreDocument<T>> extends FirestoreModelIdentityRef, FirestoreContextReference, FirestoreAccessorDriverRef {
+export interface LimitedFirestoreDocumentAccessorFactoryConfig<T, D extends FirestoreDocument<T> = FirestoreDocument<T>> extends FirestoreDataConverterRef<T>, FirestoreModelIdentityRef, FirestoreContextReference, FirestoreAccessorDriverRef {
   /**
    * Optional InterceptAccessorFactoryFunction to intercept/return a modified accessor factory.
    */
@@ -164,7 +164,7 @@ export interface LimitedFirestoreDocumentAccessorFactoryConfig<T, D extends Fire
 }
 
 export function limitedFirestoreDocumentAccessorFactory<T, D extends FirestoreDocument<T> = FirestoreDocument<T>>(config: LimitedFirestoreDocumentAccessorFactoryConfig<T, D>): LimitedFirestoreDocumentAccessorFactoryFunction<T, D> {
-  const { firestoreContext, firestoreAccessorDriver, makeDocument, accessorFactory: interceptAccessorFactory, modelIdentity } = config;
+  const { firestoreContext, firestoreAccessorDriver, makeDocument, accessorFactory: interceptAccessorFactory, converter, modelIdentity } = config;
   const expectedCollectionName = firestoreAccessorDriver.fuzzedPathForPath ? firestoreAccessorDriver.fuzzedPathForPath(modelIdentity.collection) : modelIdentity.collection;
 
   return (context?: FirestoreDocumentContext<T>) => {
@@ -177,6 +177,7 @@ export function limitedFirestoreDocumentAccessorFactory<T, D extends FirestoreDo
       }
 
       const accessor = dataAccessorFactory.accessorFor(ref);
+
       return makeDocument(accessor, documentAccessor);
     }
 
@@ -187,7 +188,7 @@ export function limitedFirestoreDocumentAccessorFactory<T, D extends FirestoreDo
         throw new Error(`unexpected key/path "${fullPath}" for expected type "${modelIdentity.collection}"/"${modelIdentity.model}".`);
       }
 
-      return ref;
+      return ref.withConverter(converter);
     }
 
     function loadDocumentForKey(fullPath: FirestoreModelKey): D {
@@ -230,11 +231,11 @@ export interface FirestoreDocumentAccessorFactoryConfig<T, D extends FirestoreDo
 }
 
 export function firestoreDocumentAccessorFactory<T, D extends FirestoreDocument<T> = FirestoreDocument<T>>(config: FirestoreDocumentAccessorFactoryConfig<T, D>): FirestoreDocumentAccessorFactoryFunction<T, D> {
-  const { firestoreAccessorDriver, collection } = config;
+  const { firestoreAccessorDriver, collection, converter } = config;
   const limitedFirestoreDocumentAccessor = limitedFirestoreDocumentAccessorFactory(config);
 
-  function documentRefForId(path: string, ...pathSegments: string[]): DocumentReference<T> {
-    return firestoreAccessorDriver.doc(collection, path, ...pathSegments);
+  function documentRefForId(path: string): DocumentReference<T> {
+    return firestoreAccessorDriver.doc(collection, path).withConverter(converter);
   }
 
   return (context?: FirestoreDocumentContext<T>) => {
@@ -250,12 +251,12 @@ export function firestoreDocumentAccessorFactory<T, D extends FirestoreDocument<
 
         x.documentRefForId = documentRefForId;
 
-        x.loadDocumentForId = (path: string, ...pathSegments: string[]): D => {
+        x.loadDocumentForId = (path: string): D => {
           if (!path) {
             throw new Error('Path was not provided to loadDocumentForId(). Use newDocument() for generating an id.');
           }
 
-          return documentAccessor.loadDocument(documentRefForId(path, ...pathSegments));
+          return documentAccessor.loadDocument(documentRefForId(path));
         };
       }
     });

packages/firebase/src/lib/common/firestore/context.ts

@@ -44,15 +44,20 @@ export type FirestoreContextFactory<F extends Firestore = Firestore> = (firestor
  */
 export function firestoreContextFactory<F extends Firestore = Firestore>(drivers: FirestoreDrivers): FirestoreContextFactory<F> {
   return (firestore: F) => {
-    const makeFirestoreCollectionConfig = <T, PT, D extends FirestoreDocument<T> = FirestoreDocument<T>, PD extends FirestoreDocument<PT> = FirestoreDocument<PT>>(config: FirestoreContextFirestoreCollectionConfig<T, D> | FirestoreContextFirestoreCollectionGroupConfig<T, D> | FirestoreContextFirestoreCollectionWithParentConfig<T, PT, D, PD> | FirestoreContextSingleItemFirestoreCollectionConfig<T, PT, D, PD>) => ({
-      ...config,
-      queryLike: (config as FirestoreContextFirestoreCollectionConfig<T, D>).collection ?? (config as FirestoreContextFirestoreCollectionGroupConfig<T, D>).queryLike,
-      firestoreContext: context,
-      driverIdentifier: drivers.driverIdentifier,
-      driverType: drivers.driverType,
-      firestoreQueryDriver: drivers.firestoreQueryDriver,
-      firestoreAccessorDriver: drivers.firestoreAccessorDriver
-    });
+    const makeFirestoreCollectionConfig = <T, PT, D extends FirestoreDocument<T> = FirestoreDocument<T>, PD extends FirestoreDocument<PT> = FirestoreDocument<PT>>(config: FirestoreContextFirestoreCollectionConfig<T, D> | FirestoreContextFirestoreCollectionGroupConfig<T, D> | FirestoreContextFirestoreCollectionWithParentConfig<T, PT, D, PD> | FirestoreContextSingleItemFirestoreCollectionConfig<T, PT, D, PD>) => {
+      const queryLike = (config as FirestoreContextFirestoreCollectionConfig<T, D>).collection ?? (config as FirestoreContextFirestoreCollectionGroupConfig<T, D>).queryLike;
+
+      return {
+        ...config,
+        collection: config.converter ? (config as FirestoreContextFirestoreCollectionConfig<T, D>).collection?.withConverter(config.converter) : (config as FirestoreContextFirestoreCollectionConfig<T, D>).collection,
+        queryLike: config.converter ? queryLike.withConverter(config.converter) : queryLike,
+        firestoreContext: context,
+        driverIdentifier: drivers.driverIdentifier,
+        driverType: drivers.driverType,
+        firestoreQueryDriver: drivers.firestoreQueryDriver,
+        firestoreAccessorDriver: drivers.firestoreAccessorDriver
+      };
+    };
 
     const firestoreCollection = <T, D extends FirestoreDocument<T>>(config: FirestoreContextFirestoreCollectionConfig<T, D>) => makeFirestoreCollection(makeFirestoreCollectionConfig(config) as FirestoreCollectionConfig<T, D>);
     const firestoreCollectionGroup = <T, D extends FirestoreDocument<T>>(config: FirestoreContextFirestoreCollectionGroupConfig<T, D>) => makeFirestoreCollectionGroup(makeFirestoreCollectionConfig(config));

packages/firebase/src/lib/common/firestore/reference.ts

@@ -1,5 +1,5 @@
 import { FirestoreContext } from './context';
-import { CollectionReference, DocumentReference, Firestore, Query, Transaction } from './types';
+import { CollectionReference, DocumentReference, Firestore, FirestoreDataConverter, Query, Transaction } from './types';
 
 /**
  * Contains a reference to a Query.
@@ -35,3 +35,10 @@ export interface FirestoreContextReference<F extends Firestore = Firestore> {
 export interface FirebaseTransactionContext {
   readonly transaction?: Transaction;
 }
+
+/**
+ * Contains contextual information about the current Transaction, if available.
+ */
+export interface FirestoreDataConverterRef<U = unknown> {
+  readonly converter: FirestoreDataConverter<U>;
+}