Feature/closure table custom naming #7120

alexey2baranov · 2020-11-29T00:59:19Z

Description of change

This PR Implements custom naming for closure-table tree.
Closes feature request #7068

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

nebkat

Some minor comments there but overall nice change! Make sure you also follow the commit message guidelines. You can squash all of these commits into a single one called feat: Closure table custom naming.

src/decorator/tree/Tree.ts

nebkat · 2020-11-29T01:07:35Z

src/metadata-builder/ClosureJunctionEntityMetadataBuilder.ts

@@ -32,7 +32,7 @@ export class ClosureJunctionEntityMetadataBuilder {
            connection: this.connection,
            args: {
                target: "",
-                name: parentClosureEntityMetadata.tableNameWithoutPrefix,
+                name: parentClosureEntityMetadata.treeOptions && parentClosureEntityMetadata.treeOptions.closureTableName ? parentClosureEntityMetadata.treeOptions.closureTableName : parentClosureEntityMetadata.tableNameWithoutPrefix,


This and the column names should probably be extracted to a variable since they are very long.

This might not have been the intended change here, see also

typeorm/src/metadata/EntityMetadata.ts

Line 792 in c4a36da

this.tableNameWithoutPrefix = namingStrategy.closureJunctionTableName(this.givenTableName!);

This and the column names should probably be extracted to a variable since they are very long.

Yes, looks very long. Very soon it will be shorten twice and looks good for reading.

parentClosureEntityMetadata?.treeOptions?.ancestorColumnName?.(primaryColumn) || primaryColumn.propertyName + "_ancestor"

What do you think? Maybe we stay it as is to prevent rollback refactor into single command?

This might not have been the intended change here, see also

typeorm/src/metadata/EntityMetadata.ts

Line 792 in c4a36da

this.tableNameWithoutPrefix = namingStrategy.closureJunctionTableName(this.givenTableName!);

I prefer to skip NamingStrategy transformation if closureTableName was set explicitly and use transformation if it was not. What do you think?

src/metadata-builder/ClosureJunctionEntityMetadataBuilder.ts

nebkat · 2020-11-29T01:14:05Z

test/github-issues/7068/issue-7068.ts

+import {Connection} from "../../../src/connection/Connection";
+import {closeTestingConnections, createTestingConnections, reloadTestingDatabases} from "../../utils/test-utils";
+
+describe.only("github issues > #7068", () => {


Probably excessive, to have a copy of all of the old tests.

Just try access the EntityMetadata from connection by filtering for one with the correct name as expected with your configration i.e. "category_xyz_closure". Then check it's columns property to have columns named "ancestor_xyz_" ...

IMHO metadata checks doesn't cover real work. One time i saw SubjectExecutor ignores metadata naming. Maybe other parts of code do the same. Are you shure it's good idea to reduce tests?

https://github.com/typeorm/typeorm/blob/master/src/persistence/tree/ClosureSubjectExecutor.ts#L79

alexey2baranov

You can squash all of these commits into a single one called feat: Closure table custom naming.

Yes! I would like to do this. How can I do this?

alexey2baranov · 2020-11-29T08:54:28Z

src/metadata-builder/ClosureJunctionEntityMetadataBuilder.ts

@@ -32,7 +32,7 @@ export class ClosureJunctionEntityMetadataBuilder {
            connection: this.connection,
            args: {
                target: "",
-                name: parentClosureEntityMetadata.tableNameWithoutPrefix,
+                name: parentClosureEntityMetadata.treeOptions && parentClosureEntityMetadata.treeOptions.closureTableName ? parentClosureEntityMetadata.treeOptions.closureTableName : parentClosureEntityMetadata.tableNameWithoutPrefix,


This and the column names should probably be extracted to a variable since they are very long.

Yes, looks very long. Very soon it will be shorten twice and looks good for reading.

parentClosureEntityMetadata?.treeOptions?.ancestorColumnName?.(primaryColumn) || primaryColumn.propertyName + "_ancestor"

What do you think? Maybe we stay it as is to prevent rollback refactor into single command?

alexey2baranov · 2020-11-29T08:57:25Z

src/metadata-builder/ClosureJunctionEntityMetadataBuilder.ts

@@ -32,7 +32,7 @@ export class ClosureJunctionEntityMetadataBuilder {
            connection: this.connection,
            args: {
                target: "",
-                name: parentClosureEntityMetadata.tableNameWithoutPrefix,
+                name: parentClosureEntityMetadata.treeOptions && parentClosureEntityMetadata.treeOptions.closureTableName ? parentClosureEntityMetadata.treeOptions.closureTableName : parentClosureEntityMetadata.tableNameWithoutPrefix,


This might not have been the intended change here, see also

typeorm/src/metadata/EntityMetadata.ts

Line 792 in c4a36da

this.tableNameWithoutPrefix = namingStrategy.closureJunctionTableName(this.givenTableName!);

I prefer to skip NamingStrategy transformation if closureTableName was set explicitly and use transformation if it was not. What do you think?

src/metadata-builder/ClosureJunctionEntityMetadataBuilder.ts

pleerock · 2021-01-12T09:25:54Z

I think this PR can be merged. Thank you for contributions!

* typeorm-0.2.30: (212 commits) version bump docs: fix javascript usage examples (typeorm#7031) fix: resolve migration for UpdateDateColumn without ON UPDATE clause (typeorm#7057) fix: Error when sorting by an embedded entity while using join and skip/take (typeorm#7082) fix: resolves Postgres sequence identifier length error (typeorm#7115) feat: closure table custom naming (typeorm#7120) feat: relations: Orphaned row action (typeorm#7105) docs: fix invalid code block in "find many options" (typeorm#7268) docs: Embodying the example (typeorm#7116) docs: document withDeleted option (typeorm#7132) fix: return 'null' (instead of 'undefined') on lazy relations that have no results (typeorm#7146) (typeorm#7147) docs: update cascade options (typeorm#7140) docs: add .ts to supported ormconfig formats (typeorm#7139) fix: improve stack traces when using persist executor (typeorm#7218) refactor: remove Oracle multirow insert workaround (since typeorm#6927) (typeorm#7083) feat: add NOWAIT and SKIP LOCKED lock support for MySQL (typeorm#7236) docs: update OneToMany grammar (typeorm#7252) feat: JavaScript file migrations output (typeorm#7253) docs: update Repository.ts (typeorm#7254) chore: update dependency cli-highlight to v2.1.10 (typeorm#7265) ...

alexey2baranov added 10 commits November 28, 2020 17:11

add metadata to @Tree

ae7b3d6

add simple test

3fd962a

ref column naming from string to callback(column)=>name

b64c552

fix Closure junction metadata builder

5764255

fix ClosureSubjectExecutor columns naming

52525ae

ref ClosureSubjectExecutor columns naming

9e0febc

fix ClosureEntityMetadataBuilder

460edef

add tests

24d47bd

fix test name

5f13ce6

doc: add closure-table custom naming docs.

784e0b3

nebkat suggested changes Nov 29, 2020

View reviewed changes

alexey2baranov added 2 commits November 29, 2020 11:32

refactor: small code-style changing.

ae37115

refactor: minor refactoring during code review

b659487

alexey2baranov commented Nov 29, 2020

View reviewed changes

pleerock merged commit bcd998b into typeorm:master Jan 12, 2021

pleerock mentioned this pull request Jan 12, 2021

Custom column names for closure tree table #7068

Closed

tgandrews mentioned this pull request Jan 16, 2021

Bug - Test suite only running one test #7284

Closed

4 tasks

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Feature/closure table custom naming #7120

Feature/closure table custom naming #7120

alexey2baranov commented Nov 29, 2020

nebkat left a comment

nebkat Nov 29, 2020

nebkat Nov 29, 2020

alexey2baranov Nov 29, 2020 •

edited

alexey2baranov Nov 29, 2020

nebkat Nov 29, 2020

alexey2baranov Nov 29, 2020

alexey2baranov left a comment

alexey2baranov Nov 29, 2020 •

edited

alexey2baranov Nov 29, 2020

pleerock commented Jan 12, 2021

Feature/closure table custom naming #7120

Feature/closure table custom naming #7120

Conversation

alexey2baranov commented Nov 29, 2020

Description of change

Pull-Request Checklist

nebkat left a comment

Choose a reason for hiding this comment

nebkat Nov 29, 2020

Choose a reason for hiding this comment

nebkat Nov 29, 2020

Choose a reason for hiding this comment

alexey2baranov Nov 29, 2020 • edited

Choose a reason for hiding this comment

alexey2baranov Nov 29, 2020

Choose a reason for hiding this comment

nebkat Nov 29, 2020

Choose a reason for hiding this comment

alexey2baranov Nov 29, 2020

Choose a reason for hiding this comment

alexey2baranov left a comment

Choose a reason for hiding this comment

alexey2baranov Nov 29, 2020 • edited

Choose a reason for hiding this comment

alexey2baranov Nov 29, 2020

Choose a reason for hiding this comment

pleerock commented Jan 12, 2021

alexey2baranov Nov 29, 2020 •

edited

alexey2baranov Nov 29, 2020 •

edited