exceptionfactory commented on code in PR #7781:
URL: https://github.com/apache/nifi/pull/7781#discussion_r1341754971
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/IdentifyMimeType.java:
##########
@@ -99,6 +101,9 @@
}
)
public class IdentifyMimeType extends AbstractProcessor {
+ static final AllowableValue REPLACE = new AllowableValue("replace",
"replace", "Use config MIME Types only.");
+ static final AllowableValue ADD = new AllowableValue("add", "add",
Review Comment:
Recommend adjusting the casing of the values:
```suggestion
static final AllowableValue REPLACE = new AllowableValue("Replace",
"Replace", "Use config MIME Types only.");
static final AllowableValue ADD = new AllowableValue("Add", "Add",
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/IdentifyMimeType.java:
##########
@@ -206,34 +236,50 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
final ComponentLog logger = getLogger();
final AtomicReference<String> mimeTypeRef = new
AtomicReference<>(null);
+ final AtomicReference<String> extensionRef = new
AtomicReference<>(null);
final String filename =
flowFile.getAttribute(CoreAttributes.FILENAME.key());
session.read(flowFile, new InputStreamCallback() {
@Override
public void process(final InputStream stream) throws IOException {
try (final InputStream in = new BufferedInputStream(stream);
final TikaInputStream tikaStream =
TikaInputStream.get(in)) {
- Metadata metadata = new Metadata();
- if (filename != null &&
context.getProperty(USE_FILENAME_IN_DETECTION).asBoolean()) {
- metadata.add(TikaCoreProperties.RESOURCE_NAME_KEY,
filename);
+ if (customDetector != null) {
+ detectMimeType(customDetector, customMimeTypes,
tikaStream);
+ }
+
+ String mimeType = mimeTypeRef.get();
+
+ if (defaultDetector != null && (mimeType == null ||
mimeType.equals(MediaType.OCTET_STREAM.toString()) ||
mimeType.equals(MediaType.TEXT_PLAIN.toString()))) {
+ detectMimeType(defaultDetector, defaultMimeTypes,
tikaStream);
}
- // Get mime type
- MediaType mediatype = detector.detect(tikaStream,
metadata);
- mimeTypeRef.set(mediatype.toString());
}
}
+
+ private void detectMimeType(Detector detector, MimeTypes
mimeTypes, TikaInputStream tikaStream) throws IOException {
+ Metadata metadata = new Metadata();
+ if (filename != null &&
context.getProperty(USE_FILENAME_IN_DETECTION).asBoolean()) {
+ metadata.add(TikaCoreProperties.RESOURCE_NAME_KEY,
filename);
+ }
+
+ MediaType mediatype = detector.detect(tikaStream, metadata);
+ String extension = "";
+
+ try {
+ MimeType mimetype =
mimeTypes.forName(mediatype.toString());
+ extension = mimetype.getExtension();
+ } catch (MimeTypeException ex) {
+ logger.warn("MIME type extension lookup failed: {}", new
Object[]{ex});
Review Comment:
The `Object[]` wrapper is deprecated and should not be used.
```suggestion
logger.warn("MIME type extension lookup failed", e);
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/IdentifyMimeType.java:
##########
@@ -161,31 +180,42 @@ protected void init(final ProcessorInitializationContext
context) {
public void setup(final ProcessContext context) {
String configBody = context.getProperty(MIME_CONFIG_BODY).getValue();
String configFile =
context.getProperty(MIME_CONFIG_FILE).evaluateAttributeExpressions().getValue();
+ String configStrategy =
context.getProperty(CONFIG_STRATEGY).getValue();
- if (configBody == null && configFile == null){
- this.detector = config.getDetector();
- this.mimeTypes = config.getMimeRepository();
- } else if (configBody != null) {
- try {
- this.detector = MimeTypesFactory.create(new
ByteArrayInputStream(configBody.getBytes()));
- this.mimeTypes = (MimeTypes)this.detector;
- } catch (Exception e) {
- context.yield();
- throw new ProcessException("Failed to load config body", e);
- }
-
+ if (configBody == null && configFile == null) {
+ setDefaultMimeTypes();
+ } else if (configStrategy.equals(ADD.getValue())) {
+ setDefaultMimeTypes();
+ setCustomMimeTypes(configBody, configFile, context);
} else {
- try (final FileInputStream fis = new FileInputStream(configFile);
- final InputStream bis = new BufferedInputStream(fis)) {
- this.detector = MimeTypesFactory.create(bis);
- this.mimeTypes = (MimeTypes)this.detector;
- } catch (Exception e) {
- context.yield();
- throw new ProcessException("Failed to load config file", e);
- }
+ setCustomMimeTypes(configBody, configFile, context);
}
}
+ private void setDefaultMimeTypes() {
+ this.defaultDetector = config.getDetector();
+ this.defaultMimeTypes = config.getMimeRepository();
+ }
+
+ private void setCustomMimeTypes(String configBody, String configFile,
ProcessContext context) {
+ try (final InputStream customInputStream = configBody != null ? new
ByteArrayInputStream(configBody.getBytes()) : new FileInputStream(configFile)) {
+ this.customDetector = MimeTypesFactory.create(customInputStream);
+ this.customMimeTypes = (MimeTypes) this.customDetector;
+ } catch (Exception e) {
+ context.yield();
+ String configSource = configBody != null ? "body" : "file";
+ throw new ProcessException("Failed to load config " +
configSource, e);
+ }
+ }
+
+ @OnStopped
+ public void onStopped() {
+ defaultDetector = null;
+ defaultMimeTypes = null;
+
+ customDetector = null;
+ customMimeTypes = null;
+ }
Review Comment:
Setting these values to `null` is not necessary and this method can be
removed.
```suggestion
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/IdentifyMimeType.java:
##########
@@ -127,6 +132,16 @@ public class IdentifyMimeType extends AbstractProcessor {
.expressionLanguageSupported(ExpressionLanguageScope.NONE)
.build();
+ public static final PropertyDescriptor CONFIG_STRATEGY = new
PropertyDescriptor.Builder()
+ .displayName("Config Strategy")
+ .name("config-strategy")
+ .description("Replace default NiFi MIME Types or add to them. "
+ + "Only applicable if Config File or Config Body is used.")
+ .required(false)
Review Comment:
The `required` setting should be `true` when providing a default value.
```suggestion
.required(true)
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/IdentifyMimeType.java:
##########
@@ -136,8 +151,11 @@ public class IdentifyMimeType extends AbstractProcessor {
private List<PropertyDescriptor> properties;
private final TikaConfig config;
- private Detector detector;
- private MimeTypes mimeTypes;
+ private volatile Detector defaultDetector;
+ private volatile MimeTypes defaultMimeTypes;
+
+ private volatile Detector customDetector;
+ private volatile MimeTypes customMimeTypes;
Review Comment:
Is there a reason for marking these as `volatile`? They should not be
changed after Processor initialization.
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/IdentifyMimeType.java:
##########
@@ -127,6 +132,16 @@ public class IdentifyMimeType extends AbstractProcessor {
.expressionLanguageSupported(ExpressionLanguageScope.NONE)
.build();
+ public static final PropertyDescriptor CONFIG_STRATEGY = new
PropertyDescriptor.Builder()
+ .displayName("Config Strategy")
+ .name("config-strategy")
+ .description("Replace default NiFi MIME Types or add to them. "
Review Comment:
Recommend adjusting the description:
```suggestion
.description("Select the loading strategy for MIME Type
configuration provided in Config Body or Config File properties. "
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/IdentifyMimeType.java:
##########
@@ -127,6 +132,16 @@ public class IdentifyMimeType extends AbstractProcessor {
.expressionLanguageSupported(ExpressionLanguageScope.NONE)
.build();
+ public static final PropertyDescriptor CONFIG_STRATEGY = new
PropertyDescriptor.Builder()
+ .displayName("Config Strategy")
+ .name("config-strategy")
+ .description("Replace default NiFi MIME Types or add to them. "
+ + "Only applicable if Config File or Config Body is used.")
+ .required(false)
+ .allowableValues(REPLACE, ADD)
+ .defaultValue(REPLACE.getDisplayName())
Review Comment:
This needs to be the value instead of the display name:
```suggestion
.defaultValue(REPLACE.getValue())
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/IdentifyMimeType.java:
##########
@@ -206,34 +236,50 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
final ComponentLog logger = getLogger();
final AtomicReference<String> mimeTypeRef = new
AtomicReference<>(null);
+ final AtomicReference<String> extensionRef = new
AtomicReference<>(null);
final String filename =
flowFile.getAttribute(CoreAttributes.FILENAME.key());
session.read(flowFile, new InputStreamCallback() {
@Override
public void process(final InputStream stream) throws IOException {
try (final InputStream in = new BufferedInputStream(stream);
final TikaInputStream tikaStream =
TikaInputStream.get(in)) {
- Metadata metadata = new Metadata();
- if (filename != null &&
context.getProperty(USE_FILENAME_IN_DETECTION).asBoolean()) {
- metadata.add(TikaCoreProperties.RESOURCE_NAME_KEY,
filename);
+ if (customDetector != null) {
+ detectMimeType(customDetector, customMimeTypes,
tikaStream);
+ }
+
+ String mimeType = mimeTypeRef.get();
+
+ if (defaultDetector != null && (mimeType == null ||
mimeType.equals(MediaType.OCTET_STREAM.toString()) ||
mimeType.equals(MediaType.TEXT_PLAIN.toString()))) {
+ detectMimeType(defaultDetector, defaultMimeTypes,
tikaStream);
}
- // Get mime type
- MediaType mediatype = detector.detect(tikaStream,
metadata);
- mimeTypeRef.set(mediatype.toString());
}
}
+
+ private void detectMimeType(Detector detector, MimeTypes
mimeTypes, TikaInputStream tikaStream) throws IOException {
+ Metadata metadata = new Metadata();
+ if (filename != null &&
context.getProperty(USE_FILENAME_IN_DETECTION).asBoolean()) {
+ metadata.add(TikaCoreProperties.RESOURCE_NAME_KEY,
filename);
+ }
+
+ MediaType mediatype = detector.detect(tikaStream, metadata);
+ String extension = "";
+
+ try {
+ MimeType mimetype =
mimeTypes.forName(mediatype.toString());
+ extension = mimetype.getExtension();
+ } catch (MimeTypeException ex) {
Review Comment:
```suggestion
} catch (MimeTypeException e) {
```
##########
nifi-nar-bundles/nifi-standard-bundle/nifi-standard-processors/src/main/java/org/apache/nifi/processors/standard/IdentifyMimeType.java:
##########
@@ -161,31 +180,42 @@ protected void init(final ProcessorInitializationContext
context) {
public void setup(final ProcessContext context) {
String configBody = context.getProperty(MIME_CONFIG_BODY).getValue();
String configFile =
context.getProperty(MIME_CONFIG_FILE).evaluateAttributeExpressions().getValue();
+ String configStrategy =
context.getProperty(CONFIG_STRATEGY).getValue();
- if (configBody == null && configFile == null){
- this.detector = config.getDetector();
- this.mimeTypes = config.getMimeRepository();
- } else if (configBody != null) {
- try {
- this.detector = MimeTypesFactory.create(new
ByteArrayInputStream(configBody.getBytes()));
- this.mimeTypes = (MimeTypes)this.detector;
- } catch (Exception e) {
- context.yield();
- throw new ProcessException("Failed to load config body", e);
- }
-
+ if (configBody == null && configFile == null) {
+ setDefaultMimeTypes();
+ } else if (configStrategy.equals(ADD.getValue())) {
+ setDefaultMimeTypes();
+ setCustomMimeTypes(configBody, configFile, context);
} else {
- try (final FileInputStream fis = new FileInputStream(configFile);
- final InputStream bis = new BufferedInputStream(fis)) {
- this.detector = MimeTypesFactory.create(bis);
- this.mimeTypes = (MimeTypes)this.detector;
- } catch (Exception e) {
- context.yield();
- throw new ProcessException("Failed to load config file", e);
- }
+ setCustomMimeTypes(configBody, configFile, context);
}
}
+ private void setDefaultMimeTypes() {
+ this.defaultDetector = config.getDetector();
+ this.defaultMimeTypes = config.getMimeRepository();
+ }
+
+ private void setCustomMimeTypes(String configBody, String configFile,
ProcessContext context) {
+ try (final InputStream customInputStream = configBody != null ? new
ByteArrayInputStream(configBody.getBytes()) : new FileInputStream(configFile)) {
+ this.customDetector = MimeTypesFactory.create(customInputStream);
+ this.customMimeTypes = (MimeTypes) this.customDetector;
+ } catch (Exception e) {
+ context.yield();
+ String configSource = configBody != null ? "body" : "file";
+ throw new ProcessException("Failed to load config " +
configSource, e);
+ }
+ }
Review Comment:
Instead of creating multiple MimeTypes, it looks like it should be possible
to combine multiple entries use one of several `create()` methods.
--
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]
