From 622b9df8303b34943d07aa3c038f33bbad20fca0 Mon Sep 17 00:00:00 2001 From: Rodrigo Date: Thu, 7 Mar 2024 12:21:58 -0500 Subject: [PATCH 1/4] HPCC4J-581 WsFS Client should add path delim only if needed (#690) - Introduces functionality to strip trailing white space - Adds trimright utility functionality - Adds new test cases - Adjusts pre-existing test cases to expected | actual format - Do not trim original string parameter - Do not duplicate Path URL param Signed-off-by: Rodrigo Pastrana --- .../ws/client/HPCCFileSprayClient.java | 24 +++-- .../hpccsystems/ws/client/utils/Utils.java | 43 ++++++++ .../ws/client/utils/UtilsTest.java | 98 +++++++++++-------- 3 files changed, 115 insertions(+), 50 deletions(-) diff --git a/wsclient/src/main/java/org/hpccsystems/ws/client/HPCCFileSprayClient.java b/wsclient/src/main/java/org/hpccsystems/ws/client/HPCCFileSprayClient.java index 212bb281c..a7283a6cd 100644 --- a/wsclient/src/main/java/org/hpccsystems/ws/client/HPCCFileSprayClient.java +++ b/wsclient/src/main/java/org/hpccsystems/ws/client/HPCCFileSprayClient.java @@ -1004,9 +1004,8 @@ public ProgressResponseWrapper sprayVariable(DelimitedDataOptions options, DropZ if (targetDropZone == null) throw new Exception("TargetDropZone object not available!"); SprayVariable request = new SprayVariable(); - request.setSourceIP(targetDropZone.getNetAddress()); - request.setSourcePath(targetDropZone.getPath() + "/" + sourceFileName); + request.setSourcePath(Utils.ensureTrailingPathSlash(targetDropZone.getPath()) + sourceFileName); request.setDestGroup(destGroup); request.setDestLogicalName(targetFileName); request.setOverwrite(overwrite); @@ -1162,7 +1161,7 @@ public ProgressResponseWrapper sprayXML(DropZoneWrapper targetDropZone, String s request.setDestGroup(destGroup); request.setSourceIP(targetDropZone.getNetAddress()); - request.setSourcePath(targetDropZone.getPath() + "/" + sourceFileName); + request.setSourcePath(Utils.ensureTrailingPathSlash(targetDropZone.getPath()) + sourceFileName); request.setDestLogicalName(targetFileName); request.setOverwrite(overwrite); request.setSourceFormat(format.getValue()); @@ -1318,7 +1317,7 @@ public ProgressResponseWrapper sprayFixed(DropZoneWrapper targetDropZone, String request.setDestGroup(destGroup); request.setSourceRecordSize(recordSize); request.setSourceIP(targetDropZone.getNetAddress()); - request.setSourcePath(targetDropZone.getPath() + "/" + sourceFileName); + request.setSourcePath(Utils.ensureTrailingPathSlash(targetDropZone.getPath()) + sourceFileName); request.setDestLogicalName(targetFileLabel); request.setOverwrite(overwrite); request.setPrefix(prefix); @@ -1481,11 +1480,18 @@ public boolean uploadLargeFile(File uploadFile, DropZoneWrapper dropZone) URLConnection fileUploadConnection = null; URL fileUploadURL = null; String uploadurlbuilder = UPLOADURI; - uploadurlbuilder += "&NetAddress=" + dropZone.getNetAddress(); - String path = dropZone.getPath().trim(); - if (!path.endsWith("/")) - path += "/"; - uploadurlbuilder += "&Path=" + path; + + if (dropZone.getPath().isEmpty()) + { + log.error("HPCCFileSprayClient::uploadLargeFile: empty dropZone path detected!"); + return false; + } + + uploadurlbuilder += "&NetAddress=" + dropZone.getNetAddress() + "&Path=" + Utils.ensureTrailingPathSlash(dropZone.getPath()); + + if (!dropZone.getName().isEmpty()) + uploadurlbuilder += "&DropZoneName=" + dropZone.getName(); + uploadurlbuilder += "&OS=" + (dropZone.getLinux().equalsIgnoreCase("true") ? "2" : "1"); uploadurlbuilder += "&rawxml_=1"; WritableByteChannel outchannel = null; diff --git a/wsclient/src/main/java/org/hpccsystems/ws/client/utils/Utils.java b/wsclient/src/main/java/org/hpccsystems/ws/client/utils/Utils.java index f33d83cf1..df7a56162 100644 --- a/wsclient/src/main/java/org/hpccsystems/ws/client/utils/Utils.java +++ b/wsclient/src/main/java/org/hpccsystems/ws/client/utils/Utils.java @@ -1061,21 +1061,64 @@ public static DocumentBuilder newSafeXMLDocBuilder() throws ParserConfigurationE return safeXMLDocBuilder; } + /** + * Ensures the given path contains a trailing path delimiter. + * Does not introduce duplicate trailing path delimiter if one already exists. + * Defaults to Linux style separator if the given path either contains a Linux style separator, or the path is empty. + * Strips all trailing white space character + * @param path The path to be postfixed + * @return original path with proper trailing path delimiter + */ public static String ensureTrailingPathSlash(String path) { return ensureTrailingPathSlash(path, (path.contains(Character.toString(LINUX_SEP)) || path.length() == 0) ? LINUX_SEP : WIN_SEP); } + /** + * Ensures the given path contains a trailing path delimiter. + * Does not introduce duplicate trailing path delimiter if one already exists. + * Uses Linux style path separator 'useLinuxSep' == "true", otherwise uses windows style path separator + * Strips all trailing white space character + * @param path path The path to be postfixed + * @param useLinuxSep String, if "true" linux styled path delimiter will be used + * @return original path with proper trailing path delimiter + */ public static String ensureTrailingPathSlash(String path, String useLinuxSep) { return ensureTrailingPathSlash(path, useLinuxSep.equalsIgnoreCase("true") ? LINUX_SEP : WIN_SEP); } + /** + * Ensures the given path contains a trailing path delimiter. + * Does not introduce duplicate trailing path delimiter if one already exists. + * Uses provided 'slash' as trailing path delimiter + * Strips all trailing white space character + * @param path The path to be postfixed + * @param slash The character to append + * @return original path with proper trailing path delimiter + */ public static String ensureTrailingPathSlash(String path, char slash) { + path = trimTrailing(path); + if (path.length() == 0 || path.charAt(path.length()-1) != slash) path = path + slash; return path; } + + /** + * Removes trailing whitespace characters from a string. + * + * @param originalStr the original string from which trailing whitespace should be removed + * @return a new string with the same characters as the original string, minus any trailing whitespace + */ + public static String trimTrailing(String originalStr) + { + int strIndex = originalStr.length()-1; + while(strIndex >= 0 && Character.isWhitespace(originalStr.charAt(strIndex))) + strIndex--; + + return originalStr.substring(0,strIndex+1); + } } diff --git a/wsclient/src/test/java/org/hpccsystems/ws/client/utils/UtilsTest.java b/wsclient/src/test/java/org/hpccsystems/ws/client/utils/UtilsTest.java index 1ff309ebf..eef268e47 100644 --- a/wsclient/src/test/java/org/hpccsystems/ws/client/utils/UtilsTest.java +++ b/wsclient/src/test/java/org/hpccsystems/ws/client/utils/UtilsTest.java @@ -8,59 +8,75 @@ public class UtilsTest { + @Test + public void testEnsureTrailingSlashTrailingWhiteSpace() + { + assertEquals(Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash("")); + assertEquals(Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP)+ " ")); + assertEquals(Character.toString(Utils.WIN_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP) + " ")); + assertEquals(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)+"\t")); + assertEquals("C:\\some\\Path\\", Utils.ensureTrailingPathSlash("C:\\some\\Path ")); + assertEquals("C:\\some\\Path\\", Utils.ensureTrailingPathSlash("C:\\some\\Path\\ ")); + assertEquals("/another/path/", Utils.ensureTrailingPathSlash("/another/path ")); + assertEquals("/another/path/", Utils.ensureTrailingPathSlash("/another/path/\t\t")); + assertEquals("/another/path/", Utils.ensureTrailingPathSlash("/another/path/\n")); + assertEquals("/another/path/", Utils.ensureTrailingPathSlash("/another/path/" + '\u005Cn')); + assertEquals("/another/path/", Utils.ensureTrailingPathSlash("/another/path/ " + '\u005Ct')); + } + @Test public void testEnsureTrailingSlashNoSlashSpecified() { - assertEquals(Utils.ensureTrailingPathSlash(""), Character.toString(Utils.LINUX_SEP)); //no sep in path, default to lin sep - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP)), Character.toString(Utils.LINUX_SEP));//no change expected - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)), Character.toString(Utils.WIN_SEP)); //no change expected - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)), Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP));//no change expected - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path"), "C:\\some\\Path\\"); - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path\\"), "C:\\some\\Path\\"); - assertEquals(Utils.ensureTrailingPathSlash("/another/path"), "/another/path/"); - assertEquals(Utils.ensureTrailingPathSlash("/another/path/"), "/another/path/"); + assertEquals(Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash("")); //no sep in path, default to lin sep + assertEquals(Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP)));//no change expected + assertEquals(Character.toString(Utils.WIN_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP))); //no change expected + assertEquals(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)));//no change expected + assertEquals("C:\\some\\Path\\", Utils.ensureTrailingPathSlash("C:\\some\\Path")); + assertEquals("C:\\some\\Path\\", Utils.ensureTrailingPathSlash("C:\\some\\Path\\")); + assertEquals("/another/path/", Utils.ensureTrailingPathSlash("/another/path")); + assertEquals("/another/path/", Utils.ensureTrailingPathSlash("/another/path/")); } @Test public void testEnsureTrailingSlashSlashSpecified() { - assertEquals(Utils.ensureTrailingPathSlash("", Utils.LINUX_SEP), Character.toString(Utils.LINUX_SEP)); - assertEquals(Utils.ensureTrailingPathSlash("", Utils.WIN_SEP), Character.toString(Utils.WIN_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP), Utils.WIN_SEP), Character.toString(Utils.LINUX_SEP)+Utils.WIN_SEP); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP), Utils.LINUX_SEP), Character.toString(Utils.LINUX_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP), Utils.WIN_SEP), Character.toString(Utils.WIN_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP), Utils.LINUX_SEP), Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.LINUX_SEP), Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.WIN_SEP), Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)+Character.toString(Utils.WIN_SEP)); - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path", Utils.LINUX_SEP), "C:\\some\\Path\\"+Utils.LINUX_SEP); - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path", Utils.WIN_SEP), "C:\\some\\Path\\"+Utils.WIN_SEP); - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path\\", Utils.LINUX_SEP), "C:\\some\\Path\\" + Utils.LINUX_SEP); - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path\\", Utils.WIN_SEP), "C:\\some\\Path\\"); - assertEquals(Utils.ensureTrailingPathSlash("/another/path", Utils.LINUX_SEP), "/another/path" + Utils.LINUX_SEP); - assertEquals(Utils.ensureTrailingPathSlash("/another/path", Utils.WIN_SEP), "/another/path/"+ Utils.WIN_SEP); - assertEquals(Utils.ensureTrailingPathSlash("/another/path/", Utils.LINUX_SEP), "/another/path/"); - assertEquals(Utils.ensureTrailingPathSlash("/another/path/", Utils.WIN_SEP), "/another/path/"+Utils.WIN_SEP); + assertEquals(Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash("", Utils.LINUX_SEP)); + assertEquals(Character.toString(Utils.WIN_SEP), Utils.ensureTrailingPathSlash("", Utils.WIN_SEP)); + assertEquals(Character.toString(Utils.LINUX_SEP)+Utils.WIN_SEP, Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP), Utils.WIN_SEP)); + assertEquals(Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP), Utils.LINUX_SEP)); + assertEquals(Character.toString(Utils.WIN_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP), Utils.WIN_SEP)); + assertEquals(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP), Utils.LINUX_SEP)); + assertEquals(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.LINUX_SEP)); + assertEquals(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)+Character.toString(Utils.WIN_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.WIN_SEP)); + assertEquals("C:\\some\\Path\\"+Utils.LINUX_SEP, Utils.ensureTrailingPathSlash("C:\\some\\Path", Utils.LINUX_SEP)); + assertEquals("C:\\some\\Path\\"+Utils.WIN_SEP, Utils.ensureTrailingPathSlash("C:\\some\\Path", Utils.WIN_SEP)); + assertEquals("C:\\some\\Path\\" + Utils.LINUX_SEP, Utils.ensureTrailingPathSlash("C:\\some\\Path\\", Utils.LINUX_SEP)); + assertEquals("C:\\some\\Path\\", Utils.ensureTrailingPathSlash("C:\\some\\Path\\", Utils.WIN_SEP)); + assertEquals("/another/path" + Utils.LINUX_SEP, Utils.ensureTrailingPathSlash("/another/path", Utils.LINUX_SEP)); + assertEquals("/another/path/"+ Utils.WIN_SEP, Utils.ensureTrailingPathSlash("/another/path", Utils.WIN_SEP)); + assertEquals("/another/path/", Utils.ensureTrailingPathSlash("/another/path/", Utils.LINUX_SEP)); + assertEquals("/another/path/"+Utils.WIN_SEP, Utils.ensureTrailingPathSlash("/another/path/", Utils.WIN_SEP)); } @Test public void testEnsureTrailingSlashUseLinuxBoolTest() { - assertEquals(Utils.ensureTrailingPathSlash("", "true"), Character.toString(Utils.LINUX_SEP)); - assertEquals(Utils.ensureTrailingPathSlash("", "false"), Character.toString(Utils.WIN_SEP)); - assertEquals(Utils.ensureTrailingPathSlash("", "xyz"), Character.toString(Utils.WIN_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP), "false"), Character.toString(Utils.LINUX_SEP)+Utils.WIN_SEP); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP), "true"), Character.toString(Utils.LINUX_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP), "false"), Character.toString(Utils.WIN_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP), "true"), Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), "true"), Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)); - assertEquals(Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), "false"), Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)+Character.toString(Utils.WIN_SEP)); - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path", "true"), "C:\\some\\Path\\"+Utils.LINUX_SEP); - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path", "false"), "C:\\some\\Path\\"+Utils.WIN_SEP); - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path\\", "true"), "C:\\some\\Path\\" + Utils.LINUX_SEP); - assertEquals(Utils.ensureTrailingPathSlash("C:\\some\\Path\\", "false"), "C:\\some\\Path\\"); - assertEquals(Utils.ensureTrailingPathSlash("/another/path", "TRUE"), "/another/path" + Utils.LINUX_SEP); - assertEquals(Utils.ensureTrailingPathSlash("/another/path", "FALSE"), "/another/path/"+ Utils.WIN_SEP); - assertEquals(Utils.ensureTrailingPathSlash("/another/path/", "TrUe"), "/another/path/"); - assertEquals(Utils.ensureTrailingPathSlash("/another/path/", "FalSe"), "/another/path/"+Utils.WIN_SEP); + assertEquals(Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash("", "true")); + assertEquals(Character.toString(Utils.WIN_SEP), Utils.ensureTrailingPathSlash("", "false")); + assertEquals(Character.toString(Utils.WIN_SEP), Utils.ensureTrailingPathSlash("", "xyz")); + assertEquals(Character.toString(Utils.LINUX_SEP)+Utils.WIN_SEP, Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP), "false")); + assertEquals(Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.LINUX_SEP), "true")); + assertEquals(Character.toString(Utils.WIN_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP), "false")); + assertEquals(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP), "true")); + assertEquals(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), "true")); + assertEquals(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP)+Character.toString(Utils.WIN_SEP), Utils.ensureTrailingPathSlash(Character.toString(Utils.WIN_SEP)+Character.toString(Utils.LINUX_SEP), "false")); + assertEquals("C:\\some\\Path\\"+Utils.LINUX_SEP, Utils.ensureTrailingPathSlash("C:\\some\\Path", "true")); + assertEquals("C:\\some\\Path\\"+Utils.WIN_SEP, Utils.ensureTrailingPathSlash("C:\\some\\Path", "false")); + assertEquals("C:\\some\\Path\\" + Utils.LINUX_SEP, Utils.ensureTrailingPathSlash("C:\\some\\Path\\", "true")); + assertEquals("C:\\some\\Path\\", Utils.ensureTrailingPathSlash("C:\\some\\Path\\", "false")); + assertEquals("/another/path" + Utils.LINUX_SEP, Utils.ensureTrailingPathSlash("/another/path", "TRUE")); + assertEquals("/another/path/"+ Utils.WIN_SEP, Utils.ensureTrailingPathSlash("/another/path", "FALSE")); + assertEquals("/another/path/", Utils.ensureTrailingPathSlash("/another/path/", "TrUe")); + assertEquals("/another/path/"+Utils.WIN_SEP, Utils.ensureTrailingPathSlash("/another/path/", "FalSe")); } } From 70da8b640fc3d4d37a28c7fbc8a726f6d04a3b6c Mon Sep 17 00:00:00 2001 From: Gordon Smith Date: Thu, 7 Mar 2024 17:39:31 +0000 Subject: [PATCH 2/4] Split off 9.0.90 Signed-off-by: Gordon Smith --- commons-hpcc/pom.xml | 2 +- dfsclient/pom.xml | 2 +- pom.xml | 2 +- wsclient/pom.xml | 2 +- 4 files changed, 4 insertions(+), 4 deletions(-) diff --git a/commons-hpcc/pom.xml b/commons-hpcc/pom.xml index 4ab00f139..682896e7c 100644 --- a/commons-hpcc/pom.xml +++ b/commons-hpcc/pom.xml @@ -9,7 +9,7 @@ org.hpccsystems hpcc4j - 9.0.89-0-SNAPSHOT + 9.0.91-0-SNAPSHOT diff --git a/dfsclient/pom.xml b/dfsclient/pom.xml index e3881de01..27c639f62 100644 --- a/dfsclient/pom.xml +++ b/dfsclient/pom.xml @@ -9,7 +9,7 @@ org.hpccsystems hpcc4j - 9.0.89-0-SNAPSHOT + 9.0.91-0-SNAPSHOT diff --git a/pom.xml b/pom.xml index 83a3acbf2..54138d23f 100644 --- a/pom.xml +++ b/pom.xml @@ -4,7 +4,7 @@ 4.0.0 org.hpccsystems hpcc4j - 9.0.89-0-SNAPSHOT + 9.0.91-0-SNAPSHOT pom HPCC Systems Java Projects https://hpccsystems.com diff --git a/wsclient/pom.xml b/wsclient/pom.xml index cd172da0c..1585ce4d6 100644 --- a/wsclient/pom.xml +++ b/wsclient/pom.xml @@ -9,7 +9,7 @@ org.hpccsystems hpcc4j - 9.0.89-0-SNAPSHOT + 9.0.91-0-SNAPSHOT From 02ecaf0eaec829046d564c84bccf2c38dd1c82b7 Mon Sep 17 00:00:00 2001 From: James McMullan Date: Wed, 13 Mar 2024 09:34:21 -0400 Subject: [PATCH 3/4] HPCC4J-578 Migrate existing jirabot logic to new Jira environment (#692) - Updated Jirabot to work with current Jira and new Cloud Jira Signed-off-by: James McMullan James.McMullan@lexisnexis.com Signed-off-by: James McMullan James.McMullan@lexisnexis.com --- .github/workflows/Jirabot.yml | 69 +++++++++++++++++++++-------------- 1 file changed, 42 insertions(+), 27 deletions(-) diff --git a/.github/workflows/Jirabot.yml b/.github/workflows/Jirabot.yml index a5c8bd0c3..fd7f91013 100644 --- a/.github/workflows/Jirabot.yml +++ b/.github/workflows/Jirabot.yml @@ -39,28 +39,22 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} GHUB_JIRA_USER_MAP: ${{ vars.GHUB_JIRA_USER_MAP }} JIRA_ISSUE_PROPERTY_MAP: ${{ vars.JIRA_ISSUE_PROPERTY_MAP }} + JIRA_ISSUE_TRANSITION_MAP: ${{ vars.JIRA_ISSUE_TRANSITION_MAP }} run: | import os import re import json from jira.client import JIRA - def updateIssue(jira, issue, prAuthor: str, propertyMap: dict, pull_url: str) -> str: + def updateIssue(jira, issue, prAuthorEmail : str, transitionMap: dict, propertyMap: dict, pull_url: str) -> str: result = '' statusName = str(issue.fields.status) - if statusName == 'Open': - transition = 'Start Progress' - elif statusName == 'In Progress': - transition = '' - elif statusName == 'Resolved': - transition = 'Reopen Issue' - elif statusName == 'Closed': - transition = 'Reopen Issue' - else: - transition = '' + transition = transitionMap.get(statusName, None) - if transition != '': + if transition == None: + print('Error: Unable to find transition for status: ' + statusName) + elif transition != '': try: jira.transition_issue(issue, transition) result += 'Workflow Transition: ' + transition + '\n' @@ -81,13 +75,17 @@ jobs: elif currentPR is not None and currentPR != pull_url: result += 'Additional PR: ' + pull_url + '\n' - if prAuthor: + if prAuthorEmail: if issue.fields.assignee is None: - jira.assign_issue(issue, prAuthor) - result += 'Assigning user: ' + prAuthor + '\n' - elif issue.fields.assignee is not None and issue.fields.assignee.name.lower() != prAuthor.lower(): - result += 'Changing assignee from: ' + issue.fields.assignee.name + ' to: ' + prAuthor + '\n' - jira.assign_issue(issue, prAuthor) + jira.assign_issue(issue, prAuthorEmail) + result += 'Assigning user: ' + prAuthorEmail + '\n' + else: + assigneeEmail = None + if issue.fields.assignee: + assigneeEmail = issue.fields.assignee.emailAddress + if assigneeEmail is None or assigneeEmail.lower() != assigneeEmail.lower(): + result += 'Changing assignee from: ' + assigneeEmail + ' to: ' + prAuthorEmail + '\n' + jira.assign_issue(issue, prAuthorEmail) return result @@ -122,24 +120,41 @@ jobs: jira = JIRA(options=options, basic_auth=(jirabot_user, jirabot_pass)) - # Check if prAuthor exists in Jira - try: + # Need to change how we find users for Jira Cloud, unfortunately the API doesn't provide this information. + # At the moment checking if the URL contains atlassian.net appears to be the easiest way to determine if it's Jira Cloud. + isJiraCloud = False + if jira_url.find('atlassian.net') > 0: + isJiraCloud = True + + if isJiraCloud: + res = jira.search_users(query=prAuthor) + if res and len(res) > 0: + jiraUser = res[0] + else: jiraUser = jira.user(prAuthor) - if jiraUser is None: - prAuthor = None - print('Error: Unable to find Jira user: ' + prAuthor + ' continuing without assigning') - except Exception as error: - prAuthor = None - print('Error: Unable to find Jira user: ' + prAuthor + ' with error: ' + str(error) + ' continuing without assigning') + + jiraUserEmail = None + if jiraUser is None: + print('Error: Unable to find Jira user: ' + prAuthor + ' continuing without assigning') + else: + jiraUserEmail = jiraUser.emailAddress + + print("Jira User Email:" + str(jiraUserEmail)) issue = jira.issue(issue_name) result = 'Jirabot Action Result:\n' + transitionMap = json.loads(os.environ['JIRA_ISSUE_TRANSITION_MAP']) + if not isinstance(transitionMap, dict): + print('Error: JIRA_ISSUE_TRANSITION_MAP is not a valid JSON object, ignoring.') + transitionMap = {} + jiraIssuePropertyMap = json.loads(os.environ['JIRA_ISSUE_PROPERTY_MAP']) if not isinstance(jiraIssuePropertyMap, dict): + print('Error: JIRA_ISSUE_PROPERTY_MAP is not a valid JSON object, ignoring.') jiraIssuePropertyMap = {} - result += updateIssue(jira, issue, prAuthor, jiraIssuePropertyMap, pull_url) + result += updateIssue(jira, issue, jiraUserEmail, transitionMap, jiraIssuePropertyMap, pull_url) jira.add_comment(issue, result) else: print('Unable to find Jira issue name in title') From 8fd9fb8e8279782e7780830d3b8a0e325ec497f3 Mon Sep 17 00:00:00 2001 From: James McMullan Date: Thu, 14 Mar 2024 09:51:45 -0400 Subject: [PATCH 4/4] HPCC4J-588 HpccRemoteFileReader doesn't pass constuctor params (#694) - Fixed issue where some constructor params were not being passed from overloaded constructors Signed-off-by: James McMullan James.McMullan@lexisnexis.com Signed-off-by: James McMullan James.McMullan@lexisnexis.com --- .../dfs/client/HpccRemoteFileReader.java | 44 +++++++++---------- 1 file changed, 22 insertions(+), 22 deletions(-) diff --git a/dfsclient/src/main/java/org/hpccsystems/dfs/client/HpccRemoteFileReader.java b/dfsclient/src/main/java/org/hpccsystems/dfs/client/HpccRemoteFileReader.java index 716e6c5e8..7913e9748 100644 --- a/dfsclient/src/main/java/org/hpccsystems/dfs/client/HpccRemoteFileReader.java +++ b/dfsclient/src/main/java/org/hpccsystems/dfs/client/HpccRemoteFileReader.java @@ -21,7 +21,7 @@ import java.util.Iterator; /** - * Remote file reader the reads the data represented by a @see org.hpccsystems.dfs.client.DataPartition + * Remote file reader the reads the data represented by a @see org.hpccsystems.dfs.client.DataPartition * and constructs records via the provided @see org.hpccsystems.dfs.client#IRecordBuilder. */ public class HpccRemoteFileReader implements Iterator @@ -72,7 +72,7 @@ public HpccRemoteFileReader(DataPartition dp, FieldDef originalRD, IRecordBuilde * the record defintion for the dataset * @param recBuilder * the IRecordBuilder used to construct records - * @param connectTimeout + * @param connectTimeout * the connection timeout in seconds, -1 for default * @throws Exception * the exception @@ -105,47 +105,47 @@ public HpccRemoteFileReader(DataPartition dp, FieldDef originalRD, IRecordBuilde /** * A remote file reader that reads the part identified by the HpccPart object using the record definition provided. - * + * * @param dp * the part of the file, name and location * @param originalRD * the record defintion for the dataset * @param recBuilder * the IRecordBuilder used to construct records - * @param connectTimeout + * @param connectTimeout * the connection timeout in seconds, -1 for default - * @param limit + * @param limit * the maximum number of records to read from the provided data partition, -1 specifies no limit - * @param createPrefetchThread + * @param createPrefetchThread * the input stream should create and manage prefetching on its own thread. If false prefetch needs to be called on another thread periodically. - * @param readSizeKB + * @param readSizeKB * read request size in KB, -1 specifies use default value * @throws Exception * general exception */ public HpccRemoteFileReader(DataPartition dp, FieldDef originalRD, IRecordBuilder recBuilder, int connectTimeout, int limit, boolean createPrefetchThread, int readSizeKB) throws Exception { - this(dp, originalRD, recBuilder, connectTimeout, limit, true, DEFAULT_READ_SIZE_OPTION, null); + this(dp, originalRD, recBuilder, connectTimeout, limit, createPrefetchThread, readSizeKB, null); } /** * A remote file reader that reads the part identified by the HpccPart object using the record definition provided. - * + * * @param dp * the part of the file, name and location * @param originalRD * the record defintion for the dataset * @param recBuilder * the IRecordBuilder used to construct records - * @param connectTimeout + * @param connectTimeout * the connection timeout in seconds, -1 for default - * @param limit + * @param limit * the maximum number of records to read from the provided data partition, -1 specifies no limit - * @param createPrefetchThread + * @param createPrefetchThread * the input stream should create and manage prefetching on its own thread. If false prefetch needs to be called on another thread periodically. - * @param readSizeKB + * @param readSizeKB * read request size in KB, -1 specifies use default value - * @param resumeInfo + * @param resumeInfo * FileReadeResumeInfo data required to restart a read from a particular point in a file * @throws Exception * general exception @@ -204,7 +204,7 @@ public HpccRemoteFileReader(DataPartition dp, FieldDef originalRD, IRecordBuilde /** * Returns the stream position within the file. - * + * * @return stream position */ public long getStreamPosition() @@ -214,7 +214,7 @@ public long getStreamPosition() /** * Returns read resume info for the current position within the file. - * + * * @return FileReadResumeInfo */ public FileReadResumeInfo getFileReadResumeInfo() @@ -224,7 +224,7 @@ public FileReadResumeInfo getFileReadResumeInfo() /** * Returns read resume info for the specified position within the file. - * + * * @param streamPosition the stream position to resume from * @return FileReadResumeInfo */ @@ -242,7 +242,7 @@ public FileReadResumeInfo getFileReadResumeInfo(Long streamPosition) /** * Returns the number of messages created during the reading process - * + * * @return number of messages created */ public int getRemoteReadMessageCount() @@ -256,7 +256,7 @@ public int getRemoteReadMessageCount() /** * Returns messages created during the file reading process - * + * * @return Messages concatenated into a String */ public String getRemoteReadMessages() @@ -284,7 +284,7 @@ public void prefetch() /** * Is there more data - * + * * @return true if there is a next record */ @Override @@ -352,7 +352,7 @@ public int getAvailable() throws IOException /** * Returns the RowServiceInputStream used to read the file from dafilesrv - * + * * @return the input stream */ public RowServiceInputStream getInputStream() @@ -362,7 +362,7 @@ public RowServiceInputStream getInputStream() /** * Returns the BinaryRecordReader used to construct records - * + * * @return the record reader */ public BinaryRecordReader getRecordReader()