NIFI-14892 Aligned those processors and controller services whose property names do not match the human-friendly display names in extended bundles nifi-airtable-bundle, nifi-amqp-bundle, nifi-asana-bundle, nifi-asn1-bundle and nifi-aws-bundle.

[dan-s1]

Summary

NIFI-14892

To better understand where changes were made for migrate properties in the AWS related code, it is important to note the two parent classes in two distinct hierarchies whose children override the migrateProperties method. The two parent classes are AbstractAwsProcessor and AbstractAWSCredentialsProviderProcessor. The tables below detail the children which extend these classes and whose children extend those children. Also detailed are which classes which override migrateProperties which is important to keep track of in terms of calling the parent's migrateProperties method with super.migrateProperties.

Class	Direct Children	Children which override migrateProperties
AbstractAwsProcessor	AbstractAwsAsyncProcessor AbstractAwsSyncProcessor
AbstractAwsAsyncProcessor	ConsumeKinesisStream	ConsumeKinesisStream
AbstractAwsSyncProcessor	AbstractAwsMachineLearningJobStarter AbstractAwsMachineLearningJobStatusProcessor AbstractDynamoDBProcessor DeleteSQS GetSQS PutKinesisFirehose PutKinesisStream PutLambda PutSNS PutCloudWatchMetric PuSQS	AbstractAwsMachineLearningJobStarter AbstractAwsMachineLearningJobStatusProcessor PutCloudWatchMetric PutKinesisStream PutSQS
AbstractAwsMachineLearningJobStarter	StartAwsPollyJob StartAwsTextractJob StartAwsTranscribeJob StartAwsTranslateJob	StartAwsTextractJob
AbstractAwsMachineLearningJobStatusProcessor	GetAwsPollyJobStatus GetAwsTextractJobStatus GetAwsTranscribeJobStatus GetAwsTranslateJobStatus	GetAwsTextractJobStatus
AbstractDynamoDBProcessor	DeleteDynamoDB GetDynamoDB PutDynamoDB PutDynamoDBRecord	PutDynamoDBRecord

Class	Direct Children	Children which override migrateProperties
AbstractAWSCredentialsProviderProcessor	AbstractS3Processor	AbstractS3Processor
AbstractS3Processor	CopyS3Object DeleteS3Object FetchS3Object GetS3ObjectMetadata GetS3ObjectTags ListS3 PutS3Object TagS3Object	FetchS3Object ListS3 PutS3Object TagS3Object

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 ./mvnw clean install -P contrib-check
  • JDK 21

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

[exceptionfactory]

Thanks for working on these changes @dan-s1.

This is one of the rare cases where I don't think unit tests are helpful. The problem is that the tests requiring reproducing the deprecated property names and expected names in the test. If the migrate methods included more complex logic, testing could be useful, but simply asserting that expected mappings occurred seems like unnecessary repetition.

[dan-s1]

@exceptionfactory I understand for the those unit tests which test the properties defined in the actual processor. But what about processors which inherit properties from a parent and have their own properties defined? I would think we would want to to test super.migrateProperties was called to ensure all properties are renamed. In fact I had pointed out to you on the Jira ticket a case in point where super.migrateProperties was not called but should have been. Also when working the Azure code I noticed there are properties defined in utility classes and not the actual processor/controller service and used across various processors/controller services. Wouldn't it be a good idea to to test those migrations?

[exceptionfactory]

Thanks for the reply @dan-s1, that's a good point about Processors with migrate methods in super classes, or those that use utilities for migration. I am supportive of unit tests for those scenarios. What do you think about using that as the decision point, and removing some of the unit tests where the migration is a simple one-to-one in the same class?

[dan-s1]

@exceptionfactory I just want to clarify what I should do for those classes which inherit properties from a parent but do not have properties of there own to rename hence they do not define override migrateProperties. In that case only the parent migrateProperties is called. Its not exactly a simple one-to-one but it possibly could be considered one. Do we want unit tests for those scenarios (e.g. CopyS3Object) to ensure those properties were renamed?

Also a separate question I have is I noticed that in AbstractAwsMachineLearningJobStatusProcessor it has the line

config.renameProperty("aws-region", REGION.getName());

The REGION property is defined in AbstractAwsProcessor and is used in many AWS related classes yet I do not see a renaming in any other class except this one. I added a rename to PutSQS although I am not sure if I should have . I am thinking the rename should probably be in done in the migrateProperties method of AbstractAwsProcessor as it is the ultimate parent of many of the AWS related processors. Please advise.

[dan-s1]

@exceptionfactory If you are okay with it, I am going to pull up the
config.renameProperty("aws-region", REGION.getName());
to AbstractAwsProcessor where I believe it rightfully should be.

[exceptionfactory]

@exceptionfactory If you are okay with it, I am going to pull up the config.renameProperty("aws-region", REGION.getName()); to AbstractAwsProcessor where I believe it rightfully should be.

Sounds good, thanks.

[dan-s1]

Please see the tables in the Summary above detailing which classes are overriding migrateProperties and should be calling super.migrateProperties.

[exceptionfactory]

Thanks for the updates @dan-s1, this looks close to completion, I noted a couple property names to adjust, and a couple places where the superclass method should be called.

[exceptionfactory]

Thanks @dan-s1, the latest version looks good. +1 merging

[dan-s1]

@exceptionfactory Thanks for approving and merging!