Repository: typedorm/typedorm (ts)

Issue:
bug: findOne returns an object when it should not
Howdy!

I've been using TypeDORM in one of my projects recently, and it has been good so far! Except for one problem: even when there is no item with a primary key in a table, EntityManager will always return an object (although empty). This breaks some other things, like `EntityManager.exists` and other places in the library that may depend on those functions somehow.

https://github.com/typedorm/typedorm/blob/6ccc93e376905a82412f09d7bb62f29bb5ea57fb/packages/core/src/classes/manager/entity-manager.ts#L153-L158

In `EntityManager.ts:155`, even though `response.Item` may be undefined, the implementation disregards that non-existence and just forces object conversion (to the desired entity class) with an empty object. The result is that the function does not really comply with its interface, and the `Promise<Entity | undefined>` return type is really just `Promise<Entity | {}>`, which is... not ideal, to say the least (as it will potentially and inadvertently break invariants of the entity class, which the library have no knowledge of, for obvious reasons).

In my application, I use `EntityManager.exists` with a primary key (PK + SK) to check for the existence of an entity, and to determine whether to call create or update. But because `EntityManager.findOne` always returns an object even when the primary key does not exist, `EntityManager.exists` in turn will always return true:
 https://github.com/typedorm/typedorm/blob/6ccc93e376905a82412f09d7bb62f29bb5ea57fb/packages/core/src/classes/manager/entity-manager.ts#L215-L217

Perhaps `EntityManager.findOne`'s implementation should really end up like so:

```ts
    // ...
    if (response.Item) {
      const entity = this._entityTransformer.fromDynamoEntity<Entity>(
        entityClass,
        response.Item
      );
      return entity;
    }
    return undefined;
  }
```

But I may be wrong, hence why I opened this issue. Is my analysis correct?

Pull request:
fix(core): return undefined instead of empty object
# Changes
- `EntityManage.findOne` will now return `undefined` when the item with a given primary key does not eixst.

closes #110 

Patch:
diff --git a/packages/core/src/classes/manager/entity-manager.ts b/packages/core/src/classes/manager/entity-manager.ts
index 704d359..d662241 100644
--- a/packages/core/src/classes/manager/entity-manager.ts
+++ b/packages/core/src/classes/manager/entity-manager.ts
@@ -150,10 +150,16 @@ export class EntityManager {
       .get(dynamoGetItem)
       .promise();
 
+    // return early when not found
+    if (!response.Item) {
+      return;
+    }
+
     const entity = this._entityTransformer.fromDynamoEntity<Entity>(
       entityClass,
-      response.Item ?? {}
+      response.Item
     );
+
     return entity;
   }
 


Test patch:
diff --git a/packages/core/src/classes/manager/__test__/entity-manager.spec.ts b/packages/core/src/classes/manager/__test__/entity-manager.spec.ts
index b348888..f1c528d 100644
--- a/packages/core/src/classes/manager/__test__/entity-manager.spec.ts
+++ b/packages/core/src/classes/manager/__test__/entity-manager.spec.ts
@@ -232,6 +232,27 @@ test('finds one entity by given primary key', async () => {
   });
 });
 
+// issue: 110
+test('returns undefined when no item was found with given primary key', async () => {
+  dcMock.get.mockReturnValue({
+    promise: () => ({}),
+  });
+
+  const userEntity = await manager.findOne<User, UserPrimaryKey>(User, {
+    id: '1',
+  });
+
+  expect(dcMock.get).toHaveBeenCalledTimes(1);
+  expect(dcMock.get).toHaveBeenCalledWith({
+    Key: {
+      PK: 'USER#1',
+      SK: 'USER#1',
+    },
+    TableName: 'test-table',
+  });
+  expect(userEntity).toBeUndefined();
+});
+
 test('throws an error when trying to do a get request with non primary key attributes', async () => {
   await expect(
     manager.findOne(User, {
@@ -277,7 +298,39 @@ test('checks if given item exists', async () => {
   expect(userEntity).toEqual(true);
 });
 
+// issue: 110
+test('returns correct value when trying check existence of item that does not exist', async () => {
+  dcMock.get.mockReturnValue({
+    promise: () => ({}),
+  });
+
+  const userEntity = await manager.exists<User, UserUniqueEmailPrimaryKey>(
+    User,
+    {
+      id: '1',
+    }
+  );
+
+  expect(dcMock.get).toHaveBeenCalledWith({
+    Key: {
+      PK: 'USER#1',
+      SK: 'USER#1',
+    },
+    TableName: 'test-table',
+  });
+  expect(userEntity).toEqual(false);
+});
+
 test('checks if item with given unique attribute exists', async () => {
+  dcMock.get.mockReturnValue({
+    promise: () => ({
+      Item: {
+        PK: 'DRM_GEN_USERUNIQUEEMAIL.EMAIL#user@example.com',
+        SK: 'DRM_GEN_USERUNIQUEEMAIL.EMAIL#user@example.com',
+      },
+    }),
+  });
+
   const userEntity = await manager.exists<UserUniqueEmail>(UserUniqueEmail, {
     email: 'user@example.com',
   });