exceptionfactory commented on code in PR #10322:
URL: https://github.com/apache/nifi/pull/10322#discussion_r2474825803
##########
nifi-commons/nifi-web-client-api/src/main/java/org/apache/nifi/web/client/api/StandardMultipartFormDataStreamBuilder.java:
##########
@@ -55,30 +55,29 @@ public class StandardMultipartFormDataStreamBuilder
implements MultipartFormData
private final List<Part> parts = new ArrayList<>();
- /**
- * Build Sequence Input Stream from collection of Form Data Parts
formatted with boundaries
- *
- * @return Input Stream
- */
@Override
public InputStream build() {
if (parts.isEmpty()) {
throw new IllegalStateException("Parts required");
}
- final List<InputStream> partInputStreams = new ArrayList<>();
+ final List<InputStream> streams = new ArrayList<>();
+ for (int index = 0; index < parts.size(); index++) {
+ final Part part = parts.get(index);
+ final String boundaryPrefix = (index == 0 ? BOUNDARY_SEPARATOR +
boundary + CARRIAGE_RETURN_LINE_FEED
+ : CARRIAGE_RETURN_LINE_FEED + BOUNDARY_SEPARATOR +
boundary + CARRIAGE_RETURN_LINE_FEED);
Review Comment:
This ternary and string concatenation is rather difficult to read. Having
something like the `getFooter` method would be helpful.
##########
nifi-extension-bundles/nifi-atlassian-bundle/nifi-atlassian-extensions/src/main/java/org/apache/nifi/atlassian/bitbucket/BitbucketRepositoryClient.java:
##########
@@ -299,13 +419,43 @@ public String createContent(final GitCreateContentRequest
request) throws FlowRe
String.format("Error while committing content for
repository [%s] on branch %s at path %s: %s", repoName, branch, resolvedPath,
getErrorMessage(response)));
}
- final Optional<String> lastCommit = getLatestCommit(branch,
resolvedPath);
+ return getRequiredLatestCommit(branch, resolvedPath);
+ }
- if (lastCommit.isEmpty()) {
- throw new FlowRegistryException(String.format("Could not find
commit for the file %s we just tried to commit on branch %s", resolvedPath,
branch));
+ private String createContentDataCenter(final GitCreateContentRequest
request, final String resolvedPath, final String branch) throws
FlowRegistryException {
+ final StandardMultipartFormDataStreamBuilder multipartBuilder = new
StandardMultipartFormDataStreamBuilder();
+ final String fileName = getFileName(resolvedPath);
+ multipartBuilder.addPart("content", fileName,
StandardHttpContentType.APPLICATION_OCTET_STREAM,
request.getContent().getBytes(StandardCharsets.UTF_8));
+ multipartBuilder.addPart("branch", StandardHttpContentType.TEXT_PLAIN,
branch.getBytes(StandardCharsets.UTF_8));
+
+ final String message = request.getMessage();
+ if (message != null && !message.isEmpty()) {
+ multipartBuilder.addPart("message",
StandardHttpContentType.TEXT_PLAIN, message.getBytes(StandardCharsets.UTF_8));
}
- return lastCommit.get();
+ final String existingContentSha = request.getExistingContentSha();
+ if (existingContentSha != null && !existingContentSha.isEmpty()) {
+ multipartBuilder.addPart("sourceCommitId",
StandardHttpContentType.TEXT_PLAIN,
existingContentSha.getBytes(StandardCharsets.UTF_8));
+ }
+
+ final HttpUriBuilder uriBuilder =
getRepositoryUriBuilder().addPathSegment("browse");
+ addPathSegments(uriBuilder, resolvedPath);
+ final URI uri = uriBuilder.build();
+ final byte[] requestBody = toByteArray(multipartBuilder.build());
+ final HttpResponseEntity response =
this.webClient.getWebClientService()
+ .put()
+ .uri(uri)
+ .body(new ByteArrayInputStream(requestBody),
OptionalLong.of(requestBody.length))
+ .header(AUTHORIZATION_HEADER, authToken.getAuthzHeaderValue())
+ .header(CONTENT_TYPE_HEADER,
multipartBuilder.getHttpContentType().getContentType())
+ .retrieve();
+
+ if (response.statusCode() != HttpURLConnection.HTTP_OK &&
response.statusCode() != HttpURLConnection.HTTP_CREATED) {
Review Comment:
Is it possible to return either 200 or 201? It seems like it should be one
or the other.
##########
nifi-extension-bundles/nifi-atlassian-bundle/nifi-atlassian-extensions/src/main/java/org/apache/nifi/atlassian/bitbucket/BitbucketFlowRegistryClient.java:
##########
@@ -160,7 +174,17 @@ public boolean
isStorageLocationApplicable(FlowRegistryClientConfigurationContex
@Override
protected String getStorageLocation(GitRepositoryClient repositoryClient) {
- final BitbucketRepositoryClient gitLabRepositoryClient =
(BitbucketRepositoryClient) repositoryClient;
- return
STORAGE_LOCATION_FORMAT.formatted(gitLabRepositoryClient.getWorkspace(),
gitLabRepositoryClient.getRepoName());
+ final BitbucketRepositoryClient bitbucketRepositoryClient =
(BitbucketRepositoryClient) repositoryClient;
+
+ if (bitbucketRepositoryClient.getFormFactor() ==
BitbucketFormFactor.DATA_CENTER) {
+ final String apiHost = bitbucketRepositoryClient.getApiHost();
+ final String projectKey =
bitbucketRepositoryClient.getProjectKey();
+ if (apiHost != null && projectKey != null) {
+ return "git@" + apiHost + ":" + projectKey + "/" +
bitbucketRepositoryClient.getRepoName() + ".git";
Review Comment:
Recommend using string formatting instead of concatenation to make this
structure easier to follow.
##########
nifi-extension-bundles/nifi-atlassian-bundle/nifi-atlassian-extensions/src/main/java/org/apache/nifi/atlassian/bitbucket/BitbucketFlowRegistryClient.java:
##########
@@ -92,7 +101,8 @@ public class BitbucketFlowRegistryClient extends
AbstractGitFlowRegistryClient {
static final PropertyDescriptor APP_PASSWORD = new
PropertyDescriptor.Builder()
.name("App Password")
- .description("The App Password to use for authentication")
+ .displayName("App Password or API Token")
Review Comment:
Instead of adding the `displayName`, it would be better to migrate the
property name. Alternatively, for the sake of clarity, it might be better to
have a separate property.
##########
nifi-commons/nifi-web-client-api/src/main/java/org/apache/nifi/web/client/api/MultipartFormDataStreamBuilder.java:
##########
@@ -55,4 +55,12 @@ public interface MultipartFormDataStreamBuilder {
* @return Builder
*/
MultipartFormDataStreamBuilder addPart(String name, HttpContentType
httpContentType, byte[] bytes);
+
+ default MultipartFormDataStreamBuilder addPart(String name, String
fileName, HttpContentType httpContentType, InputStream inputStream) {
+ return addPart(name, httpContentType, inputStream);
+ }
+
+ default MultipartFormDataStreamBuilder addPart(String name, String
fileName, HttpContentType httpContentType, byte[] bytes) {
+ return addPart(name, httpContentType, bytes);
+ }
Review Comment:
For completeness, it would be helpful to add JavaDoc comments like the other
methods.
##########
nifi-extension-bundles/nifi-atlassian-bundle/nifi-atlassian-extensions/src/main/java/org/apache/nifi/atlassian/bitbucket/BitbucketAuthenticationType.java:
##########
@@ -21,8 +21,9 @@
public enum BitbucketAuthenticationType implements DescribedValue {
- BASIC_AUTH("Basic Auth", """
+ BASIC_AUTH("Basic Auth & API Token", """
Review Comment:
Recommend avoiding the use of the ampersand character in labels. On the
other hand, is it necessary to mention `API Token` in the name when it is in
the description?
```suggestion
BASIC_AUTH("Basic Auth and API Token", """
```
##########
nifi-extension-bundles/nifi-atlassian-bundle/nifi-atlassian-extensions/src/main/java/org/apache/nifi/atlassian/bitbucket/BitbucketRepositoryClient.java:
##########
@@ -334,34 +493,249 @@ public InputStream deleteContent(final String filePath,
final String commitMessa
throw new FlowRegistryException(
String.format("Error while deleting content for repository
[%s] on branch %s at path %s: %s", repoName, branch, resolvedPath,
getErrorMessage(response)));
}
+ }
- return fileToBeDeleted;
+ private void deleteContentDataCenter(final String resolvedPath, final
String commitMessage, final String branch) throws FlowRegistryException {
+ final Optional<String> latestCommit = getLatestCommit(branch,
resolvedPath);
+ final HttpUriBuilder uriBuilder =
getRepositoryUriBuilder().addPathSegment("browse");
+ addPathSegments(uriBuilder, resolvedPath);
+ uriBuilder.addQueryParameter("branch", branch);
+ if (commitMessage != null) {
+ uriBuilder.addQueryParameter("message", commitMessage);
+ }
+ latestCommit.ifPresent(commit ->
uriBuilder.addQueryParameter("sourceCommitId", commit));
+
+ final URI uri = uriBuilder.build();
+ final HttpResponseEntity response =
this.webClient.getWebClientService()
+ .delete()
+ .uri(uri)
+ .header(AUTHORIZATION_HEADER, authToken.getAuthzHeaderValue())
+ .retrieve();
+
+ final int statusCode = response.statusCode();
+ if (statusCode != HttpURLConnection.HTTP_OK && statusCode !=
HttpURLConnection.HTTP_ACCEPTED
+ && statusCode != HttpURLConnection.HTTP_NO_CONTENT &&
statusCode != HttpURLConnection.HTTP_CREATED) {
+ throw new FlowRegistryException(
+ String.format("Error while deleting content for repository
[%s] on branch %s at path %s: %s", repoName, branch, resolvedPath,
getErrorMessage(response)));
+ }
}
private Iterator<JsonNode> getFiles(final String branch, final String
resolvedPath) throws FlowRegistryException {
final Optional<String> lastCommit = getLatestCommit(branch,
resolvedPath);
if (lastCommit.isEmpty()) {
- throw new FlowRegistryException(String.format("Could not find
committed files at %s on branch %s response from Bitbucket API", resolvedPath,
branch));
+ return Collections.emptyIterator();
}
- // retrieve source data
- //
https://api.bitbucket.org/2.0/repositories/{workspace}/{repoName}/src/{commit}/{path}
- final URI uri =
getUriBuilder().addPathSegment("src").addPathSegment(lastCommit.get()).addPathSegment(resolvedPath).build();
+ return formFactor == BitbucketFormFactor.DATA_CENTER
+ ? getFilesDataCenter(branch, resolvedPath, lastCommit.get())
+ : getFilesCloud(branch, resolvedPath, lastCommit.get());
+ }
+
+ private Iterator<JsonNode> getFilesCloud(final String branch, final String
resolvedPath, final String commit) throws FlowRegistryException {
+ final URI uri = getRepositoryUriBuilder()
+ .addPathSegment("src")
+ .addPathSegment(commit)
+ .addPathSegment(resolvedPath)
+ .build();
final String errorMessage = String.format("Error while listing content
for repository [%s] on branch %s at path %s", repoName, branch, resolvedPath);
return getPagedResponseValues(uri, errorMessage);
}
+ private Iterator<JsonNode> getFilesDataCenter(final String branch, final
String resolvedPath, final String commit) throws FlowRegistryException {
+ final List<JsonNode> allValues = new ArrayList<>();
+ Integer nextPageStart = null;
+
+ do {
+ final URI uri = buildBrowseUri(resolvedPath, commit,
nextPageStart, false);
+ final HttpResponseEntity response =
this.webClient.getWebClientService()
+ .get()
+ .uri(uri)
+ .header(AUTHORIZATION_HEADER,
authToken.getAuthzHeaderValue())
+ .retrieve();
+
+ if (response.statusCode() != HttpURLConnection.HTTP_OK) {
+ final String errorMessage = String.format("Error while listing
content for repository [%s] on branch %s at path %s", repoName, branch,
resolvedPath);
+ throw new FlowRegistryException(errorMessage + ": " +
getErrorMessage(response));
+ }
+
+ final JsonNode root;
+ try {
+ root = objectMapper.readTree(response.body());
+ } catch (IOException e) {
+ throw new FlowRegistryException(String.format("Could not parse
Bitbucket API response at %s", uri), e);
+ }
+
+ final JsonNode children = root.path("children");
+ final JsonNode values = children.path("values");
+ if (values.isArray()) {
+ values.forEach(allValues::add);
+ }
+
+ if (children.path("isLastPage").asBoolean(true)) {
+ nextPageStart = null;
+ } else {
+ final JsonNode nextPageStartNode =
children.get("nextPageStart");
+ nextPageStart = nextPageStartNode != null &&
nextPageStartNode.isInt() ? nextPageStartNode.intValue() : null;
+ }
+ } while (nextPageStart != null);
+
+ return allValues.iterator();
+ }
+
+ private URI buildBrowseUri(final String resolvedPath, final String commit,
final Integer start, final boolean rawContent) {
+ final HttpUriBuilder builder =
getRepositoryUriBuilder().addPathSegment("browse");
+ addPathSegments(builder, resolvedPath);
+ builder.addQueryParameter("at", commit);
+ if (start != null) {
+ builder.addQueryParameter("start", String.valueOf(start));
+ }
+ return builder.build();
+ }
+
+ private URI buildRawUri(final String resolvedPath, final String commit) {
+ final HttpUriBuilder builder =
getRepositoryUriBuilder().addPathSegment("raw");
+ addPathSegments(builder, resolvedPath);
+ builder.addQueryParameter("at", commit);
+ return builder.build();
+ }
+
private Iterator<JsonNode> getListCommits(final String branch, final
String path) throws FlowRegistryException {
- // retrieve latest commit for that branch
- //
https://api.bitbucket.org/2.0/repositories/{workspace}/{repoName}/commits/{branch}
- final URI uri =
getUriBuilder().addPathSegment("commits").addPathSegment(branch).addQueryParameter("path",
path).build();
- final String errorMessage = String.format("Error while listing commits
for repository [%s] on branch %s", repoName, branch);
+ if (formFactor == BitbucketFormFactor.DATA_CENTER) {
+ return getListCommitsDataCenter(branch, path);
+ }
+ return getListCommitsCloud(branch, path);
+ }
+
+ private Iterator<JsonNode> getListCommitsCloud(final String branch, final
String path) throws FlowRegistryException {
+ if (!hasCommits) {
+ // very specific behavior when there is no commit at all yet in
the repo
+ final URI uri = getRepositoryUriBuilder()
+ .addPathSegment("commits")
+ .build();
+
+ final HttpResponseEntity response = webClient.getWebClientService()
+ .get()
+ .uri(uri)
+ .header(AUTHORIZATION_HEADER,
authToken.getAuthzHeaderValue())
+ .retrieve();
+
+ if (response.statusCode() != HttpURLConnection.HTTP_OK) {
+ final String errorMessage = String.format("Error while listing
commits for repository [%s] on branch %s", repoName, branch);
+ throw new FlowRegistryException(errorMessage + ": " +
getErrorMessage(response));
+ }
+
+ final JsonNode root;
+ try {
+ root = objectMapper.readTree(response.body());
+ } catch (IOException e) {
+ throw new FlowRegistryException(String.format("Could not parse
Bitbucket API response at %s", uri), e);
+ }
+
+ final JsonNode values = root.path("values");
+ if (values.isArray() && values.isEmpty()) {
+ return Collections.emptyIterator();
+ } else {
+ // There is at least one commit, proceed as usual
+ // and never check again
+ hasCommits = true;
+ }
+ }
+
+ final URI uri = getRepositoryUriBuilder()
+ .addPathSegment("commits")
+ .addPathSegment(branch)
+ .addQueryParameter("path", path)
+ .build();
+ final String errorMessage = String.format("Error while listing commits
for repository [%s] on branch %s", repoName, branch);
return getPagedResponseValues(uri, errorMessage);
}
+ private Iterator<JsonNode> getListCommitsDataCenter(final String branch,
final String path) throws FlowRegistryException {
+ final List<JsonNode> allValues = new ArrayList<>();
+ Integer nextPageStart = null;
+
+ do {
+ if (!hasCommits) {
+ // very specific behavior when there is no commit at all yet
in the repo
+ final URI uri = buildCommitsUri(null, null, null);
+ final HttpResponseEntity response =
this.webClient.getWebClientService()
+ .get()
+ .uri(uri)
+ .header(AUTHORIZATION_HEADER,
authToken.getAuthzHeaderValue())
+ .retrieve();
+ if (response.statusCode() != HttpURLConnection.HTTP_OK) {
+ final String errorMessage = String.format("Error while
listing commits for repository [%s] on branch %s", repoName, branch);
+ throw new FlowRegistryException(errorMessage + ": " +
getErrorMessage(response));
+ }
+ final JsonNode root;
+ try {
+ root = objectMapper.readTree(response.body());
+ } catch (IOException e) {
+ throw new FlowRegistryException(String.format("Could not
parse Bitbucket API response at %s", uri), e);
+ }
+
+ final JsonNode values = root.path("values");
+ if (values.isArray() && values.isEmpty()) {
+ return Collections.emptyIterator();
+ } else {
+ // There is at least one commit, proceed as usual
+ // and never check again
+ hasCommits = true;
+ }
+ }
+
+ final URI uri = buildCommitsUri(branch, path, nextPageStart);
+ final HttpResponseEntity response =
this.webClient.getWebClientService()
+ .get()
+ .uri(uri)
+ .header(AUTHORIZATION_HEADER,
authToken.getAuthzHeaderValue())
+ .retrieve();
+
+ if (response.statusCode() != HttpURLConnection.HTTP_OK) {
+ final String errorMessage = String.format("Error while listing
commits for repository [%s] on branch %s", repoName, branch);
+ throw new FlowRegistryException(errorMessage + ": " +
getErrorMessage(response));
+ }
+
+ final JsonNode root;
+ try {
+ root = objectMapper.readTree(response.body());
+ } catch (IOException e) {
+ throw new FlowRegistryException(String.format("Could not parse
Bitbucket API response at %s", uri), e);
+ }
+
+ final JsonNode values = root.path("values");
Review Comment:
It looks like there are a number of repeated uses of certain expected field
names, which could be useful to declare as static final variables.
--
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]
