chore: apply comments

Author: mrnagydavid

src/commondao/common.dao.model.ts

@@ -285,20 +285,30 @@ export interface CommonDaoPatchByIdOptions<DBM extends BaseDBEntity>
   extends CommonDaoSaveBatchOptions<DBM> {
   /**
    * Defaults to false.
-   * With false, if the row doesn't exist - it will be auto-created with `dao.create`.
-   * With true, if the row doesn't exist - it will throw an error.
+   * With false, if the row doesn't exist - it will throw an error.
+   * With true, if the row doesn't exist - it will be auto-created with `dao.create`.
    *
    * Use true when you expect the row to exist and it would be an error if it doesn't.
    */
-  requireToExist?: boolean
+  createIfMissing?: boolean
 }
 
 export interface CommonDaoPatchOptions<DBM extends BaseDBEntity>
   extends CommonDaoSaveBatchOptions<DBM> {
   /**
    * If true - patch will skip loading from DB, and will just optimistically patch passed object.
+   *
+   * Consequently, when the row doesn't exist - it will be auto-created with `dao.create`.
    */
   skipDBRead?: boolean
+  /**
+   * Defaults to false.
+   * With false, if the row doesn't exist - it will throw an error.
+   * With true, if the row doesn't exist - it will be auto-created with `dao.create`.
+   *
+   * Use true when you expect the row to exist and it would be an error if it doesn't.
+   */
+  createIfMissing?: boolean
 }
 
 /**

src/commondao/common.dao.test.ts

@@ -156,6 +156,14 @@ test('should propagate pipe errors', async () => {
 
 test('patchById', async () => {
   const id = '123456'
+  const testItem: TestItemBM = {
+    id,
+    k1: 'k1',
+    created: 1529539200 as UnixTimestamp,
+    updated: 1529539200 as UnixTimestamp,
+  }
+  await dao.save(testItem)
+
   const r = await dao.patchById(id, {
     k1: 'k111',
   })
@@ -173,7 +181,7 @@ test('patchById', async () => {
   `)
 })
 
-test('patchById requireToExist', async () => {
+test('patchById createIfMissing false', async () => {
   const id = '123456'
   expect(
     await pExpectedErrorString(
@@ -183,11 +191,11 @@ test('patchById requireToExist', async () => {
           k1: 'k111',
         },
         {
-          requireToExist: true,
+          createIfMissing: false,
         },
       ),
     ),
-  ).toMatchInlineSnapshot(`"AssertionError: TEST_TABLE.patchById row is required, but missing"`)
+  ).toMatchInlineSnapshot(`"AssertionError: DB row required, but not found in TEST_TABLE"`)
 })
 
 describe('patch', () => {
@@ -222,10 +230,10 @@ describe('patch', () => {
       }),
     )
 
-    expect(error).toBe('AppError: DB row required, but not found in TEST_TABLE')
+    expect(error).toBe(`AssertionError: DB row required, but not found in TEST_TABLE`)
   })
 
-  test('should not throw when data does not exist but `skipDBRead` is specified', async () => {
+  test('should create the data when it does not exist and `skipDBRead` is specified', async () => {
     const testItem: TestItemBM = {
       id: 'id1',
       k1: 'k1',
@@ -246,6 +254,55 @@ describe('patch', () => {
     const updatedTestItem = await dao.requireById('id1')
     expect(updatedTestItem).toMatchObject({ k1: 'k111' })
   })
+
+  test('should create the data when it does not exist and `createIfMissing` is specified', async () => {
+    const testItem: TestItemBM = {
+      id: 'id1',
+      k1: 'k1',
+      created: 1529539200 as UnixTimestamp,
+      updated: 1529539200 as UnixTimestamp,
+    }
+
+    await dao.patch(
+      testItem,
+      {
+        k1: 'k111',
+      },
+      {
+        createIfMissing: true,
+      },
+    )
+
+    const updatedTestItem = await dao.requireById('id1')
+    expect(updatedTestItem).toMatchObject({ k1: 'k111' })
+  })
+
+  test('should throw when `skipDBRead` is true and `createIfMissing` is `false`', async () => {
+    const testItem: TestItemBM = {
+      id: 'id1',
+      k1: 'k1',
+      created: 1529539200 as UnixTimestamp,
+      updated: 1529539200 as UnixTimestamp,
+    }
+    await dao.save(testItem)
+
+    const error = await pExpectedErrorString(
+      dao.patch(
+        testItem,
+        {
+          k1: 'k111',
+        },
+        {
+          skipDBRead: true,
+          createIfMissing: false,
+        },
+      ),
+    )
+
+    expect(error).toBe(
+      'AssertionError: When `skipDBRead` is set to `true`, `createIfMissing` must not be `false`',
+    )
+  })
 })
 
 test('patch', async () => {

src/commondao/common.dao.ts

@@ -679,7 +679,7 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM, I
       }
     } else {
       const table = opt.table || this.cfg.table
-      _assert(!opt.requireToExist, `${table}.patchById row is required, but missing`, {
+      _assert(opt.createIfMissing, `DB row required, but not found in ${table}`, {
         id,
         table,
       })
@@ -717,6 +717,10 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM, I
     }
 
     if (opt.skipDBRead) {
+      _assert(
+        opt.createIfMissing === undefined || opt.createIfMissing,
+        'When `skipDBRead` is set to `true`, `createIfMissing` must not be `false`',
+      )
       const patched: BM = {
         ...bm,
         ...patch,
@@ -728,24 +732,33 @@ export class CommonDao<BM extends BaseDBEntity, DBM extends BaseDBEntity = BM, I
       }
       Object.assign(bm, patch)
     } else {
-      const loaded = await this.requireById(bm.id as ID, {
+      const loaded = await this.getById(bm.id as ID, {
         // Skipping validation here for performance reasons.
         // Validation is going to happen on save anyway, just down below.
         skipValidation: true,
         ...opt,
       })
 
-      const loadedWithPatch: BM = {
-        ...loaded,
-        ...patch,
-      }
+      if (loaded) {
+        const loadedWithPatch: BM = {
+          ...loaded,
+          ...patch,
+        }
 
-      // Make `bm` exactly the same as `loadedWithPatch`
-      _objectAssignExact(bm, loadedWithPatch)
+        // Make `bm` exactly the same as `loadedWithPatch`
+        _objectAssignExact(bm, loadedWithPatch)
 
-      if (_deepJsonEquals(loaded, loadedWithPatch)) {
-        // Skipping the save operation, as data is the same
-        return bm
+        if (_deepJsonEquals(loaded, loadedWithPatch)) {
+          // Skipping the save operation, as data is the same
+          return bm
+        }
+      } else {
+        const table = opt.table || this.cfg.table
+        _assert(opt.createIfMissing, `DB row required, but not found in ${table}`, {
+          id: bm.id,
+          table,
+        })
+        Object.assign(bm, patch)
       }
     }