awelless commented on code in PR #10716:
URL: https://github.com/apache/nifi/pull/10716#discussion_r2694325104
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/FetchBoxFileInfo.java:
##########
@@ -148,45 +149,45 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
}
final String fileId =
context.getProperty(FILE_ID).evaluateAttributeExpressions(flowFile).getValue();
+ if (fileId == null || fileId.isEmpty()) {
+ getLogger().error("File ID is required but was not provided. Check
that the File ID property is configured correctly.");
+ flowFile = session.putAttribute(flowFile, ERROR_MESSAGE, "File ID
is required but was not provided");
+ flowFile = session.penalize(flowFile);
+ session.transfer(flowFile, REL_FAILURE);
+ return;
+ }
try {
flowFile = fetchFileMetadata(fileId, session, flowFile);
session.transfer(flowFile, REL_SUCCESS);
- } catch (final BoxAPIResponseException e) {
+ } catch (final BoxAPIError e) {
flowFile = session.putAttribute(flowFile, ERROR_MESSAGE,
e.getMessage());
- flowFile = session.putAttribute(flowFile, ERROR_CODE,
String.valueOf(e.getResponseCode()));
-
- if (e.getResponseCode() == 404) {
- getLogger().warn("Box file with ID {} was not found.", fileId);
- session.transfer(flowFile, REL_NOT_FOUND);
- } else {
- getLogger().error("Failed to retrieve Box file representation
for file [{}]", fileId, e);
- session.transfer(flowFile, REL_FAILURE);
+ if (e.getResponseInfo() != null) {
+ flowFile = session.putAttribute(flowFile, ERROR_CODE,
String.valueOf(e.getResponseInfo().getStatusCode()));
+ if (e.getResponseInfo().getStatusCode() == 404) {
+ getLogger().warn("Box file with ID {} was not found.",
fileId);
+ session.transfer(flowFile, REL_NOT_FOUND);
+ return;
+ }
}
- } catch (final BoxAPIException e) {
+ getLogger().error("Failed to retrieve Box file representation for
file [{}]", fileId, e);
+ session.transfer(flowFile, REL_FAILURE);
+ } catch (final Exception e) {
flowFile = session.putAttribute(flowFile, ERROR_MESSAGE,
e.getMessage());
- flowFile = session.putAttribute(flowFile, ERROR_CODE,
String.valueOf(e.getResponseCode()));
flowFile = session.penalize(flowFile);
session.transfer(flowFile, REL_FAILURE);
}
}
- /**
- * Fetches the BoxFile instance for a given file ID. For testing purposes.
- *
- * @param fileId the ID of the file
- * @return BoxFile instance
- */
- protected BoxFile getBoxFile(final String fileId) {
- return new BoxFile(boxAPIConnection, fileId);
- }
-
private FlowFile fetchFileMetadata(final String fileId,
final ProcessSession session,
final FlowFile flowFile) {
- final BoxFile boxFile = getBoxFile(fileId);
- final BoxFile.Info fileInfo = boxFile.getInfo("name", "description",
"size", "created_at", "modified_at",
- "owned_by", "parent", "etag", "sha1", "item_status",
"sequence_id", "path_collection",
- "content_created_at", "content_modified_at", "trashed_at",
"purged_at", "shared_link");
+ final GetFileByIdQueryParams queryParams = new
GetFileByIdQueryParams.Builder()
+ .fields(List.of("name", "description", "size", "created_at",
"modified_at",
Review Comment:
Nitpicking. A constant for the parameter list.
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/FetchBoxFile.java:
##########
@@ -150,21 +151,25 @@ public void migrateProperties(PropertyConfiguration
config) {
config.renameProperty("box-file-id", FILE_ID.getName());
}
- BoxFile getBoxFile(String fileId) {
- return new BoxFile(boxAPIConnection, fileId);
- }
-
private FlowFile fetchFile(String fileId, ProcessSession session, FlowFile
flowFile) {
- final BoxFile boxFile = getBoxFile(fileId);
- flowFile = session.write(flowFile, outputStream ->
boxFile.download(outputStream));
- flowFile = session.putAllAttributes(flowFile,
BoxFileUtils.createAttributeMap(boxFile.getInfo()));
+ try (InputStream inputStream =
boxClient.getDownloads().downloadFile(fileId)) {
+ flowFile = session.importFrom(inputStream, flowFile);
+ } catch (Exception e) {
+ throw new ProcessException("Failed to download file from Box", e);
+ }
Review Comment:
Previously, the SDK errors thrown by `fetchFile` would've been propagated to
the caller and handled in `onTrigger`.
Since everything is wrapped into a `ProcessException`, so the handling of
`BoxAPIError` in `onTrigger` won't take place. Let's catch only `IOException`
here, passing the rest up the call chain
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/GetBoxFileCollaborators.java:
##########
@@ -256,17 +242,21 @@ private Set<String> parseFilter(final String filter) {
* @param attributeValues the map to populate with collaboration attributes
* @return the total count of collaborations processed (before filtering)
*/
- private int processCollaborations(final Iterable<BoxCollaboration.Info>
collaborations,
+ private int processCollaborations(final Collaborations collaborations,
final Set<String> allowedRoles,
final Set<String> allowedStatuses,
final Map<String, List<String>>
attributeValues) {
int count = 0;
Review Comment:
Nitpicking. No need to keep `count`, as `collaborations.getEntries()`
returns a list, so `size()` can be used.
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/FetchBoxFile.java:
##########
@@ -150,21 +151,25 @@ public void migrateProperties(PropertyConfiguration
config) {
config.renameProperty("box-file-id", FILE_ID.getName());
}
- BoxFile getBoxFile(String fileId) {
- return new BoxFile(boxAPIConnection, fileId);
- }
-
private FlowFile fetchFile(String fileId, ProcessSession session, FlowFile
flowFile) {
- final BoxFile boxFile = getBoxFile(fileId);
- flowFile = session.write(flowFile, outputStream ->
boxFile.download(outputStream));
- flowFile = session.putAllAttributes(flowFile,
BoxFileUtils.createAttributeMap(boxFile.getInfo()));
+ try (InputStream inputStream =
boxClient.getDownloads().downloadFile(fileId)) {
+ flowFile = session.importFrom(inputStream, flowFile);
+ } catch (Exception e) {
+ throw new ProcessException("Failed to download file from Box", e);
+ }
+
+ final GetFileByIdQueryParams queryParams = new
GetFileByIdQueryParams.Builder()
+ .fields(List.of("id", "name", "size", "modified_at",
"path_collection"))
Review Comment:
Nitpicking. These fields can be moved to a constant.
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/ListBoxFileMetadataInstances.java:
##########
@@ -134,28 +132,32 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
final String fileId =
context.getProperty(FILE_ID).evaluateAttributeExpressions(flowFile).getValue();
try {
- final BoxFile boxFile = getBoxFile(fileId);
+ final Metadatas metadatas = getFileMetadata(fileId);
final List<Map<String, Object>> instanceList = new ArrayList<>();
- final Iterable<Metadata> metadataList = boxFile.getAllMetadata();
- final Iterator<Metadata> iterator = metadataList.iterator();
final Set<String> templateNames = new LinkedHashSet<>();
- if (!iterator.hasNext()) {
+ if (metadatas.getEntries() == null ||
metadatas.getEntries().isEmpty()) {
flowFile = session.putAttribute(flowFile, "box.id", fileId);
flowFile = session.putAttribute(flowFile,
"box.metadata.instances.count", "0");
session.transfer(flowFile, REL_SUCCESS);
return;
}
- while (iterator.hasNext()) {
- final Metadata metadata = iterator.next();
+ for (final Metadata metadata : metadatas.getEntries()) {
final Map<String, Object> instanceFields = new HashMap<>();
+ final String templateName = metadata.getTemplate();
- templateNames.add(metadata.getTemplateName());
+ if (templateName != null) {
+ templateNames.add(templateName);
+ }
// Add standard metadata fields
- processBoxMetadataInstance(fileId, metadata, instanceFields);
+ instanceFields.put("$parent", metadata.getParent());
+ instanceFields.put("$template", templateName);
+ instanceFields.put("$scope", metadata.getScope());
+ instanceFields.put("$version", metadata.getVersion());
Review Comment:
Since we don't get `MetadataFull` here, we can't populate all the fields in
`processBoxMetadataInstance`. Theoretically, this can break some flows,
although might not be necessary for everyone.
Shall we add fetch `MetadataFull` as an optional behavior, which is enabled
by default?
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/FetchBoxFileRepresentation.java:
##########
@@ -157,60 +156,86 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
final String representationType =
context.getProperty(REPRESENTATION_TYPE).evaluateAttributeExpressions(flowFile).getValue();
try {
- final BoxFile boxFile = getBoxFile(fileId);
- final Info fileInfo = boxFile.getInfo();
+ // Get file info with representations
+ final GetFileByIdQueryParams queryParams = new
GetFileByIdQueryParams.Builder()
+ .fields(List.of("id", "name", "size", "created_at",
"modified_at", "representations"))
+ .build();
+ final FileFull fileInfo = boxClient.getFiles().getFileById(fileId,
queryParams);
+
+ // Find the matching representation
+ final String representationUrl = findRepresentationUrl(fileInfo,
representationType);
+ if (representationUrl == null) {
+ logger.warn("Representation {} is not available for file {}",
representationType, fileId);
+ flowFile = session.putAttribute(flowFile, "box.error.message",
"No matching representation found");
+ session.transfer(flowFile, REL_REPRESENTATION_NOT_FOUND);
+ return;
+ }
- flowFile = session.write(flowFile, outputStream ->
- // Download the file representation, box sdk handles a
request to create representation if it doesn't exist
- boxFile.getRepresentationContent("[" + representationType
+ "]", "", outputStream, MAX_RETRIES)
- );
+ // Download the representation content
+ flowFile = session.write(flowFile, outputStream -> {
+ try {
+ HttpClient client = HttpClient.newHttpClient();
+ HttpRequest request = HttpRequest.newBuilder()
Review Comment:
I didn't find a method in the SDK to download the representation, either.
I have some concerns about using java's `HttpClient` here. Any configuration
of `BoxClientService` is ignored.
I recommend considering the usage of `NetworkSession` and `NetworkClient`
from `boxClient` and see how it's done in
`boxClient.getDownloads().downloadFile()` method. This is going to be more
verbose, but will use an already configured Box connection.
If we decide to proceed with another client, will a dedicated `WebClient` be
a better option?
If we decide to keep using java `HttpClient`, shall we create it only once
in `@OnScheduled`?
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/ListBoxFileMetadataTemplates.java:
##########
@@ -133,41 +132,31 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
final String fileId =
context.getProperty(FILE_ID).evaluateAttributeExpressions(flowFile).getValue();
try {
- final BoxFile boxFile = getBoxFile(fileId);
+ final Metadatas metadatas = getFileMetadata(fileId);
final List<Map<String, Object>> templatesList = new ArrayList<>();
- final Iterable<Metadata> metadataList = boxFile.getAllMetadata();
- final Iterator<Metadata> iterator = metadataList.iterator();
final Set<String> templateNames = new LinkedHashSet<>();
- if (!iterator.hasNext()) {
+ if (metadatas.getEntries() == null ||
metadatas.getEntries().isEmpty()) {
flowFile = session.putAttribute(flowFile, "box.file.id",
fileId);
flowFile = session.putAttribute(flowFile,
"box.metadata.templates.count", "0");
session.transfer(flowFile, REL_SUCCESS);
return;
}
- while (iterator.hasNext()) {
- final Metadata metadata = iterator.next();
+ for (final Metadata metadata : metadatas.getEntries()) {
final Map<String, Object> templateFields = new HashMap<>();
+ final String templateName = metadata.getTemplate();
- templateNames.add(metadata.getTemplateName());
+ if (templateName != null) {
+ templateNames.add(templateName);
+ }
// Add standard metadata fields
- templateFields.put("$id", metadata.getID());
Review Comment:
Since we don't get `MetadataFull` here, we can't get the id Theoretically,
this can break some flows, although might not be necessary for everyone.
Shall we add fetch `MetadataFull` as an optional behavior, which is enabled
by default?
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/PutBoxFile.java:
##########
@@ -221,21 +229,22 @@ public void onTrigger(final ProcessContext context, final
ProcessSession session
try {
final long size = flowFile.getSize();
- final BoxFolder parentFolder =
getOrCreateDirectParentFolder(context, flowFile);
- fullPath = BoxFileUtils.getFolderPath(parentFolder.getInfo());
- BoxFile.Info uploadedFileInfo = null;
+ final String parentFolderId =
getOrCreateDirectParentFolder(context, flowFile);
+ final FolderFull parentFolderInfo = getFolderInfo(parentFolderId);
Review Comment:
Fetching a full folder representation seems redundant. See my comment in the
`getFolderIdByName` method.
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/PutBoxFile.java:
##########
@@ -271,145 +281,178 @@ public void migrateProperties(PropertyConfiguration
config) {
config.renameProperty("chunked-upload-threshold",
CHUNKED_UPLOAD_THRESHOLD.getName());
}
- BoxFolder getFolder(String folderId) {
- return new BoxFolder(boxAPIConnection, folderId);
+ private FolderFull getFolderInfo(String folderId) {
+ final GetFolderByIdQueryParams queryParams = new
GetFolderByIdQueryParams.Builder()
+ .fields(List.of("id", "name", "path_collection"))
+ .build();
+ return boxClient.getFolders().getFolderById(folderId, queryParams);
}
- private BoxFolder getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
+ private String getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
final String subfolderPath =
context.getProperty(SUBFOLDER_NAME).evaluateAttributeExpressions(flowFile).getValue();
final String folderId =
context.getProperty(FOLDER_ID).evaluateAttributeExpressions(flowFile).getValue();
- BoxFolder parentFolder = getFolderById(folderId);
+ String parentFolderId = getFolderById(folderId);
if (subfolderPath != null) {
final boolean createFolder =
context.getProperty(CREATE_SUBFOLDER).asBoolean();
final Queue<String> subFolderNames =
getSubFolderNames(subfolderPath);
- parentFolder = getOrCreateSubfolders(subFolderNames, parentFolder,
createFolder);
+ parentFolderId = getOrCreateSubfolders(subFolderNames,
parentFolderId, createFolder);
}
- return parentFolder;
+ return parentFolderId;
}
- private BoxFile.Info replaceBoxFileIfExists(BoxFolder parentFolder, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
- throws IOException, InterruptedException {
- final Optional<BoxFile> existingBoxFileInfo = getFileByName(filename,
parentFolder);
- if (existingBoxFileInfo.isPresent()) {
- final BoxFile existingBoxFile = existingBoxFileInfo.get();
-
- if (size > chunkUploadThreshold) {
- return existingBoxFile.uploadLargeFile(inputStream, size);
- } else {
- return existingBoxFile.uploadNewVersion(inputStream);
+ private FileFull replaceBoxFileIfExists(String parentFolderId, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
+ throws Exception {
+ final Optional<String> existingFileId = getFileIdByName(filename,
parentFolderId);
+ if (existingFileId.isPresent()) {
+ final String fileId = existingFileId.get();
+
+ // Upload new version
+ final UploadFileVersionRequestBodyAttributesField attributes = new
UploadFileVersionRequestBodyAttributesField(filename);
+ final UploadFileVersionRequestBody requestBody = new
UploadFileVersionRequestBody(attributes, inputStream);
+ final Files files =
boxClient.getUploads().uploadFileVersion(fileId, requestBody);
+ if (files.getEntries() != null && !files.getEntries().isEmpty()) {
+ return files.getEntries().get(0);
}
}
return null;
}
- private BoxFile.Info createBoxFile(BoxFolder parentFolder, String
filename, InputStream inputStream, long size, final long chunkUploadThreshold)
- throws IOException, InterruptedException {
- if (size > chunkUploadThreshold) {
- return parentFolder.uploadLargeFile(inputStream, filename, size);
- } else {
- return parentFolder.uploadFile(inputStream, filename);
+ private FileFull createBoxFile(String parentFolderId, String filename,
InputStream inputStream, long size, final long chunkUploadThreshold)
+ throws Exception {
+ final UploadFileRequestBodyAttributesParentField parent = new
UploadFileRequestBodyAttributesParentField(parentFolderId);
+ final UploadFileRequestBodyAttributesField attributes = new
UploadFileRequestBodyAttributesField(filename, parent);
+ final UploadFileRequestBody requestBody = new
UploadFileRequestBody(attributes, inputStream);
+ final Files files = boxClient.getUploads().uploadFile(requestBody);
+ if (files.getEntries() != null && !files.getEntries().isEmpty()) {
+ return files.getEntries().get(0);
}
+ return null;
Review Comment:
Shall we throw an exception if file upload succeeds, but no file metadata is
returned?
How shall the caller handle absent `FileFull`?
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/PutBoxFile.java:
##########
@@ -271,145 +281,178 @@ public void migrateProperties(PropertyConfiguration
config) {
config.renameProperty("chunked-upload-threshold",
CHUNKED_UPLOAD_THRESHOLD.getName());
}
- BoxFolder getFolder(String folderId) {
- return new BoxFolder(boxAPIConnection, folderId);
+ private FolderFull getFolderInfo(String folderId) {
+ final GetFolderByIdQueryParams queryParams = new
GetFolderByIdQueryParams.Builder()
+ .fields(List.of("id", "name", "path_collection"))
+ .build();
+ return boxClient.getFolders().getFolderById(folderId, queryParams);
}
- private BoxFolder getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
+ private String getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
final String subfolderPath =
context.getProperty(SUBFOLDER_NAME).evaluateAttributeExpressions(flowFile).getValue();
final String folderId =
context.getProperty(FOLDER_ID).evaluateAttributeExpressions(flowFile).getValue();
- BoxFolder parentFolder = getFolderById(folderId);
+ String parentFolderId = getFolderById(folderId);
if (subfolderPath != null) {
final boolean createFolder =
context.getProperty(CREATE_SUBFOLDER).asBoolean();
final Queue<String> subFolderNames =
getSubFolderNames(subfolderPath);
- parentFolder = getOrCreateSubfolders(subFolderNames, parentFolder,
createFolder);
+ parentFolderId = getOrCreateSubfolders(subFolderNames,
parentFolderId, createFolder);
}
- return parentFolder;
+ return parentFolderId;
}
- private BoxFile.Info replaceBoxFileIfExists(BoxFolder parentFolder, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
- throws IOException, InterruptedException {
- final Optional<BoxFile> existingBoxFileInfo = getFileByName(filename,
parentFolder);
- if (existingBoxFileInfo.isPresent()) {
- final BoxFile existingBoxFile = existingBoxFileInfo.get();
-
- if (size > chunkUploadThreshold) {
- return existingBoxFile.uploadLargeFile(inputStream, size);
- } else {
- return existingBoxFile.uploadNewVersion(inputStream);
+ private FileFull replaceBoxFileIfExists(String parentFolderId, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
+ throws Exception {
+ final Optional<String> existingFileId = getFileIdByName(filename,
parentFolderId);
+ if (existingFileId.isPresent()) {
+ final String fileId = existingFileId.get();
+
+ // Upload new version
+ final UploadFileVersionRequestBodyAttributesField attributes = new
UploadFileVersionRequestBodyAttributesField(filename);
+ final UploadFileVersionRequestBody requestBody = new
UploadFileVersionRequestBody(attributes, inputStream);
+ final Files files =
boxClient.getUploads().uploadFileVersion(fileId, requestBody);
+ if (files.getEntries() != null && !files.getEntries().isEmpty()) {
+ return files.getEntries().get(0);
}
Review Comment:
Shall we throw an exception if file upload succeeds, but no file metadata is
returned?
How shall the caller handle absent `FileFull`?
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/PutBoxFile.java:
##########
@@ -271,145 +281,178 @@ public void migrateProperties(PropertyConfiguration
config) {
config.renameProperty("chunked-upload-threshold",
CHUNKED_UPLOAD_THRESHOLD.getName());
}
- BoxFolder getFolder(String folderId) {
- return new BoxFolder(boxAPIConnection, folderId);
+ private FolderFull getFolderInfo(String folderId) {
+ final GetFolderByIdQueryParams queryParams = new
GetFolderByIdQueryParams.Builder()
+ .fields(List.of("id", "name", "path_collection"))
+ .build();
+ return boxClient.getFolders().getFolderById(folderId, queryParams);
}
- private BoxFolder getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
+ private String getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
final String subfolderPath =
context.getProperty(SUBFOLDER_NAME).evaluateAttributeExpressions(flowFile).getValue();
final String folderId =
context.getProperty(FOLDER_ID).evaluateAttributeExpressions(flowFile).getValue();
- BoxFolder parentFolder = getFolderById(folderId);
+ String parentFolderId = getFolderById(folderId);
if (subfolderPath != null) {
final boolean createFolder =
context.getProperty(CREATE_SUBFOLDER).asBoolean();
final Queue<String> subFolderNames =
getSubFolderNames(subfolderPath);
- parentFolder = getOrCreateSubfolders(subFolderNames, parentFolder,
createFolder);
+ parentFolderId = getOrCreateSubfolders(subFolderNames,
parentFolderId, createFolder);
}
- return parentFolder;
+ return parentFolderId;
}
- private BoxFile.Info replaceBoxFileIfExists(BoxFolder parentFolder, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
- throws IOException, InterruptedException {
- final Optional<BoxFile> existingBoxFileInfo = getFileByName(filename,
parentFolder);
- if (existingBoxFileInfo.isPresent()) {
- final BoxFile existingBoxFile = existingBoxFileInfo.get();
-
- if (size > chunkUploadThreshold) {
- return existingBoxFile.uploadLargeFile(inputStream, size);
- } else {
- return existingBoxFile.uploadNewVersion(inputStream);
+ private FileFull replaceBoxFileIfExists(String parentFolderId, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
+ throws Exception {
+ final Optional<String> existingFileId = getFileIdByName(filename,
parentFolderId);
+ if (existingFileId.isPresent()) {
+ final String fileId = existingFileId.get();
Review Comment:
Nitpicking. `Optional.isPresent() - get()` can be replaced with a more
type-safe and idiomatic handling. Otherwise it doesn't differ from a plain null
check.
```java
return getFileIdByName(filename, parentFolderId)
.flatMap(fileId -> {
final UploadFileVersionRequestBodyAttributesField
attributes = new UploadFileVersionRequestBodyAttributesField(filename);
final UploadFileVersionRequestBody requestBody = new
UploadFileVersionRequestBody(attributes, inputStream);
final Files files =
boxClient.getUploads().uploadFileVersion(fileId, requestBody);
return files.getEntries() != null &&
!files.getEntries().isEmpty()
? Optional.of(files.getEntries().getFirst())
: Optional.empty();
});
```
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/PutBoxFile.java:
##########
@@ -271,145 +281,178 @@ public void migrateProperties(PropertyConfiguration
config) {
config.renameProperty("chunked-upload-threshold",
CHUNKED_UPLOAD_THRESHOLD.getName());
}
- BoxFolder getFolder(String folderId) {
- return new BoxFolder(boxAPIConnection, folderId);
+ private FolderFull getFolderInfo(String folderId) {
+ final GetFolderByIdQueryParams queryParams = new
GetFolderByIdQueryParams.Builder()
+ .fields(List.of("id", "name", "path_collection"))
+ .build();
+ return boxClient.getFolders().getFolderById(folderId, queryParams);
}
- private BoxFolder getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
+ private String getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
final String subfolderPath =
context.getProperty(SUBFOLDER_NAME).evaluateAttributeExpressions(flowFile).getValue();
final String folderId =
context.getProperty(FOLDER_ID).evaluateAttributeExpressions(flowFile).getValue();
- BoxFolder parentFolder = getFolderById(folderId);
+ String parentFolderId = getFolderById(folderId);
if (subfolderPath != null) {
final boolean createFolder =
context.getProperty(CREATE_SUBFOLDER).asBoolean();
final Queue<String> subFolderNames =
getSubFolderNames(subfolderPath);
- parentFolder = getOrCreateSubfolders(subFolderNames, parentFolder,
createFolder);
+ parentFolderId = getOrCreateSubfolders(subFolderNames,
parentFolderId, createFolder);
}
- return parentFolder;
+ return parentFolderId;
}
- private BoxFile.Info replaceBoxFileIfExists(BoxFolder parentFolder, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
- throws IOException, InterruptedException {
- final Optional<BoxFile> existingBoxFileInfo = getFileByName(filename,
parentFolder);
- if (existingBoxFileInfo.isPresent()) {
- final BoxFile existingBoxFile = existingBoxFileInfo.get();
-
- if (size > chunkUploadThreshold) {
- return existingBoxFile.uploadLargeFile(inputStream, size);
- } else {
- return existingBoxFile.uploadNewVersion(inputStream);
+ private FileFull replaceBoxFileIfExists(String parentFolderId, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
+ throws Exception {
+ final Optional<String> existingFileId = getFileIdByName(filename,
parentFolderId);
+ if (existingFileId.isPresent()) {
+ final String fileId = existingFileId.get();
Review Comment:
Plain null check is also fine, but then there is little value in wrapping
the result of `getFileIdByName` in `Optional`.
##########
nifi-extension-bundles/nifi-box-bundle/nifi-box-processors/src/main/java/org/apache/nifi/processors/box/PutBoxFile.java:
##########
@@ -271,145 +281,178 @@ public void migrateProperties(PropertyConfiguration
config) {
config.renameProperty("chunked-upload-threshold",
CHUNKED_UPLOAD_THRESHOLD.getName());
}
- BoxFolder getFolder(String folderId) {
- return new BoxFolder(boxAPIConnection, folderId);
+ private FolderFull getFolderInfo(String folderId) {
+ final GetFolderByIdQueryParams queryParams = new
GetFolderByIdQueryParams.Builder()
+ .fields(List.of("id", "name", "path_collection"))
+ .build();
+ return boxClient.getFolders().getFolderById(folderId, queryParams);
}
- private BoxFolder getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
+ private String getOrCreateDirectParentFolder(ProcessContext context,
FlowFile flowFile) {
final String subfolderPath =
context.getProperty(SUBFOLDER_NAME).evaluateAttributeExpressions(flowFile).getValue();
final String folderId =
context.getProperty(FOLDER_ID).evaluateAttributeExpressions(flowFile).getValue();
- BoxFolder parentFolder = getFolderById(folderId);
+ String parentFolderId = getFolderById(folderId);
if (subfolderPath != null) {
final boolean createFolder =
context.getProperty(CREATE_SUBFOLDER).asBoolean();
final Queue<String> subFolderNames =
getSubFolderNames(subfolderPath);
- parentFolder = getOrCreateSubfolders(subFolderNames, parentFolder,
createFolder);
+ parentFolderId = getOrCreateSubfolders(subFolderNames,
parentFolderId, createFolder);
}
- return parentFolder;
+ return parentFolderId;
}
- private BoxFile.Info replaceBoxFileIfExists(BoxFolder parentFolder, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
- throws IOException, InterruptedException {
- final Optional<BoxFile> existingBoxFileInfo = getFileByName(filename,
parentFolder);
- if (existingBoxFileInfo.isPresent()) {
- final BoxFile existingBoxFile = existingBoxFileInfo.get();
-
- if (size > chunkUploadThreshold) {
- return existingBoxFile.uploadLargeFile(inputStream, size);
- } else {
- return existingBoxFile.uploadNewVersion(inputStream);
+ private FileFull replaceBoxFileIfExists(String parentFolderId, String
filename, final InputStream inputStream, final long size, final long
chunkUploadThreshold)
+ throws Exception {
+ final Optional<String> existingFileId = getFileIdByName(filename,
parentFolderId);
+ if (existingFileId.isPresent()) {
+ final String fileId = existingFileId.get();
+
+ // Upload new version
+ final UploadFileVersionRequestBodyAttributesField attributes = new
UploadFileVersionRequestBodyAttributesField(filename);
+ final UploadFileVersionRequestBody requestBody = new
UploadFileVersionRequestBody(attributes, inputStream);
+ final Files files =
boxClient.getUploads().uploadFileVersion(fileId, requestBody);
+ if (files.getEntries() != null && !files.getEntries().isEmpty()) {
+ return files.getEntries().get(0);
}
}
return null;
}
- private BoxFile.Info createBoxFile(BoxFolder parentFolder, String
filename, InputStream inputStream, long size, final long chunkUploadThreshold)
- throws IOException, InterruptedException {
- if (size > chunkUploadThreshold) {
- return parentFolder.uploadLargeFile(inputStream, filename, size);
- } else {
- return parentFolder.uploadFile(inputStream, filename);
+ private FileFull createBoxFile(String parentFolderId, String filename,
InputStream inputStream, long size, final long chunkUploadThreshold)
+ throws Exception {
+ final UploadFileRequestBodyAttributesParentField parent = new
UploadFileRequestBodyAttributesParentField(parentFolderId);
+ final UploadFileRequestBodyAttributesField attributes = new
UploadFileRequestBodyAttributesField(filename, parent);
+ final UploadFileRequestBody requestBody = new
UploadFileRequestBody(attributes, inputStream);
+ final Files files = boxClient.getUploads().uploadFile(requestBody);
+ if (files.getEntries() != null && !files.getEntries().isEmpty()) {
+ return files.getEntries().get(0);
}
+ return null;
}
- private Queue<String> getSubFolderNames(String subfolderPath) {
+ private Queue<String> getSubFolderNames(String subfolderPath) {
final Queue<String> subfolderNames = new LinkedList<>();
Collections.addAll(subfolderNames, subfolderPath.split("/"));
return subfolderNames;
}
- private BoxFolder getOrCreateSubfolders(Queue<String> subFolderNames,
BoxFolder parentFolder, boolean createFolder) {
- final BoxFolder newParentFolder =
getOrCreateFolder(subFolderNames.poll(), parentFolder, createFolder);
+ private String getOrCreateSubfolders(Queue<String> subFolderNames, String
parentFolderId, boolean createFolder) {
+ final String newParentFolderId =
getOrCreateFolder(subFolderNames.poll(), parentFolderId, createFolder);
if (!subFolderNames.isEmpty()) {
- return getOrCreateSubfolders(subFolderNames, newParentFolder,
createFolder);
+ return getOrCreateSubfolders(subFolderNames, newParentFolderId,
createFolder);
} else {
- return newParentFolder;
+ return newParentFolderId;
}
}
- private BoxFolder getOrCreateFolder(String folderName, BoxFolder
parentFolder, boolean createFolder) {
- final Optional<BoxFolder> existingFolder = getFolderByName(folderName,
parentFolder);
+ private String getOrCreateFolder(String folderName, String parentFolderId,
boolean createFolder) {
+ final Optional<String> existingFolderId =
getFolderIdByName(folderName, parentFolderId);
- if (existingFolder.isPresent()) {
- return existingFolder.get();
+ if (existingFolderId.isPresent()) {
+ return existingFolderId.get();
}
if (!createFolder) {
throw new ProcessException(format("The specified subfolder [%s]
does not exist and [%s] is false.",
folderName, CREATE_SUBFOLDER.getDisplayName()));
}
- return createFolder(folderName, parentFolder);
+ return createFolder(folderName, parentFolderId);
}
- private BoxFolder createFolder(final String folderName, final BoxFolder
parentFolder) {
- getLogger().info("Creating Folder [{}], Parent [{}]", folderName,
parentFolder.getID());
+ private String createFolder(final String folderName, final String
parentFolderId) {
+ getLogger().info("Creating Folder [{}], Parent [{}]", folderName,
parentFolderId);
try {
- return parentFolder.createFolder(folderName).getResource();
- } catch (BoxAPIResponseException e) {
- if (e.getResponseCode() != CONFLICT_RESPONSE_CODE) {
- throw e;
+ final CreateFolderRequestBodyParentField parent = new
CreateFolderRequestBodyParentField(parentFolderId);
+ final CreateFolderRequestBody requestBody = new
CreateFolderRequestBody(folderName, parent);
+ final FolderFull createdFolder =
boxClient.getFolders().createFolder(requestBody);
+ return createdFolder.getId();
+ } catch (BoxAPIError e) {
+ if (e.getResponseInfo() != null &&
e.getResponseInfo().getStatusCode() != CONFLICT_RESPONSE_CODE) {
+ throw new ProcessException("Failed to create folder: " +
e.getMessage(), e);
} else {
- Optional<BoxFolder> createdFolder =
waitForOngoingFolderCreationToFinish(folderName, parentFolder);
- return createdFolder.orElseThrow(() -> new
ProcessException(format("Created subfolder [%s] can not be found under [%s]",
- folderName, parentFolder.getID())));
+ Optional<String> createdFolderId =
waitForOngoingFolderCreationToFinish(folderName, parentFolderId);
+ return createdFolderId.orElseThrow(() -> new
ProcessException(format("Created subfolder [%s] can not be found under [%s]",
+ folderName, parentFolderId)));
}
}
}
- private Optional<BoxFolder> waitForOngoingFolderCreationToFinish(final
String folderName, final BoxFolder parentFolder) {
+ private Optional<String> waitForOngoingFolderCreationToFinish(final String
folderName, final String parentFolderId) {
try {
- Optional<BoxFolder> createdFolder = getFolderByName(folderName,
parentFolder);
+ Optional<String> createdFolderId = getFolderIdByName(folderName,
parentFolderId);
- for (int i = 0; i < NUMBER_OF_RETRIES && createdFolder.isEmpty();
i++) {
+ for (int i = 0; i < NUMBER_OF_RETRIES &&
createdFolderId.isEmpty(); i++) {
getLogger().debug("Subfolder [{}] under [{}] has not been
created yet, waiting {} ms",
- folderName, parentFolder.getID(), WAIT_TIME_MS);
+ folderName, parentFolderId, WAIT_TIME_MS);
Thread.sleep(WAIT_TIME_MS);
- createdFolder = getFolderByName(folderName, parentFolder);
+ createdFolderId = getFolderIdByName(folderName,
parentFolderId);
}
- return createdFolder;
+ return createdFolderId;
} catch (InterruptedException ie) {
throw new RuntimeException(format("Waiting for creation of
subfolder [%s] under [%s] was interrupted",
- folderName, parentFolder.getID()), ie);
+ folderName, parentFolderId), ie);
}
}
- private BoxFolder getFolderById(final String folderId) {
- final BoxFolder folder = getFolder(folderId);
+ private String getFolderById(final String folderId) {
try {
- //Error is returned for nonexistent folder only when a method is
called on BoxFolder.
- folder.getInfo();
- } catch (BoxAPIResponseException e) {
- if (e.getResponseCode() == NOT_FOUND_RESPONSE_CODE) {
+ final GetFolderByIdQueryParams queryParams = new
GetFolderByIdQueryParams.Builder()
+ .fields(List.of("id"))
+ .build();
+ boxClient.getFolders().getFolderById(folderId, queryParams);
+ return folderId;
+ } catch (BoxAPIError e) {
+ if (e.getResponseInfo() != null &&
e.getResponseInfo().getStatusCode() == NOT_FOUND_RESPONSE_CODE) {
throw new ProcessException(format("The Folder [%s] specified
by [%s] does not exist", folderId, FOLDER_ID.getDisplayName()));
}
+ throw new ProcessException("Failed to get folder: " +
e.getMessage(), e);
}
- return folder;
}
- private Optional<BoxFolder> getFolderByName(final String folderName, final
BoxFolder parentFolder) {
- return getItemByName(folderName, parentFolder, BoxFolder.Info.class)
- .map(BoxFolder.Info::getResource);
- }
+ private Optional<String> getFolderIdByName(final String folderName, final
String parentFolderId) {
+ final GetFolderItemsQueryParams queryParams = new
GetFolderItemsQueryParams.Builder()
+ .fields(List.of("name", "type"))
+ .build();
- private Optional<BoxFile> getFileByName(final String filename, final
BoxFolder parentFolder) {
- return getItemByName(filename, parentFolder, BoxFile.Info.class)
- .map(BoxFile.Info::getResource);
+ final Items items =
boxClient.getFolders().getFolderItems(parentFolderId, queryParams);
+
+ if (items.getEntries() != null) {
+ for (Object itemObj : items.getEntries()) {
Review Comment:
`items.getEntries()` returns a list of `Item`. `Item` is
`OneOfThree<FileFull, FolderFull, WebLink>`.
So:
1. We should use `OneOfThree` api instead of instanceof checks.
2. We can operate on `FolderFull` instead of folder ids. This will allow us
to get `FolderFull` in `onTrigger` (line 232), so fetching a folder info on
line 233 would be redundant.
--
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]