feat: add tree entities update and delete logic

[mateussilva92]

Allows tree entities relations to be updated and deleted via the save function from the Repository

Closes: #7155

Description of change

Main changes are done to main SubjectExecutor.ts and all the tree executors.

I added logic to all the tree executors to allow tree relation changes via the update function and also added the delete logic to NestedSetSubjectExecutor.ts.
On the SubjectExecutor.ts the biggest change done was to run the nested set updates in series instead of parallel just like all the other entities updates are done. The reason for that is explained in #7155.
Enable onDelete option for TreeParent in all RDBMS except for mssql.
Also cascade delete for closure junction table is also enabled to all RDBMS and as custom code to add the same behaviour in mssql.

All the other file changes are either small format changes, code cleanup or some changes that allow the main new feature to work.

Fixes #193
Fixes #2032
Fixes #2418
Fixes #3691
Fixes #3988
Fixes #4174
Fixes #4353
Fixes #5268
Fixes #6118

Duplicated by #3556

Note:
The development also prevents the creation of more than one root entity for nested sets I'm not sure if that's an approach that you would like to have or not.

Pull-Request Checklist

• Code is up-to-date with the master branch
• npm run lint passes with this change
• npm run test passes with this change
• This pull request links relevant issues as Fixes #0000
• There are new or updated unit tests validating the change
• Documentation has been updated to reflect this change
• The new commits follow conventions explained in CONTRIBUTING.md

[boydaihungst]

I got Cannot read property 'find' of undefined error when used with TableInheritance.

FileFolderBase

@Entity('file_folder')
@TableInheritance({ column: { type: 'varchar', name: 'type' } })
@Tree('closure-table')
export class FileFolderBase extends CustomBaseEntity {
  @PrimaryGeneratedColumn('uuid')
  id: string;

  @Column({ type: 'nvarchar', length: 255, nullable: false })
  @MaxLength(255, { message: DbError.FIELD_LENGTH_MAX })
  name: string;

  @TreeChildren({ cascade: ['soft-remove', 'remove', 'recover'] })
  childrens: FileFolderBase[];

  @TreeParent({ onDelete: 'CASCADE' })
  parentFolder: FileFolderBase;

  @OneToOne(() => FileStatistic, (x) => x.file, {
    onDelete: 'CASCADE',
    cascade: true,
    nullable: true,
  })
  @JoinColumn()
  statistic: FileStatistic;
}

Folder

@ChildEntity()
@Tree('closure-table')
export class Folder extends FileFolderBase {
  @Column({ type: 'boolean', nullable: false, default: false })
  isRoot?: boolean;

  @Column({ type: 'varchar', length: 45, nullable: true })
  color?: string;
}

FileStatistic

@Entity()
export class FileStatistic extends CustomBaseEntity {
  @PrimaryGeneratedColumn('uuid')
  id: string;

  @Column({
    type: 'bigint',
    nullable: false,
    unsigned: true,
    default: 0,
    transformer: {
      from: (val: any) => BigInt(val || '0'),
      to: (val: bigint) => val?.toString(),
    },
  })
  downloaded: bigint;

  @Column({
    type: 'bigint',
    nullable: false,
    unsigned: true,
    default: 0,
    transformer: {
      from: (val: any) => BigInt(val || '0'),
      to: (val: bigint) => val?.toString(),
    },
  })
  viewed?: bigint;

  @OneToOne(() => FileFolderBase, (x) => x.statistic, {
    nullable: false,
    primary: true,
    onDelete: 'CASCADE',
  })
  file: FileFolderBase;
}

This is how I create and save a new folder.

    const result = await this.folderRepository.save(
      this.folderRepository.create({
        name: `ROOT/${userId}`,
        isRoot: true,
        statistic: { 
          downloaded: BigInt(0),
          viewed: BigInt(0),
        },
      }),
    );

Error at line 70

typeorm/src/persistence/tree/ClosureSubjectExecutor.ts

Lines 67 to 73 in 1358dfb

	let entity = subject.databaseEntity; // if entity was attached via parent
	if (!entity) // if entity was attached via children
	entity = subject.metadata.treeChildrenRelation!.getEntityValue(parent).find((child: any) => {
	return Object.entries(subject.identifier!).every(([key, value]) => child[key] === value);
	});

And if I check undefined its works normally

        if (!entity) {
            // if entity was attached via children
            const chilrens = subject.metadata.treeChildrenRelation!.getEntityValue(
                parent
            );
            if (!!chilrens)
                entity = chilrens!.find((child: any) => {
                    return Object.entries(subject.identifier!).every(
                        ([key, value]) => child[key] === value
                    );
                });
        }

[mateussilva92]

@boydaihungst
Going to fix that later on and add some new tests to prevent that from happening again. Thanks for the feedback.

[mateussilva92]

@boydaihungst
I tried to replicate the issue on my side but I was not able to, can you please tell me what database are you using and also share the code that you use to update your entity since the one you share is to do the inicial save which should not go through the update flow.

[boydaihungst]

@mateussilva92
Sorry for the lack of information. I use Mysql. And I updated the comment above. I think the problem is I use cascade: true for statistic property

statistic: { 
   downloaded: BigInt(0),
   viewed: BigInt(0),
}

[mateussilva92]

@boydaihungst
Still can't replicate even after the update you gave.
I don't think it's the cascade: true on the statistic because that entity is not even a TreeEntity.
Is there any change you could provide a minimal repo with all the code to help me replicate the issue?

[boydaihungst]

@mateussilva92
Here is minimal repo
https://github.com/boydaihungst/typeorm-tree-error

[mateussilva92]

@boydaihungst
Thanks for the repo, it really helped. The issue is now fixed.

[kauandotnet]

@pleerock
Please, check this! I'm working for a project that need it.

Thank you!

[phil294]

Hi @mateussilva92, thanks for doing this! I've been using it for a few weeks in production with success. There is one issue however: Setting a parent property to null and saving the entity afterwards leads to

error: "CannotAttachTreeChildrenEntityError: Cannot attach entity \"Post\" to its parent. Please make sure parent is saved in the database before saving children nodes.
    at new CannotAttachTreeChildrenEntityError (/project/src/error/CannotAttachTreeChildrenEntityError.ts:8:9)
    at /project/src/persistence/tree/ClosureSubjectExecutor.ts:251:23
    at Array.map (<anonymous>)
    at ClosureSubjectExecutor.<anonymous> (/project/src/persistence/tree/ClosureSubjectExecutor.ts:246:88)
    at step (/project/node_modules/tslib/tslib.js:141:27)
    at Object.next (/project/node_modules/tslib/tslib.js:122:57)
    at /project/node_modules/tslib/tslib.js:115:75
    at new Promise (<anonymous>)
    at Object.__awaiter (/project/node_modules/tslib/tslib.js:111:16)
    at ClosureSubjectExecutor.insertClosureEntry (/project/node_modules/typeorm/persistence/tree/ClosureSubjectExecutor.js:242:24)"

Setting the parentId field to null instead does not throw an error, but competely fails to drop the relevant closure table entries, leading to faulty state. So there seem to be two bugs here.
Does this make sense / am I overseeing something?
If it should work, please tell me so I can create a reproduction repository.

[phil294]

Actually, I am not sure about my previous comment. I did get this error message, but not consistently :-/
However, I do have another bug report for your PR: Setting a parent property to null, saving, and then setting it back to some value fails to insert closure table records for nested grandchildren. I made a test repo to reproduce @ https://github.com/phil294/typeorm-tree-entities-PR-bug. The pseudo test case at index.ts should be pretty clear. Hope this helps.

What would be the expected workflow for fixing faulty tree table states? For example, the closure table state and the actual parent relations can diverge, either because of bugs like the mentioned one or when you edited the tables manually – now they're out of sync. Would a treeRepo.rebuildClosureTable() or the like make sense?

[mateussilva92]

Hey @phil294 thanks for the good catch and for the awesome feedback along side the repo to help visualize the bug.
I'll try to fix the issue on the next few days.

[AlexMesser]

try ...catch can be replaced with hasMetadata(table.name) condition

[mateussilva92]

Thanks for the catch, fixed.

[AlexMesser]

thank you for contribution!