-
Notifications
You must be signed in to change notification settings - Fork 2.8k
NIFI-14459: Creating processor to update box metadata template #9866
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: main
Are you sure you want to change the base?
Conversation
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Solid draft.
Added a few comments related to behavior during field changes and a bunch of comments related to the style.
.addValidator(StandardValidators.NON_EMPTY_VALIDATOR) | ||
.build(); | ||
|
||
public static final PropertyDescriptor SCOPE = new PropertyDescriptor.Builder() |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In CreateBoxMetadataTemplate
the used scope is enterprise
only. Without any possible configuration. Shall we do the same here?
private static class FieldDefinition { | ||
String key; | ||
String type; | ||
String displayName; | ||
boolean hidden; | ||
String description; | ||
List<String> options; | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
I'm vary of having a class with uncontrollably mutable fields.
What do you think about making it a record and providing a builder? If that sounds too verbose, a record with with
methods, perhaps?
private record FieldDefinition(
String key,
String type,
String displayName,
boolean hidden,
String description,
List<String> options
) {
static class Builder {
private String key;
private String type;
private String displayName;
private boolean hidden;
private String description;
private List<String> options;
Builder(String key, String type) {
// ...
}
void displayName(String displayName) {
// ...
}
}
}
private record FieldDefinition(
String key,
String type,
String displayName,
boolean hidden,
String description,
List<String> options
) {
FieldDefinition(String key, String type) {
this(key, type, null, false, null, emptyList());
}
FieldDefinition withDisplayName(String displayName) {
return new FieldDefinition(key, type, displayName, hidden, description, options);
}
// ...
}
final StringBuilder jsonBuilder = new StringBuilder(); | ||
jsonBuilder.append("{\"op\":\"addField\",\"data\":{") | ||
.append("\"key\":\"").append(field.key).append("\",") | ||
.append("\"type\":\"").append(field.type).append("\","); | ||
if (field.displayName != null) { | ||
jsonBuilder.append("\"displayName\":\"").append(field.displayName).append("\","); | ||
} | ||
jsonBuilder.append("\"hidden\":").append(field.hidden); | ||
|
||
if (field.description != null) { | ||
jsonBuilder.append(",\"description\":\"").append(field.description).append("\""); | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's use a Box class instead of manual json creation.
final MetadataTemplate.Field data = new MetadataTemplate.Field();
data.setKey(field.key);
data.setType(field.type);
final MetadataTemplate.FieldOperation operation = new MetadataTemplate.FieldOperation();
operation.setOp(MetadataTemplate.Operation.addField);
operation.setData(data);
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Interesting, was looking at https://developer.box.com/guides/metadata/templates/update/ so i thought i would have to format everything. I'll try it out
} | ||
|
||
private MetadataTemplate.FieldOperation createEditFieldOperation(final String fieldKey, final Map<String, Object> changes) { | ||
final StringBuilder jsonBuilder = new StringBuilder(); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Same here. Let's build MetadataTemplate.FieldOperation
in-place.
final FieldDefinition desiredField) { | ||
final Map<String, Object> changes = new HashMap<>(); | ||
|
||
if (!existingField.getKey().equals(desiredField.key)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In this processor the key always matches. That's our immutable field identifier and we use it to match existing and desired fields.
Otherwise it's impossible to detect field key changes using the desired state only.
changes.put("displayName", desiredField.displayName); | ||
} | ||
|
||
if (!existingField.getType().equals(desiredField.type)) { |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
According to this doc a field type can't be changed.
So perhaps we should create an error if the types don't match?
// Check for updated options on enum or multiSelect fields | ||
boolean isEnumOrMultiSelect = "enum".equals(desiredField.type) || "multiSelect".equals(desiredField.type); | ||
if (isEnumOrMultiSelect && desiredField.options != null && !desiredField.options.isEmpty()) { | ||
final List<String> existingOptions = existingField.getOptions(); | ||
if (existingOptions == null || !new HashSet<>(existingOptions).equals(new HashSet<>(desiredField.options))) { | ||
changes.put("options", desiredField.options); | ||
} | ||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Unfortunately, this isn't that simple :(
// Build JSON for the removeField operation | ||
final String removeFieldJson = String.format("{\"op\":\"removeField\",\"fieldKey\":\"%s\"}", fieldKey); | ||
return new MetadataTemplate.FieldOperation(removeFieldJson); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
And here. MetadataTemplate.FieldOperation
// Verify operations were generated correctly (should create multiple operations) | ||
assertEquals(SCOPE, capturedScope); | ||
assertEquals(TEMPLATE_KEY, capturedTemplateKey); | ||
assertFalse(capturedOperations.isEmpty(), "Should generate operations for template updates"); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Let's also check what operations were created.
final MetadataTemplate.Field field1 = mock(MetadataTemplate.Field.class); | ||
when(field1.getKey()).thenReturn("name"); | ||
when(field1.getDisplayName()).thenReturn("Name"); | ||
when(field1.getType()).thenReturn("string"); | ||
when(field1.getIsHidden()).thenReturn(false); |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
There is no need to mock the fields, as they can be created easily.
final MetadataTemplate.Field field1 = mock(MetadataTemplate.Field.class); | |
when(field1.getKey()).thenReturn("name"); | |
when(field1.getDisplayName()).thenReturn("Name"); | |
when(field1.getType()).thenReturn("string"); | |
when(field1.getIsHidden()).thenReturn(false); | |
final MetadataTemplate.Field field1 = new MetadataTemplate.Field(); | |
field1.setKey("name"); | |
field1.setDisplayName("Name"); | |
field1.setType("string"); | |
field1.setIsHidden(false); |
Summary
NIFI-14459
Creating processor to update a box metadata template given a desired state. Fetches existing metadata template and compares with what is provided in flowFile content. Reference documentation used:
Tracking
Please complete the following tracking steps prior to pull request creation.
Issue Tracking
Pull Request Tracking
NIFI-00000
NIFI-00000
Pull Request Formatting
main
branchVerification
Please indicate the verification steps performed prior to pull request creation.
Build
mvn clean install -P contrib-check
Licensing
LICENSE
andNOTICE
filesDocumentation