pvillard31 commented on code in PR #11081:
URL: https://github.com/apache/nifi/pull/11081#discussion_r3021985072
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceTextWithMapping.java:
##########
@@ -227,49 +243,56 @@ protected String
fillReplacementValueBackReferences(String rawReplacementValue,
}
private void updateMapping(final ProcessContext context) {
- if (processorLock.tryLock()) {
- final ComponentLog logger = getLogger();
- try {
- // if not queried mapping file lastUpdate time in
- // mapppingRefreshPeriodSecs, do so.
- long currentTimeSecs = System.currentTimeMillis() / 1000;
- long mappingRefreshPeriodSecs =
context.getProperty(MAPPING_FILE_REFRESH_INTERVAL).asTimePeriod(TimeUnit.SECONDS);
-
- boolean retry = (currentTimeSecs > (mappingTestTime.get() +
mappingRefreshPeriodSecs));
- if (retry) {
- mappingTestTime.set(System.currentTimeMillis() / 1000);
- // see if the mapping file needs to be reloaded
- final String fileName =
context.getProperty(MAPPING_FILE).getValue();
- final File file = new File(fileName);
- if (file.exists() && file.isFile() && file.canRead()) {
- if (file.lastModified() > lastModified.get()) {
- lastModified.getAndSet(file.lastModified());
- try (FileInputStream is = new
FileInputStream(file)) {
- logger.info("Reloading mapping file: {}",
fileName);
-
- final Map<String, String> mapping =
loadMappingFile(is);
- final ConfigurationState newState = new
ConfigurationState(mapping);
- configurationStateRef.set(newState);
- } catch (IOException e) {
- logger.error("Error reading mapping file", e);
+ final ResourceReference resourceReference =
context.getProperty(MAPPING_FILE).asResource();
+
+ if (resourceReference.getResourceType() == ResourceType.FILE) {
+ if (processorLock.tryLock()) {
+ final ComponentLog logger = getLogger();
+ try {
+ // if not queried mapping file lastUpdate time in
+ // mappingRefreshPeriodSecs, do so.
+ long currentTimeSecs = System.currentTimeMillis() / 1000;
+ long mappingRefreshPeriodSecs =
context.getProperty(MAPPING_FILE_REFRESH_INTERVAL).asTimePeriod(TimeUnit.SECONDS);
+
+ boolean retry = (currentTimeSecs > (mappingTestTime.get()
+ mappingRefreshPeriodSecs));
Review Comment:
```suggestion
final long currentTimeSecs = System.currentTimeMillis()
/ 1000;
final long mappingRefreshPeriodSecs =
context.getProperty(MAPPING_FILE_REFRESH_INTERVAL).asTimePeriod(TimeUnit.SECONDS);
final boolean retry = (currentTimeSecs >
(mappingTestTime.get() + mappingRefreshPeriodSecs));
```
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceTextWithMapping.java:
##########
@@ -249,72 +257,72 @@ public void testEscapingDollarSign() throws IOException {
runner.run();
runner.assertAllFlowFilesTransferred(ReplaceTextWithMapping.REL_SUCCESS, 1);
- final MockFlowFile out =
runner.getFlowFilesForRelationship(ReplaceTextWithMapping.REL_SUCCESS).get(0);
- String outputString = new String(out.toByteArray());
- String expected = "-roses- are -$1 apple-\n"
- + "violets are -$1 blueberry-\n"
- + "something else is -$1 grape-\n"
- + "I'm not good at writing poems";
+ final MockFlowFile out =
runner.getFlowFilesForRelationship(ReplaceTextWithMapping.REL_SUCCESS).getFirst();
+ final String outputString = out.getContent();
+ final String expected = """
+ -roses- are -$1 apple-
+ violets are -$1 blueberry-
+ something else is -$1 grape-
+ I'm not good at writing poems""";
assertEquals(expected, outputString);
}
@Test
public void testEscapingDollarSignSimple() throws IOException {
- final TestRunner runner = getRunner();
runner.setProperty(ReplaceTextWithMapping.MAPPING_FILE,
Paths.get("src/test/resources/TestReplaceTextWithMapping/color-fruit-escaped-dollar-mapping.txt").toFile().getAbsolutePath());
runner.enqueue(Paths.get("src/test/resources/TestReplaceTextWithMapping/colors-without-dashes.txt"));
runner.run();
runner.assertAllFlowFilesTransferred(ReplaceTextWithMapping.REL_SUCCESS, 1);
- final MockFlowFile out =
runner.getFlowFilesForRelationship(ReplaceTextWithMapping.REL_SUCCESS).get(0);
- String outputString = new String(out.toByteArray());
- String expected = "roses are $1 apple\n"
- + "violets are $1 blueberry\n"
- + "something else is $1 grape\n"
- + "I'm not good at writing poems";
+ final MockFlowFile out =
runner.getFlowFilesForRelationship(ReplaceTextWithMapping.REL_SUCCESS).getFirst();
+ final String outputString = out.getContent();
+ final String expected = """
+ roses are $1 apple
+ violets are $1 blueberry
+ something else is $1 grape
+ I'm not good at writing poems""";
assertEquals(expected, outputString);
}
@Test
public void testReplaceWithEmptyString() throws IOException {
- final TestRunner runner = getRunner();
runner.setProperty(ReplaceTextWithMapping.MAPPING_FILE,
Paths.get("src/test/resources/TestReplaceTextWithMapping/color-fruit-blank-mapping.txt").toFile().getAbsolutePath());
runner.enqueue(Paths.get("src/test/resources/TestReplaceTextWithMapping/colors-without-dashes.txt"));
runner.run();
runner.assertAllFlowFilesTransferred(ReplaceTextWithMapping.REL_SUCCESS, 1);
- final MockFlowFile out =
runner.getFlowFilesForRelationship(ReplaceTextWithMapping.REL_SUCCESS).get(0);
- String outputString = new String(out.toByteArray());
- String expected = "roses are \n"
- + "violets are \n"
- + "something else is \n"
- + "I'm not good at writing poems";
+ final MockFlowFile out =
runner.getFlowFilesForRelationship(ReplaceTextWithMapping.REL_SUCCESS).getFirst();
+ final String outputString = out.getContent();
+ String expected = """
Review Comment:
```suggestion
final String expected = """
```
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceTextWithMapping.java:
##########
@@ -176,6 +178,20 @@ public Set<Relationship> getRelationships() {
return RELATIONSHIPS;
}
+ @OnScheduled
+ public void onScheduled(final ProcessContext context) {
+ final ResourceReference resourceReference =
context.getProperty(MAPPING_FILE).asResource();
+ if (resourceReference.getResourceType() == ResourceType.TEXT) {
+ try {
+ getLogger().info("Reloading mapping file contents");
+ final InputStream inputStream = resourceReference.read();
Review Comment:
Should be in a try-with-resources
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/ReplaceTextWithMapping.java:
##########
@@ -176,6 +178,20 @@ public Set<Relationship> getRelationships() {
return RELATIONSHIPS;
}
+ @OnScheduled
+ public void onScheduled(final ProcessContext context) {
+ final ResourceReference resourceReference =
context.getProperty(MAPPING_FILE).asResource();
+ if (resourceReference.getResourceType() == ResourceType.TEXT) {
+ try {
+ getLogger().info("Reloading mapping file contents");
Review Comment:
```suggestion
getLogger().info("Loading mapping content");
```
##########
nifi-extension-bundles/nifi-standard-bundle/nifi-standard-processors/src/test/java/org/apache/nifi/processors/standard/TestReplaceTextWithMapping.java:
##########
@@ -229,18 +237,18 @@ public void
testBackReferenceWithInvalidReferenceIsEscaped() throws IOException
runner.run();
runner.assertAllFlowFilesTransferred(ReplaceTextWithMapping.REL_SUCCESS, 1);
- final MockFlowFile out =
runner.getFlowFilesForRelationship(ReplaceTextWithMapping.REL_SUCCESS).get(0);
- String outputString = new String(out.toByteArray());
- String expected = "roses are red$d apple\n"
- + "violets are blue$d blueberry\n"
- + "something else is green$d grape\n"
- + "I'm not good at writing poems";
+ final MockFlowFile out =
runner.getFlowFilesForRelationship(ReplaceTextWithMapping.REL_SUCCESS).getFirst();
+ final String outputString = out.getContent();
+ String expected = """
Review Comment:
```suggestion
final String expected = """
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
