NIFI-11858 Configurable Column Name Normalization in PutDatabaseRecord and UpdateDatabaseTable

[ravinarayansingh]

Summary

NiFi 11858

Tracking

Please complete the following tracking steps prior to pull request creation.

Issue Tracking

• [ ] Apache NiFi Jira issue created

Pull Request Tracking

• [ ] Pull Request title starts with Apache NiFi Jira issue number, such as NIFI-00000
• [ ] Pull Request commit message starts with Apache NiFi Jira issue number, as such NIFI-00000

Pull Request Formatting

• [ ] Pull Request based on current revision of the main branch
• [ ] Pull Request refers to a feature branch with one commit containing changes

Verification

Please indicate the verification steps performed prior to pull request creation.

Build

• [ ] Build completed using mvn clean install -P contrib-check
  • [ ] JDK 17

Licensing

• [ ] New dependencies are compatible with the Apache License 2.0 according to the License Policy
• [ ] New dependencies are documented in applicable LICENSE and NOTICE files

Documentation

• [ ] Documentation formatting appears as expected in rendered files

[ravinarayansingh]

Hi @exceptionfactory ,

following is the rat report:

---

Summary

Generated at: 2023-08-01T08:57:42-07:00

Notes: 24 Binaries: 5 Archives: 3 Standards: 27

Apache Licensed: 13 Generated Documents: 0

JavaDocs are generated, thus a license header is optional. Generated files do not require license headers.

14 Unknown Licenses

---

Files with unapproved licenses:

nifi-h2/nifi-h2-database/nifi-h2-database.iml nifi-h2/nifi-h2-database/target/.plxarc nifi-h2/nifi-h2-database/target/maven-archiver/pom.properties nifi-h2/nifi-h2-database-migrator/nifi-h2-database-migrator.iml nifi-h2/nifi-h2-database-migrator/target/.plxarc nifi-h2/nifi-h2-database-migrator/target/maven-archiver/pom.properties nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/compile/default-compile/createdFiles.lst nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/compile/default-compile/inputFiles.lst nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/testCompile/default-testCompile/createdFiles.lst nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/testCompile/default-testCompile/inputFiles.lst nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/testCompile/groovy-tests/createdFiles.lst nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/testCompile/groovy-tests/inputFiles.lst nifi-h2/nifi-h2.iml nifi-h2/target/.plxarc

---

Archives:

• nifi-h2/nifi-h2-database/target/nifi-h2-database-1.18.0-SNAPSHOT.jar

• nifi-h2/nifi-h2-database/target/original-nifi-h2-database-1.18.0-SNAPSHOT.jar

• nifi-h2/nifi-h2-database-migrator/target/nifi-h2-database-migrator-1.18.0-SNAPSHOT.jar

---

Files with Apache License headers will be marked AL Binary files (which do not require any license headers) will be marked B Compressed archives will be marked A Notices, licenses etc. will be marked N AL .asf.yaml AL .github/PULL_REQUEST_TEMPLATE.md AL .github/workflows/ci-workflow.yml AL .github/workflows/stale.yml AL .github/workflows/system-tests.yml AL .mvn/wrapper/maven-wrapper.properties AL checkstyle.xml B jffi10666034361742842335.dll N KEYS N LICENSE AL mvnw AL mvnw.cmd AL nifi-dependency-check-maven/suppressions.xml !????? nifi-h2/nifi-h2-database/nifi-h2-database.iml !????? nifi-h2/nifi-h2-database/target/.plxarc N nifi-h2/nifi-h2-database/target/classes/META-INF/DEPENDENCIES N nifi-h2/nifi-h2-database/target/classes/META-INF/LICENSE N nifi-h2/nifi-h2-database/target/classes/META-INF/NOTICE !????? nifi-h2/nifi-h2-database/target/maven-archiver/pom.properties N nifi-h2/nifi-h2-database/target/maven-shared-archive-resources/META-INF/DEPENDENCIES N nifi-h2/nifi-h2-database/target/maven-shared-archive-resources/META-INF/LICENSE N nifi-h2/nifi-h2-database/target/maven-shared-archive-resources/META-INF/NOTICE A nifi-h2/nifi-h2-database/target/nifi-h2-database-1.18.0-SNAPSHOT.jar A nifi-h2/nifi-h2-database/target/original-nifi-h2-database-1.18.0-SNAPSHOT.jar N nifi-h2/nifi-h2-database/target/test-classes/META-INF/DEPENDENCIES N nifi-h2/nifi-h2-database/target/test-classes/META-INF/LICENSE N nifi-h2/nifi-h2-database/target/test-classes/META-INF/NOTICE !????? nifi-h2/nifi-h2-database-migrator/nifi-h2-database-migrator.iml !????? nifi-h2/nifi-h2-database-migrator/target/.plxarc N nifi-h2/nifi-h2-database-migrator/target/classes/META-INF/DEPENDENCIES N nifi-h2/nifi-h2-database-migrator/target/classes/META-INF/LICENSE N nifi-h2/nifi-h2-database-migrator/target/classes/META-INF/NOTICE B nifi-h2/nifi-h2-database-migrator/target/classes/org/apache/nifi/h2/database/migration/H2DatabaseMigrator.class B nifi-h2/nifi-h2-database-migrator/target/classes/org/apache/nifi/h2/database/migration/H2DatabaseUpdater.class !????? nifi-h2/nifi-h2-database-migrator/target/maven-archiver/pom.properties N nifi-h2/nifi-h2-database-migrator/target/maven-shared-archive-resources/META-INF/DEPENDENCIES N nifi-h2/nifi-h2-database-migrator/target/maven-shared-archive-resources/META-INF/LICENSE N nifi-h2/nifi-h2-database-migrator/target/maven-shared-archive-resources/META-INF/NOTICE !????? nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/compile/default-compile/createdFiles.lst !????? nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/compile/default-compile/inputFiles.lst !????? nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/testCompile/default-testCompile/createdFiles.lst !????? nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/testCompile/default-testCompile/inputFiles.lst !????? nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/testCompile/groovy-tests/createdFiles.lst !????? nifi-h2/nifi-h2-database-migrator/target/maven-status/maven-compiler-plugin/testCompile/groovy-tests/inputFiles.lst A nifi-h2/nifi-h2-database-migrator/target/nifi-h2-database-migrator-1.18.0-SNAPSHOT.jar N nifi-h2/nifi-h2-database-migrator/target/test-classes/META-INF/DEPENDENCIES N nifi-h2/nifi-h2-database-migrator/target/test-classes/META-INF/LICENSE N nifi-h2/nifi-h2-database-migrator/target/test-classes/META-INF/NOTICE B nifi-h2/nifi-h2-database-migrator/target/test-classes/nifi-flow-audit.mv.db B nifi-h2/nifi-h2-database-migrator/target/test-classes/org/apache/nifi/h2/database/migration/TestH2DatabaseUpdater.class !????? nifi-h2/nifi-h2.iml !????? nifi-h2/target/.plxarc N nifi-h2/target/maven-shared-archive-resources/META-INF/DEPENDENCIES N nifi-h2/target/maven-shared-archive-resources/META-INF/LICENSE N nifi-h2/target/maven-shared-archive-resources/META-INF/NOTICE N NOTICE AL pom.xml AL README.md AL SECURITY.md

---

let me know if i have to change anything Thanks

[exceptionfactory]

@ravinarayansingh The Static Analysis build lists the particular problem:

https://github.com/apache/nifi/actions/runs/5719909353/job/15509139654?pr=7544#step:5:5763

It looks like those nifi-h2 directories are leftover on your local system from the support branch, so they could be removed, but that will not impact the pull request build.

[ravinarayansingh]

Hi @exceptionfactory I have made the require changes Thanks

[MikeThomsen]

@ravinarayansingh you have a merge conflict.

[ravinarayansingh]

@MikeThomsen

resolved

[ravinarayansingh]

Hi @exceptionfactory , when this feature will be available in NiFi Thanks

[exceptionfactory]

Hi @exceptionfactory , when this feature will be available in NiFi Thanks

@ravinarayansingh These changes still need substantive review, along with a number of other pull requests, but thanks for addressing the initial feedback so far.

[ravinarayansingh]

Thanks for your patience on this pull request review @ravinarayansingh. Reviewing the implementation details, the general concept of a configurable column normalization strategy is helpful.

As a general recommendation, it looks like it would be helpful to make ColumnNameNormalizer an interface, with getNormalizedName(String columnName) as the interface method. Although the current implementation is simple enough, providing separate implementations would remove the need for the conditional logic. Given that this method will be called for every column, there seems to be value in having narrowly defined implementation classes.

Hi @exceptionfactory thanks for suggestion i will make required changes

[ravinarayansingh]

Hi @exceptionfactory ,

I have made the required changes, please have a look Thanks

[ravinarayansingh]

Hi @exceptionfactory i have made the required changes please have a look Thanks

[dan-s1]

@exceptionfactory Can you please restart the 'ci-workflow / Windows Zulu JDK 21 FR` build again? The error does not seem to relate to the changes in the PR. Thanks!

[ravinarayansingh]

Hi @dan-s1 and @exceptionfactory please have look and let me know if any change required

[ravinarayansingh]

@ravinarayansingh Thanks for your work and patience on this pull request.

Unfortunately I may not have been clear in previous comments regarding the interface-based approached for the ColumnNameNormalizer. I did not intend for the ColumnNameNormalizer to be implemented by each Processor, but instead, the ColumnNameNormalizer should have separate implementations for each strategy.

If it would be helpful, I could follow up with a commit that implements the approach I described.

Hi @exceptionfactory I have made the required changes please have a look

[exceptionfactory]

Thanks for the updates @ravinarayansingh, the ColumnNameNormalizer with individual implementations matches what I had in mind. I appreciate your efforts, I will take a closer look soon.

[mattyb149]

This needs a rebase against the latest main, then I will take a look also, thanks!

[ravinarayansingh]

Hi @mattyb149 Build is failing but i think that is not related to this PR please let me know if i have to change anything form my side

[ravinarayansingh]

Hi @jrsteinebrey
I have updated the code as per your suggestion, have a look

[jrsteinebrey]

Thanks for the changes you made. Some other files in this PR now have checkstyle violations:

Warning: src/main/java/org/apache/nifi/processors/standard/db/TableSchema.java:[155,47] (whitespace) WhitespaceAround: '!=' is not followed by whitespace. Warning: src/main/java/org/apache/nifi/processors/standard/db/TableSchema.java:[155,47] (whitespace) WhitespaceAround: '!=' is not preceded with whitespace. Warning: src/test/java/org/apache/nifi/processors/standard/db/impl/TestOracleDatabaseAdapter.java:[124,122] (whitespace) WhitespaceAfter: ',' is not followed by whitespace. Warning: src/test/java/org/apache/nifi/processors/standard/db/impl/TestOracle12DatabaseAdapter.java:[166,122] (whitespace) WhitespaceAfter: ',' is not followed by whitespace. Warning: src/test/java/org/apache/nifi/processors/standard/PutDatabaseRecordTest.java:[1832,11] (whitespace) WhitespaceAfter: 'catch' is not followed by whitespace. Warning: src/test/java/org/apache/nifi/processors/standard/PutDatabaseRecordTest.java:[1832,11] (whitespace) WhitespaceAround: 'catch' is not followed by whitespace.

[mattyb149]

The checkstyle rules were recently made more stringent, looks like this needs another rebase and please run your Maven build with the contrib-check profile activated.

[jrsteinebrey]

Thanks, @ravinarayansingh. My comments that are automatically marked as Outdated are all resolved. I still have some open existing comments that I recommend changing property names. I understand if you are waiting on those until you see if other reviews have feedback on my property renaming comments.

[mattyb149]

Looks like there are some merge and/or rebase issues here, might be worth a new PR applying the desired commits to a feature branch based off the latest main branch.

[ravinarayansingh]

Looks like there are some merge and/or rebase issues here, might be worth a new PR applying the desired commits to a feature branch based off the latest main branch.

Hi @mattyb149 I have created new PR please have a look

[mattyb149]

Closing this in favor of the new PR, thanks!