EPMRPP-115034 || add Jira client commands and update dependencies - #90
Conversation
|
Warning Review limit reached
More reviews will be available in 34 minutes and 57 seconds. Learn how PR review limits work. Your organization has used up its prepaid credits, and credit purchases are no longer available. Enable the review add-on in the billing tab to keep reviews running — you're only billed for reviews past your plan's rate limits ($0.25/file). ⌛ How to resolve this issue?After more reviews become available, a review can be triggered using the To avoid repeated limits, reduce automatic review volume by pausing incremental auto-reviews earlier, using label-based review opt-in, excluding WIP or generated PR titles, or requesting reviews manually when the PR is ready. If your team needs uninterrupted high-volume reviews, an organization admin can enable usage-based credits. 🚦 How do rate limits work?CodeRabbit enforces per-developer PR review limits for each organization. Most developers receive the normal plan review availability. For paid Pro and Pro+ PR reviews, CodeRabbit uses adaptive limits for sustained high-volume activity. When a developer's recent PR review activity reaches the 95th percentile or higher among CodeRabbit users, additional reviews become available more gradually as earlier reviews age out of the rolling window. Please see our Fair Usage Limits Policy for further information. ℹ️ Review info⚙️ Run configurationConfiguration used: Organization UI Review profile: CHILL Plan: Pro Run ID: 📒 Files selected for processing (1)
WalkthroughThe PR converts the Jira bugtracking integration from direct strategy methods to extension commands. It adds commands for connection checks, issue lookup, issue metadata, and ticket creation, updates shared Jira client and ticket conversion helpers, and rewrites the command tests. ChangesJira command migration
Sequence DiagramsequenceDiagram
participant PostTicketCommand
participant JiraClientProvider
participant JiraRestClient
participant DataStoreService
participant JIRATicketUtils
PostTicketCommand->>JiraClientProvider: provide(IntegrationParams)
JiraClientProvider-->>PostTicketCommand: JiraRestClient
PostTicketCommand->>JiraRestClient: issuesApi().createIssue(...)
PostTicketCommand->>DataStoreService: load(binary token)
PostTicketCommand->>JiraRestClient: attachmentsApi().addAttachment(...)
PostTicketCommand->>JiraRestClient: issuesApi().getIssue(...)
PostTicketCommand->>JIRATicketUtils: toTicket(issue, jiraUrl, objectMapper)
JIRATicketUtils-->>PostTicketCommand: Ticket
Estimated code review effort🎯 4 (Complex) | ⏱️ ~60 minutes Poem
🚥 Pre-merge checks | ✅ 4 | ❌ 1❌ Failed checks (1 warning)
✅ Passed checks (4 passed)
✨ Finishing Touches🧪 Generate unit tests (beta)
Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out. Comment |
![coderabbitai[bot]](https://avatars.githubusercontent.com/in/347564?s=60&v=4){kind=small}
There was a problem hiding this comment.
Actionable comments posted: 10
🤖 Prompt for all review comments with AI agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
Inline comments:
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueCommand.java`:
- Around line 64-65: The JQL built in GetIssueCommand.searchForIssue currently
concatenates ticketId directly into the query, which allows operator injection.
Update the issue search query to quote the ticketId value and escape any
embedded double quotes before passing it to
client.issueSearchApi().searchForIssuesUsingJql, so the constructed JQL stays as
a single issue identifier. Use GetIssueCommand and the searchForIssuesUsingJql
call as the locations to fix.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueFieldsCommand.java`:
- Around line 93-96: The Jira issue field parsing in GetIssueFieldsCommand
should not blindly cast the external “values” payload from
issueCreateMetadata.getAdditionalProperties(). Validate that the “values” entry
exists and is a list before iterating, and handle missing or unexpected shapes
as an integration error instead of letting a NullPointerException or
ClassCastException escape. In the stream setup inside GetIssueFieldsCommand,
remove the redundant convertValue step and process the map objects directly once
the type has been verified, using the existing issueCreateMetadata and
objectMapper flow.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommand.java`:
- Around line 187-214: The attachment upload path in
PostTicketCommand.addAttachment is using the raw JiraProps.PASSWORD value
instead of the decrypted password, so Basic auth is built with the wrong
credential. Update the credential retrieval in this method to decrypt the
password the same way JiraClientProvider.provide(...) does before constructing
the Authorization header, and keep the rest of the upload flow unchanged.
- Around line 156-164: The issue in PostTicketCommand is that fetch failure is
hidden by returning null after the Jira search, which can cause later NPEs and
is inconsistent with GetIssueCommand. Update the logic in the issue lookup flow
so that when client.issuesApi().getIssue(...) cannot retrieve the created issue
after search results indicate it exists, it throws a ReportPortalException with
the integration error instead of returning null. Keep the change localized to
the search/get path around JIRATicketUtils.toTicket and the jiraUrl resolution.
- Around line 109-126: The issue is that PostTicketCommand dereferences
issueType before confirming it was actually found in fields, so an empty list or
missing issuetype can fail with an uncontrolled exception. Update the validation
in PostTicketCommand to first ensure the issuetype field exists and has a value
before calling getValue() or get(0), and return the existing
UNABLE_INTERACT_WITH_INTEGRATION error through the same verification flow if it
is missing or empty. Use the issueType and fields lookup logic in the
PostTicketCommand loop as the place to add this guard.
- Around line 215-216: The manual attachment upload in PostTicketCommand uses a
default HttpClient without explicit timeout settings, so update the
HttpClients.createDefault() usage to a configured client built from
HttpClients.custom() with a RequestConfig applied. In the attachment upload path
inside PostTicketCommand, set connection and response/request timeouts on the
client before httpClient.execute(...) so slow Jira responses do not block the
command indefinitely.
- Around line 198-200: The attachment-loading logic in PostTicketCommand does
not close the InputStream returned by dataStoreService.load, so update the
data.get() handling to use try-with-resources around the loaded stream before
calling IOUtils.toByteArray; keep the existing Optional check, but ensure the
stream is automatically closed after reading the bytes.
- Around line 208-224: The attachment upload in PostTicketCommand is happening
inside the file iteration while reusing the same multipart builder, which causes
previously added files to be sent again on later passes. Move the HttpPost
creation and httpClient.execute upload logic out of the attachment collection
loop so all files are added first, then perform a single upload for the
issueKey. Keep the fix localized around the attachment-building flow in
PostTicketCommand and ensure the multipart entity is built only once after all
attachments are gathered.
In
`@src/test/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommandTest.java`:
- Around line 97-100: The addAttachmentTest in PostTicketCommandTest is only a
placeholder and always passes, so either remove this empty test or replace it
with real coverage for the attachment flow through the relevant
postTicket/addAttachment path. If you keep it, make sure it asserts the
credential handling, upload loop, and stream handling behavior in the production
attachment logic instead of just containing a comment.
- Around line 92-94: In PostTicketCommandTest, the test is dereferencing the
returned Ticket before asserting it is non-null and it no longer validates the
mapped fields. Update the assertion order so the null check happens before using
ticket.getTicketUrl(), and restore the field-level assertions in the test for
the Ticket returned by command.invokeCommand(INTEGRATION, rq), including ticket
URL, ticket id, and status.
🪄 Autofix (Beta)
Fix all unresolved CodeRabbit comments on this PR:
- Push a commit to this branch (recommended)
- Create a new PR with the fixes
ℹ️ Review info
⚙️ Run configuration
Configuration used: Organization UI
Review profile: CHILL
Plan: Pro
Run ID: 017dab35-1a68-4949-9c10-e45f011a9454
📒 Files selected for processing (15)
build.gradlesrc/main/java/com/epam/reportportal/extension/bugtracking/jira/JIRATicketUtils.javasrc/main/java/com/epam/reportportal/extension/bugtracking/jira/JiraStrategy.javasrc/main/java/com/epam/reportportal/extension/bugtracking/jira/client/JiraClientProvider.javasrc/main/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueCommand.javasrc/main/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueFieldsCommand.javasrc/main/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueTypesCommand.javasrc/main/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommand.javasrc/main/java/com/epam/reportportal/extension/bugtracking/jira/command/TestConnectionCommand.javasrc/test/java/com/epam/reportportal/extension/bugtracking/jira/command/BaseCommandTest.javasrc/test/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueCommandTest.javasrc/test/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueFieldsCommandTest.javasrc/test/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueTypesCommandTest.javasrc/test/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommandTest.javasrc/test/java/com/epam/reportportal/extension/bugtracking/jira/command/TestConnectionCommandTest.java
| SearchResults results = client.issueSearchApi() | ||
| .searchForIssuesUsingJql("issue = " + ticketId, null, 50, "", null, null, null, false, false); |
There was a problem hiding this comment.
🔒 Security & Privacy | 🟡 Minor
🧩 Analysis chain
🌐 Web query:
Jira REST API searchForIssuesUsingJql JQL escaping special characters values
💡 Result:
When working with the Jira REST API, there are two distinct layers where escaping is required: the transport layer (URL encoding for GET requests) and the query syntax layer (JQL-specific escaping). 1. REST API Transport (URL Encoding) When using GET requests to the /rest/api/3/search/jql endpoint, the entire JQL string must be URL-encoded [1]. If your JQL string contains special characters like spaces, quotes, or ampersands, they must be converted to their percent-encoded equivalents (e.g., a space becomes %20, & becomes %26) [1]. To avoid URL encoding complexities and length limitations, it is recommended to use POST requests instead [1][2]. In a POST request, the JQL is passed as a JSON object field, which eliminates the need to URL-encode the JQL string itself [1]. 2. JQL Syntax Escaping Within the JQL string itself, specific characters require handling to be interpreted correctly: - Exact Phrase Matching: To search for an exact phrase containing special characters or spaces, enclose the value in double quotes and nest additional escaped double quotes around the phrase [3][4]. For example, to search for "full screen" in a summary, use: summary ~ ""full screen"" [3][4]. - Reserved Characters: Jira's text-search index treats characters like +, -, &, |,!, (,), {, }, [, ], ^, ~, *,?, , and: as reserved [5][6]. These characters are often ignored or treated as operators by the search index; they cannot typically be "escaped" in a way that forces them to be stored or indexed as part of the text [5][6][7]. - Valid Escape Sequences: If you are constructing JQL strings programmatically, Jira recognizes standard escape sequences within strings, such as: ' (single quote), " (double quote), \t (tab), \n (newline), \r (carriage return), and \ (backslash) [8]. Summary of Best Practices: - Prefer POST over GET to avoid URL encoding issues [1]. - Use nested double quotes (e.g., ""value"") for phrase searches [3][4]. - Be aware that symbols like - or & are often stripped or treated as operators by the underlying Lucene index and cannot be escaped to force an exact match in free-text fields [5][6][9].
Citations:
- 1: https://confluence.atlassian.com/jirakb/run-jql-search-query-using-jira-cloud-rest-api-1289424308.html
- 2: https://developer.atlassian.com/cloud/jira/platform/rest/v3/api-group-issue-search/
- 3: https://confluence.atlassian.com/display/JIRACORESERVER/Advanced+searching+-+operators+reference
- 4: https://confluence.atlassian.com/jirasoftwareserver073/advanced-searching-operators-reference-861256239.html
- 5: https://jira.atlassian.com/browse/JRASERVER-63730
- 6: https://confluence.atlassian.com/servicemanagementserver0417/search-syntax-for-text-fields-1072469088.html
- 7: https://community.atlassian.com/forums/Jira-Service-Management/Jira-special-characters-in-a-JQL/qaq-p/1840793
- 8: https://community.atlassian.com/forums/Jira-questions/Escape-Character/qaq-p/1950673
- 9: https://stackoverflow.com/questions/49034233/jira-jql-search-not-working-with-special-character-or
Potential JQL injection: ticketId is not quoted
The ticketId value is concatenated directly into the JQL string without quoting. An input like ISSUE-123 OR project = "Public" executes unintended logic, bypassing the intended filter because JQL interprets unquoted tokens as operators or field names.
Wrap the value in double quotes and escape internal quotes (e.g., issue = \"" + ticketId.replace(""", "\"") + \""). Even standard issue keys should be quoted to handle special characters or spaces and prevent operator injection.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueCommand.java`
around lines 64 - 65, The JQL built in GetIssueCommand.searchForIssue currently
concatenates ticketId directly into the query, which allows operator injection.
Update the issue search query to quote the ticketId value and escape any
embedded double quotes before passing it to
client.issueSearchApi().searchForIssuesUsingJql, so the constructed JQL stays as
a single issue identifier. Use GetIssueCommand and the searchForIssuesUsingJql
call as the locations to fix.
| List<PostFormField> result = new ArrayList<>(); | ||
| ((List<Map<String, Object>>) issueCreateMetadata.getAdditionalProperties().get("values")).stream() | ||
| .map(a -> objectMapper.convertValue(a, new TypeReference<Map<String, Object>>() {})) | ||
| .map(field -> (JsonNode) objectMapper.valueToTree(field)) |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
#!/bin/bash
rg -nP 'class PageOfCreateMetaIssueTypeWithField' -g '*.java'
fd -e java PageOfCreateMetaIssueTypeWithField --exec cat -n {}Repository: reportportal/plugin-bts-jira
Length of output: 166
🏁 Script executed:
rg -nP '..getCreateIssueMetaIssueTypeId..|getCreateIssueMeta' -g '*GetIssueFieldsCommand.java' --type javaRepository: reportportal/plugin-bts-jira
Length of output: 166
🏁 Script executed:
rg -P 'class .*AdditionalProperties|class .*CreateMeta' -g '*.java' --type javaRepository: reportportal/plugin-bts-jira
Length of output: 166
🏁 Script executed:
fd -e java "GetIssueFieldsCommand" --exec cat -n {}Repository: reportportal/plugin-bts-jira
Length of output: 12104
Missing null/shape check before unchecked cast on external API data
The code at line 94 performs an unchecked cast ((List<Map<String, Object>>) ...) directly on the result of get("values") from issueCreateMetadata. If the Jira API response omits the "values" key or returns a different type, this throws a NullPointerException or ClassCastException at runtime, causing an opaque 500 error instead of a handled integration failure.
Additionally, the convertValue call on line 95 is redundant since the cast already assumes the element is a Map<String, Object>, which valueToTree can process directly.
Add a safe guard to validate the presence and type of the "values" entry before processing:
Proposed fix
- List<PostFormField> result = new ArrayList<>();
- ((List<Map<String, Object>>) issueCreateMetadata.getAdditionalProperties().get("values")).stream()
- .map(a -> objectMapper.convertValue(a, new TypeReference<Map<String, Object>>() {}))
- .map(field -> (JsonNode) objectMapper.valueToTree(field))
+ List<PostFormField> result = new ArrayList<>();
+ Object values = issueCreateMetadata.getAdditionalProperties().get("values");
+ if (!(values instanceof List<?> valueList)) {
+ throw new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION,
+ "Create issue metadata does not contain field values for issue type '" + ticketType + "'");
+ }
+ valueList.stream()
+ .map(field -> (JsonNode) objectMapper.valueToTree(field))
.forEach(jsonField -> {📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| List<PostFormField> result = new ArrayList<>(); | |
| ((List<Map<String, Object>>) issueCreateMetadata.getAdditionalProperties().get("values")).stream() | |
| .map(a -> objectMapper.convertValue(a, new TypeReference<Map<String, Object>>() {})) | |
| .map(field -> (JsonNode) objectMapper.valueToTree(field)) | |
| List<PostFormField> result = new ArrayList<>(); | |
| Object values = issueCreateMetadata.getAdditionalProperties().get("values"); | |
| if (!(values instanceof List<?> valueList)) { | |
| throw new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION, | |
| "Create issue metadata does not contain field values for issue type '" + ticketType + "'"); | |
| } | |
| valueList.stream() | |
| .map(field -> (JsonNode) objectMapper.valueToTree(field)) | |
| .forEach(jsonField -> { |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/GetIssueFieldsCommand.java`
around lines 93 - 96, The Jira issue field parsing in GetIssueFieldsCommand
should not blindly cast the external “values” payload from
issueCreateMetadata.getAdditionalProperties(). Validate that the “values” entry
exists and is a list before iterating, and handle missing or unexpected shapes
as an integration error instead of letting a NullPointerException or
ClassCastException escape. In the stream setup inside GetIssueFieldsCommand,
remove the redundant convertValue step and process the map objects directly once
the type has been verified, using the existing issueCreateMetadata and
objectMapper flow.
| PostFormField issueType = new PostFormField(); | ||
| PostFormField components = new PostFormField(); | ||
| for (PostFormField field : fields) { | ||
| if ("issuetype".equalsIgnoreCase(field.getId())) { | ||
| issueType = field; | ||
| } | ||
| if ("components".equalsIgnoreCase(field.getId())) { | ||
| components = field; | ||
| } | ||
| } | ||
|
|
||
| expect(issueType.getValue().size(), | ||
| com.epam.reportportal.base.infrastructure.persistence.commons.Predicates.equalTo(1)) | ||
| .verify(UNABLE_INTERACT_WITH_INTEGRATION, | ||
| Suppliers.formattedSupplier("[IssueType] field has multiple values '{}' but should be only one", | ||
| issueType.getValue())); | ||
|
|
||
| final String issueTypeStr = issueType.getValue().get(0); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🔴 Critical | ⚡ Quick win
Validate the required issue type before dereferencing it.
If fields is empty or does not contain issuetype, issueType.getValue().size() can throw before returning a controlled integration error.
Suggested fix
- PostFormField issueType = new PostFormField();
+ PostFormField issueType = null;
PostFormField components = new PostFormField();
for (PostFormField field : fields) {
if ("issuetype".equalsIgnoreCase(field.getId())) {
issueType = field;
}
@@
- expect(issueType.getValue().size(),
+ expect(issueType, not(isNull()))
+ .verify(UNABLE_INTERACT_WITH_INTEGRATION, "[IssueType] field is required");
+ expect(issueType.getValue(), not(isNull()))
+ .verify(UNABLE_INTERACT_WITH_INTEGRATION, "[IssueType] field is required");
+ expect(issueType.getValue().size(),
com.epam.reportportal.base.infrastructure.persistence.commons.Predicates.equalTo(1))📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| PostFormField issueType = new PostFormField(); | |
| PostFormField components = new PostFormField(); | |
| for (PostFormField field : fields) { | |
| if ("issuetype".equalsIgnoreCase(field.getId())) { | |
| issueType = field; | |
| } | |
| if ("components".equalsIgnoreCase(field.getId())) { | |
| components = field; | |
| } | |
| } | |
| expect(issueType.getValue().size(), | |
| com.epam.reportportal.base.infrastructure.persistence.commons.Predicates.equalTo(1)) | |
| .verify(UNABLE_INTERACT_WITH_INTEGRATION, | |
| Suppliers.formattedSupplier("[IssueType] field has multiple values '{}' but should be only one", | |
| issueType.getValue())); | |
| final String issueTypeStr = issueType.getValue().get(0); | |
| PostFormField issueType = null; | |
| PostFormField components = new PostFormField(); | |
| for (PostFormField field : fields) { | |
| if ("issuetype".equalsIgnoreCase(field.getId())) { | |
| issueType = field; | |
| } | |
| if ("components".equalsIgnoreCase(field.getId())) { | |
| components = field; | |
| } | |
| } | |
| expect(issueType, not(isNull())) | |
| .verify(UNABLE_INTERACT_WITH_INTEGRATION, "[IssueType] field is required"); | |
| expect(issueType.getValue(), not(isNull())) | |
| .verify(UNABLE_INTERACT_WITH_INTEGRATION, "[IssueType] field is required"); | |
| expect(issueType.getValue().size(), | |
| com.epam.reportportal.base.infrastructure.persistence.commons.Predicates.equalTo(1)) | |
| .verify(UNABLE_INTERACT_WITH_INTEGRATION, | |
| Suppliers.formattedSupplier("[IssueType] field has multiple values '{}' but should be only one", | |
| issueType.getValue())); | |
| final String issueTypeStr = issueType.getValue().get(0); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommand.java`
around lines 109 - 126, The issue is that PostTicketCommand dereferences
issueType before confirming it was actually found in fields, so an empty list or
missing issuetype can fail with an uncontrolled exception. Update the validation
in PostTicketCommand to first ensure the issuetype field exists and has a value
before calling getValue() or get(0), and return the existing
UNABLE_INTERACT_WITH_INTEGRATION error through the same verification flow if it
is missing or empty. Use the issueType and fields lookup logic in the
PostTicketCommand loop as the place to add this guard.
| SearchResults results = client.issueSearchApi() | ||
| .searchForIssuesUsingJql("issue = " + issueKey, null, 50, "", null, null, null, false, false); | ||
| if (results.getTotal() > 0) { | ||
| IssueBean issue = client.issuesApi().getIssue(issueKey, null, null, null, null, null, null); | ||
| String jiraUrl = JiraProps.URL.getParam(params) | ||
| .orElseThrow(() -> new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION, "Url is not specified.")); | ||
| return JIRATicketUtils.toTicket(issue, jiraUrl, objectMapper); | ||
| } | ||
| return null; |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Throw when the created issue cannot be fetched.
Returning null makes downstream callers fail later with an NPE and differs from GetIssueCommand, which raises a clear integration error when Jira search returns no issue.
Suggested fix
if (results.getTotal() > 0) {
IssueBean issue = client.issuesApi().getIssue(issueKey, null, null, null, null, null, null);
String jiraUrl = JiraProps.URL.getParam(params)
.orElseThrow(() -> new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION, "Url is not specified."));
return JIRATicketUtils.toTicket(issue, jiraUrl, objectMapper);
}
- return null;
+ throw new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION, "Ticket not found: " + issueKey);📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| SearchResults results = client.issueSearchApi() | |
| .searchForIssuesUsingJql("issue = " + issueKey, null, 50, "", null, null, null, false, false); | |
| if (results.getTotal() > 0) { | |
| IssueBean issue = client.issuesApi().getIssue(issueKey, null, null, null, null, null, null); | |
| String jiraUrl = JiraProps.URL.getParam(params) | |
| .orElseThrow(() -> new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION, "Url is not specified.")); | |
| return JIRATicketUtils.toTicket(issue, jiraUrl, objectMapper); | |
| } | |
| return null; | |
| SearchResults results = client.issueSearchApi() | |
| .searchForIssuesUsingJql("issue = " + issueKey, null, 50, "", null, null, null, false, false); | |
| if (results.getTotal() > 0) { | |
| IssueBean issue = client.issuesApi().getIssue(issueKey, null, null, null, null, null, null); | |
| String jiraUrl = JiraProps.URL.getParam(params) | |
| .orElseThrow(() -> new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION, "Url is not specified.")); | |
| return JIRATicketUtils.toTicket(issue, jiraUrl, objectMapper); | |
| } | |
| throw new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION, "Ticket not found: " + issueKey); |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommand.java`
around lines 156 - 164, The issue in PostTicketCommand is that fetch failure is
hidden by returning null after the Jira search, which can cause later NPEs and
is inconsistent with GetIssueCommand. Update the logic in the issue lookup flow
so that when client.issuesApi().getIssue(...) cannot retrieve the created issue
after search results indicate it exists, it throws a ReportPortalException with
the integration error instead of returning null. Keep the change localized to
the search/get path around JIRATicketUtils.toTicket and the jiraUrl resolution.
| String username = JiraProps.USER_NAME.getParam(params) | ||
| .orElseThrow(() -> new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION, "Username is not specified.")); | ||
| String password = JiraProps.PASSWORD.getParam(params) | ||
| .orElseThrow(() -> new ReportPortalException(UNABLE_INTERACT_WITH_INTEGRATION, "Password is not specified.")); | ||
|
|
||
| int count = 0; | ||
| MultipartEntityBuilder entityBuilder = MultipartEntityBuilder.create() | ||
| .setLaxMode() | ||
| .setCharset(StandardCharsets.UTF_8); | ||
|
|
||
| for (Map.Entry<String, String> entry : binaryData.entrySet()) { | ||
| Optional<InputStream> data = dataStoreService.load(entry.getKey()); | ||
| if (data.isPresent()) { | ||
| byte[] bytes = IOUtils.toByteArray(data.get()); | ||
| if (bytes.length == 0) { | ||
| LOGGER.warn("Empty file {}", entry.getValue()); | ||
| continue; | ||
| } | ||
| entityBuilder.addPart("file", new ByteArrayBody(bytes, entry.getValue())); | ||
| count++; | ||
| } | ||
| if (count > 0) { | ||
| HttpPost request = new HttpPost(url + String.format("/rest/api/latest/issue/%s/attachments", issueKey)); | ||
| request.setEntity(entityBuilder.build()); | ||
| request.setHeader("X-Atlassian-Token", "no-check"); | ||
| String plainCreds = username + ":" + password; | ||
| request.setHeader("Authorization", | ||
| "Basic " + new String(Base64.encodeBase64(plainCreds.getBytes(StandardCharsets.UTF_8)))); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟠 Major | ⚡ Quick win
Use the decrypted Jira password for attachment uploads.
JiraClientProvider.provide(...) decrypts JiraProps.PASSWORD before authenticating, but addAttachment sends the raw stored value in Basic auth, so attachment uploads will fail for encrypted integration params.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommand.java`
around lines 187 - 214, The attachment upload path in
PostTicketCommand.addAttachment is using the raw JiraProps.PASSWORD value
instead of the decrypted password, so Basic auth is built with the wrong
credential. Update the credential retrieval in this method to decrypt the
password the same way JiraClientProvider.provide(...) does before constructing
the Authorization header, and keep the rest of the upload flow unchanged.
| Optional<InputStream> data = dataStoreService.load(entry.getKey()); | ||
| if (data.isPresent()) { | ||
| byte[] bytes = IOUtils.toByteArray(data.get()); |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major | ⚡ Quick win
Close loaded attachment streams.
IOUtils.toByteArray(data.get()) consumes the stream but does not close it; wrap the loaded stream in try-with-resources.
Suggested fix
Optional<InputStream> data = dataStoreService.load(entry.getKey());
if (data.isPresent()) {
- byte[] bytes = IOUtils.toByteArray(data.get());
+ byte[] bytes;
+ try (InputStream inputStream = data.get()) {
+ bytes = IOUtils.toByteArray(inputStream);
+ }📝 Committable suggestion
‼️ IMPORTANT
Carefully review the code before committing. Ensure that it accurately replaces the highlighted code, contains no missing lines, and has no issues with indentation. Thoroughly test & benchmark the code to ensure it meets the requirements.
| Optional<InputStream> data = dataStoreService.load(entry.getKey()); | |
| if (data.isPresent()) { | |
| byte[] bytes = IOUtils.toByteArray(data.get()); | |
| Optional<InputStream> data = dataStoreService.load(entry.getKey()); | |
| if (data.isPresent()) { | |
| byte[] bytes; | |
| try (InputStream inputStream = data.get()) { | |
| bytes = IOUtils.toByteArray(inputStream); | |
| } |
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommand.java`
around lines 198 - 200, The attachment-loading logic in PostTicketCommand does
not close the InputStream returned by dataStoreService.load, so update the
data.get() handling to use try-with-resources around the loaded stream before
calling IOUtils.toByteArray; keep the existing Optional check, but ensure the
stream is automatically closed after reading the bytes.
| if (count > 0) { | ||
| HttpPost request = new HttpPost(url + String.format("/rest/api/latest/issue/%s/attachments", issueKey)); | ||
| request.setEntity(entityBuilder.build()); | ||
| request.setHeader("X-Atlassian-Token", "no-check"); | ||
| String plainCreds = username + ":" + password; | ||
| request.setHeader("Authorization", | ||
| "Basic " + new String(Base64.encodeBase64(plainCreds.getBytes(StandardCharsets.UTF_8)))); | ||
| try (CloseableHttpClient httpClient = HttpClients.createDefault()) { | ||
| httpClient.execute(request, response -> { | ||
| if (!(response.getCode() >= 200 && response.getCode() < 300)) { | ||
| LOGGER.error("{} {}", response.getCode(), response.getReasonPhrase()); | ||
| throw new ReportPortalException("Failed to upload attachment for " + issueKey); | ||
| } | ||
| return response; | ||
| }); | ||
| } | ||
| } |
There was a problem hiding this comment.
🗄️ Data Integrity & Integration | 🟠 Major | ⚡ Quick win
Move the upload request outside the attachment loop.
The current placement uploads after each found file while reusing the same multipart builder, so later iterations can re-upload earlier attachments.
Suggested fix
- if (count > 0) {
- HttpPost request = new HttpPost(url + String.format("/rest/api/latest/issue/%s/attachments", issueKey));
- request.setEntity(entityBuilder.build());
- request.setHeader("X-Atlassian-Token", "no-check");
- String plainCreds = username + ":" + password;
- request.setHeader("Authorization",
- "Basic " + new String(Base64.encodeBase64(plainCreds.getBytes(StandardCharsets.UTF_8))));
- try (CloseableHttpClient httpClient = HttpClients.createDefault()) {
- httpClient.execute(request, response -> {
- if (!(response.getCode() >= 200 && response.getCode() < 300)) {
- LOGGER.error("{} {}", response.getCode(), response.getReasonPhrase());
- throw new ReportPortalException("Failed to upload attachment for " + issueKey);
- }
- return response;
- });
- }
- }
}
+ if (count > 0) {
+ HttpPost request = new HttpPost(url + String.format("/rest/api/latest/issue/%s/attachments", issueKey));
+ request.setEntity(entityBuilder.build());
+ request.setHeader("X-Atlassian-Token", "no-check");
+ String plainCreds = username + ":" + password;
+ request.setHeader("Authorization",
+ "Basic " + new String(Base64.encodeBase64(plainCreds.getBytes(StandardCharsets.UTF_8))));
+ try (CloseableHttpClient httpClient = HttpClients.createDefault()) {
+ httpClient.execute(request, response -> {
+ if (!(response.getCode() >= 200 && response.getCode() < 300)) {
+ LOGGER.error("{} {}", response.getCode(), response.getReasonPhrase());
+ throw new ReportPortalException("Failed to upload attachment for " + issueKey);
+ }
+ return response;
+ });
+ }
+ }🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommand.java`
around lines 208 - 224, The attachment upload in PostTicketCommand is happening
inside the file iteration while reusing the same multipart builder, which causes
previously added files to be sent again on later passes. Move the HttpPost
creation and httpClient.execute upload logic out of the attachment collection
loop so all files are added first, then perform a single upload for the
issueKey. Keep the fix localized around the attachment-building flow in
PostTicketCommand and ensure the multipart entity is built only once after all
attachments are gathered.
| try (CloseableHttpClient httpClient = HttpClients.createDefault()) { | ||
| httpClient.execute(request, response -> { |
There was a problem hiding this comment.
🩺 Stability & Availability | 🟠 Major
🧩 Analysis chain
🏁 Script executed:
cat -n src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommand.javaRepository: reportportal/plugin-bts-jira
Length of output: 13474
🏁 Script executed:
find src -name "*.java" -type f -exec grep -l "CloseableHttpClient\|HttpClients.create" {} \;Repository: reportportal/plugin-bts-jira
Length of output: 260
🏁 Script executed:
grep -r "RequestConfig\|SocketTimeout\|ConnectTimeout" src/main/java --include="*.java" | head -20Repository: reportportal/plugin-bts-jira
Length of output: 166
🏁 Script executed:
python3 - <<'PY'
import subprocess
import re
# Search for HTTP client configuration patterns
result = subprocess.run(
['rg', 'RequestConfig|setTimeout|timeout', '--type=java', '-A', '3', '-B', '2', 'src/'],
capture_output=True,
text=True
)
print("HTTP Client Configuration Patterns:")
print(result.stdout[:3000] if result.stdout else "No matches found")
if result.stderr:
print("\nErrors:")
print(result.stderr[:1000])
PYRepository: reportportal/plugin-bts-jira
Length of output: 219
Configure timeouts for the manual attachment HTTP client.
The HttpClients.createDefault() call on line 215 creates an HttpClient with default configuration that lacks explicit request, connection, and socket timeouts. This can cause the command thread to block indefinitely if the Jira server is slow or unresponsive during attachment upload.
Configure timeouts using RequestConfig:
RequestConfig requestConfig = RequestConfig.custom()
.setConnectTimeout(Duration.ofSeconds(10))
.setResponseTimeout(Duration.ofSeconds(30))
.build();
CloseableHttpClient httpClient = HttpClients.custom()
.setDefaultRequestConfig(requestConfig)
.build();🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/main/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommand.java`
around lines 215 - 216, The manual attachment upload in PostTicketCommand uses a
default HttpClient without explicit timeout settings, so update the
HttpClients.createDefault() usage to a configured client built from
HttpClients.custom() with a RequestConfig applied. In the attachment upload path
inside PostTicketCommand, set connection and response/request timeouts on the
client before httpClient.execute(...) so slow Jira responses do not block the
command indefinitely.
| Ticket ticket = command.invokeCommand(INTEGRATION, rq); | ||
| log.info(ticket.getTicketUrl()); | ||
|
|
||
| assertNotNull(ticket); |
There was a problem hiding this comment.
🎯 Functional Correctness | 🟡 Minor | ⚡ Quick win
Assert before dereferencing and restore ticket-field checks.
ticket.getTicketUrl() runs before the null assertion, and the test no longer verifies the returned ticket URL/id/status, so mapping regressions can pass unnoticed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommandTest.java`
around lines 92 - 94, In PostTicketCommandTest, the test is dereferencing the
returned Ticket before asserting it is non-null and it no longer validates the
mapped fields. Update the assertion order so the null check happens before using
ticket.getTicketUrl(), and restore the field-level assertions in the test for
the Ticket returned by command.invokeCommand(INTEGRATION, rq), including ticket
URL, ticket id, and status.
| @Test | ||
| @DisabledIf("disabled") | ||
| void addAttachmentTest() { | ||
| var validJiraTicket = "EPMRPP-100426"; | ||
| Map<String, String> map = new HashMap<>(); | ||
| map.put("file1", "file1.txt"); | ||
| //map.put("file2", "file2.txt"); | ||
| lenient().when(dataStoreService.load(anyString())) | ||
| .thenReturn( | ||
| Optional.ofNullable(getClass().getClassLoader().getResourceAsStream("attachment.txt")), | ||
| Optional.ofNullable(getClass().getClassLoader().getResourceAsStream("attachment2.txt")) | ||
| ); | ||
| jiraStrategy.addAttachment(validJiraTicket, INTEGRATION, map); | ||
| // attachment testing is done via the full postTicket flow |
There was a problem hiding this comment.
📐 Maintainability & Code Quality | 🟡 Minor | ⚡ Quick win
Do not keep an empty passing attachment test.
This test body always passes while the production attachment path contains credential, upload-loop, and stream-handling logic that should be covered or the placeholder should be removed.
🤖 Prompt for AI Agents
Verify each finding against current code. Fix only still-valid issues, skip the
rest with a brief reason, keep changes minimal, and validate.
In
`@src/test/java/com/epam/reportportal/extension/bugtracking/jira/command/PostTicketCommandTest.java`
around lines 97 - 100, The addAttachmentTest in PostTicketCommandTest is only a
placeholder and always passes, so either remove this empty test or replace it
with real coverage for the attachment flow through the relevant
postTicket/addAttachment path. If you keep it, make sure it asserts the
credential handling, upload loop, and stream handling behavior in the production
attachment logic instead of just containing a comment.
2 participants
Summary by CodeRabbit