typeorm-fixture-builder icon indicating copy to clipboard operation
typeorm-fixture-builder copied to clipboard

Fixture installation is failing when a TypeORM entity declares its primary key as a one-to-one or many-to-one relationship and fixture bundle is being installed more than once in the same jest test suite

Open acisser opened this issue 3 years ago • 14 comments

First of all.

@jeanfortheweb, Great Job you have done with this library! IMO this is the best solution for handling fixtures in the Typescript/TypeORM world. I don't know why it only has 26 stars and 880 weekly downloads in npm. Numbers are not fair so far and hoping for more adoption in the future.

Problem Description:

I have a Jest test suite composed of several integration tests running sequentially with --ci --runInBand options, all configured with tear down and setup methods as:

   beforeEach(async () => {
    await clean()
    await install(connection, collect(master_bundle))
    await install(connection, collect(secondary_bundle))
  })

  afterEach(() => {
    clear()
  })

clean() method is basically an SQL script that deletes all data from tables and resets some database sequences

This test suite is also setting up a NestJS e2e app context from where I obtain the TypeORM Connection to apply all fixtures to finally call NestJS Rest API service to be tested.

~The issue I have found is that if for any reason the production code to be tested deletes any of the data being defined and inserted by the fixtures. (I actually have a Postgres CASCADE configuration deleting the whole dependency tree for 4 tables). The next integration test to run (and actually not the test deleting the data) requiring data fixtures that were previously deleted at some point by other tests, will fail with the following stack trace:~

When running the full test suite, will try to install the same fixtures several times. I'm receiving the following error.

No metadata for "Number" was found.
EntityMetadataNotFoundError: No metadata for "Number" was found.
    at EntityMetadataNotFoundError.TypeORMError [as constructor] (/Users/user/Development/src/error/TypeORMError.ts:7:9)
    at new EntityMetadataNotFoundError (/Users/user/Development/src/error/EntityMetadataNotFoundError.ts:7:9)
    at Connection.Object.<anonymous>.Connection.getMetadata (/Users/user/Development/src/connection/Connection.ts:330:19)
    at /Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:109:52
    at step (/Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:33:23)
    at Object.next (/Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:14:53)
    at /Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:8:71
    at new Promise (<anonymous>)
    at Object.<anonymous>.__awaiter (/Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:4:12)
    at persistRelations (/Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:104:12)

Versions being used:

NodeJS: v16.13.0 Jest: 27.4.5 TypeORM: ^0.2.41 Fixture Builder: 2.2.3 NestJS: 8.2.4

Do you have any clue on what's going on, might this actually be a bug?

Please let me know if you need more information.

Thanks & Regards

acisser avatar Jan 31 '22 13:01 acisser

Hey hey @acisser, thanks for your support! Before we dive deeper, have you tried the latest update? You'll find the explanation at the very end of the readme. It added some additional tools especially for unit testing.

If that does not help already, we can surely go deeper, but if thats the case, some kind of reproduction code would be nice.

jeanfortheweb avatar Feb 02 '22 19:02 jeanfortheweb

@jeanfortheweb,

I'm using the latest version of the fixture library. Let's discard that.

While trying to reproduce the issue in a minimalistic project I found, that the root cause was not the one I mention in the original post. (Deletion of records defined by a fixture). It should be something related to a possible unsupported TypeORM model scenario where one of the tables has a composite primary key. Check OneTimeCharge entity and its mappings

Here's the code to reproduce:

https://github.com/acisser/typeorm-fixture-builder-issue-57.git

README.md has got a couple of commands to put things up and running.

The e2e test fails while trying to load the fixtures into the database. If you remove oneTImeCharge fixture from the bundle it will run tests successfully.

Thanks & Regards

acisser avatar Feb 05 '22 17:02 acisser

I am not 100% sure (havent tested the repo yet), but shouldn't you use the actual column names of the relations on your index decorators instead of the properties? That they, just guessing, typeorm would see the metadata correctly. But as I said, just a quick guess which could be right.

jeanfortheweb avatar Feb 10 '22 11:02 jeanfortheweb

From what I can recall, TypeORM always looks for entity field names in that array in the Index declaration, so either you could be pointing to an entity field directly mapped to a foreign key column (that is the common use case) or an entity field mapped to a many to one association that is the case you're referring to.

I think that the index configuration is unrelated since the database is being recreated with no errors making use of synchonized:true option in the TYPEORM connection configuration.

acisser avatar Feb 10 '22 12:02 acisser

Sounds like we gotta dig deeper than! I'll have a look asap

jeanfortheweb avatar Feb 10 '22 13:02 jeanfortheweb

Hi @jeanfortheweb. Any luck digging into this scenario?

acisser avatar Mar 28 '22 13:03 acisser

@jeanfortheweb.

Another scenario I found uses a regular primary key. The problem was reduced to the fact that something really strange happens when having fixtures for entities declaring primary keys as relationships at the same time, plus a combination of installing those fixtures twice in the same Jest test spec file, even using the clear persistence cache method after each test.

import { Column, Entity, JoinColumn, OneToOne, PrimaryColumn } from 'typeorm'
import { ChangeRequest } from '../change-requests/change-request.entity'

@Entity('contract_validity', { schema: 'core' })
export class ContractValidity {
  @PrimaryColumn({ type: 'integer', name: 'id_change_request' })
  @OneToOne(
    () => ChangeRequest,
    (changeRequest) => changeRequest.rateCardItems,
    { onDelete: 'RESTRICT', onUpdate: 'CASCADE' },
  )
  @JoinColumn([{ name: 'id_change_request', referencedColumnName: 'id' }])
  changeRequest: ChangeRequest

  @Column('date', { name: 'end_date' })
  endDate: string
}

Would a PR be welcome to fix this? I would like to exhaust the possibilities of fixing it to avoid mixing fixtures with raw SQL fixtures as a workaround for this problem.

acisser avatar Aug 04 '22 13:08 acisser

Hello @acisser, unfortunately, I really had not the time to look into any github related work. Im sorry for this. Im wondering whats going on too, since the CLI will just persist the assigned related entity and take it's id (like code based cascades). PR's surely are always welcome!

jeanfortheweb avatar Aug 19 '22 14:08 jeanfortheweb

Hi @jeanfortheweb

I spent some time today extending the complex bundle coming in tests with an additional entity called project. This new entity has got fields that are foreign key and primary key at the same time

import { Entity, JoinColumn, ManyToOne, PrimaryColumn } from 'typeorm';
import { User } from './user';
import { Picture } from './picture';

@Entity()
export class Project {
  @PrimaryColumn({ type: 'integer', name: 'id_user' })
  @ManyToOne(() => User, (user) => user.projects)
  @JoinColumn([{ name: 'id_user', referencedColumnName: 'id' }])
  user: User;

  @PrimaryColumn({ type: 'integer', name: 'id_picture' })
  @ManyToOne(() => Picture, (picture) => picture.projects)
  @JoinColumn([{ name: 'id_picture', referencedColumnName: 'id' }])
  picture: Picture;
}

The actual problem is in the repository.save method

/**
 * Persists the fixture itself.
 *
 * @param manager EntityManager.
 * @param fixture Fixture.
 */
export async function persistEntity(
  manager: EntityManager,
  fixture: Record<string, unknown>,
): Promise<void> {
  return manager
    .getRepository(fixture.constructor)
    .save(await resolve(manager, fixture));
}

This repository save method is receiving an entity as a parameter like:

{ user: {id: 1, firstName: 'User'}, picture: {id: 1, file:'test.txt'} }

then, there's an internal mutation occurring in TypeORM that converts this object instance to something different:

{ user: 1, picture: 1 }

This mutation TypeORM is doing to the entity is directly affecting the exports of fixture definitions, causing big problems in a second installation of the fixtures because the original fixtures that were declared as modules are now completely mutated to something different.

A side effect is that rest of the code gets broken with errors like:

No metadata for "Number" was found.
EntityMetadataNotFoundError: No metadata for "Number" was found.
    at EntityMetadataNotFoundError.TypeORMError [as constructor] (/Users/user/Development/src/error/TypeORMError.ts:7:9)
    at new EntityMetadataNotFoundError (/Users/user/Development/src/error/EntityMetadataNotFoundError.ts:7:9)
    at Connection.Object.<anonymous>.Connection.getMetadata (/Users/user/Development/src/connection/Connection.ts:330:19)
    at /Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:109:52
    at step (/Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:33:23)
    at Object.next (/Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:14:53)
    at /Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:8:71
    at new Promise (<anonymous>)
    at Object.<anonymous>.__awaiter (/Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:4:12)
    at persistRelations (/Users/user/Development/node_modules/typeorm-fixture-builder/lib/cjs/persist.js:104:12)

I started playing with the source code trying to clone the fixture/entity right before saving it with the repository, however, I am unsure of how this could be achieved in a generic fashion.

Was the code designed expecting TypeORM to mutate entities by adding generated id's to it? Maybe cloning is not the way to go and perhaps this is a pure TypeORM problem that should be reported on its respective repository?

Any clues?

Thanks and Regards

acisser avatar Oct 24 '22 02:10 acisser

Here's the fork that I used to extend one of the test scenarios

https://github.com/acisser/typeorm-fixture-builder

acisser avatar Oct 24 '22 02:10 acisser

@jeanfortheweb Submitted PR #67 with the fix. Please let me know any comments.

acisser avatar Oct 24 '22 06:10 acisser

Hey @acisser, im sorry u always have to wait so long. life is all but easy in the moment. Afaik, TypeORM does mutate entity instances, which should be more seen as a feature than a bug, since entities are actually instances of that class (or should be). otherwise, of course, it could not find any metadata when saving. Therefore, normally, after persisting the entity, it should always have an id assigned.

This is why the fixture builder goes from leaves to branches, so that ids are always present when persisting parent entities which relate to them. This was actually always part of the e2e tests and used to work just fine.

Or did i miss something?

By the way

if(id){
    return {...fixture, ...id}
  }

Would break all this, since after this spread its not a class instance anymore

jeanfortheweb avatar Nov 08 '22 11:11 jeanfortheweb

Hi @jeanfortheweb, thanks for taking some time to check this.

After looking for code usages of "resolve" function (that is the function containing the code you mentioned above), it turns out that the returned value of this is only being used to resolve the object to be passed to TypeORM repository.save method.

TypeORM repository save method supports both sending Entity instances or literal objects. The repository declaration should be the one that holds a reference of the entity type to make use of reflection in order to convert the object literal to SQL statements.

I don't think this will break the current behavior and the fact that jest tests are passing gives me some level of confidence.

Anyway, I could submit this slight change if you think it makes more sense to you.

import {fixture as clonedFixture} from './fixture'
...
const id = repository.getId(fixture);
  if(id){
    return clonedFixture(fixture.constructor as any,{...fixture, ...id})
  }

In tests "id" is returning truthy values only for entities being declared with composite primary key, such as the entity introduced in the PR to verify this change.

Please let me know.

acisser avatar Nov 08 '22 13:11 acisser

@jeanfortheweb Any chance to give some solution to this problem with backward compatibility for TypeOrm 2?

acisser avatar Dec 08 '22 00:12 acisser