From 115e38e018c46771dc062e099a8f9e5297388aaf Mon Sep 17 00:00:00 2001 From: Maxwell Brown Date: Wed, 6 Nov 2024 16:08:46 -0800 Subject: [PATCH 1/5] switch logs to trace Signed-off-by: Maxwell Brown --- .../dataprepper/plugins/source/jira/JiraIterator.java | 2 +- .../dataprepper/plugins/source/jira/JiraService.java | 6 +++--- 2 files changed, 4 insertions(+), 4 deletions(-) diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraIterator.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraIterator.java index 1e033581b4..d05fdc97b0 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraIterator.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraIterator.java @@ -38,7 +38,7 @@ public JiraIterator(final JiraService service, @Override public boolean hasNext() { if (firstTime) { - log.info("Crawling has been started"); + log.trace("Crawling has been started"); itemInfoQueue = service.getJiraEntities(sourceConfig, lastPollTime); firstTime = false; } diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraService.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraService.java index 1eaf67e1df..c2de2e01c1 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraService.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraService.java @@ -75,7 +75,7 @@ public JiraService(JiraSourceConfig jiraSourceConfig, JiraRestClient jiraRestCli * @param timestamp timestamp. */ public Queue getJiraEntities(JiraSourceConfig configuration, Instant timestamp) { - log.info("Started to fetch entities"); + log.trace("Started to fetch entities"); Queue itemInfoQueue = new ConcurrentLinkedQueue<>(); jiraProjectCache.clear(); searchForNewTicketsAndAddToQueue(configuration, timestamp, itemInfoQueue); @@ -165,7 +165,7 @@ private StringBuilder createIssueFilterCriteria(JiraSourceConfig configuration, .collect(Collectors.joining(DELIMITER, PREFIX, SUFFIX))) .append(CLOSING_ROUND_BRACKET); } - log.info("Created issue filter criteria JiraQl query: {}", jiraQl); + log.trace("Created issue filter criteria JiraQl query: {}", jiraQl); return jiraQl; } @@ -175,7 +175,7 @@ private StringBuilder createIssueFilterCriteria(JiraSourceConfig configuration, * @param configuration Input Parameter */ private void validateProjectFilters(JiraSourceConfig configuration) { - log.info("Validating project filters"); + log.trace("Validating project filters"); List badFilters = new ArrayList<>(); Pattern regex = Pattern.compile("[^A-Z0-9]"); JiraConfigHelper.getProjectKeyFilter(configuration).forEach(projectFilter -> { From 013e9c8f1165194bf917a02c7e070a63c8c011df Mon Sep 17 00:00:00 2001 From: Maxwell Brown Date: Wed, 6 Nov 2024 16:42:07 -0800 Subject: [PATCH 2/5] remove Jira Project Cache (null pointer bug) Signed-off-by: Maxwell Brown --- .../plugins/source/jira/JiraIterator.java | 7 +++++- .../plugins/source/jira/JiraService.java | 23 ------------------- .../plugins/source/jira/JiraIteratorTest.java | 3 ++- .../plugins/source/jira/JiraServiceTest.java | 2 +- 4 files changed, 9 insertions(+), 26 deletions(-) diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraIterator.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraIterator.java index d05fdc97b0..cd5d422047 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraIterator.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraIterator.java @@ -10,6 +10,7 @@ import javax.inject.Named; import java.time.Instant; import java.util.Iterator; +import java.util.NoSuchElementException; import java.util.Queue; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.concurrent.ExecutorService; @@ -47,7 +48,11 @@ public boolean hasNext() { @Override public ItemInfo next() { - return this.itemInfoQueue.remove(); + if (hasNext()) { + return this.itemInfoQueue.remove(); + } else { + throw new NoSuchElementException(); + } } /** diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraService.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraService.java index c2de2e01c1..f2fead1b54 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraService.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraService.java @@ -8,31 +8,23 @@ import org.opensearch.dataprepper.plugins.source.jira.models.SearchResults; import org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient; import org.opensearch.dataprepper.plugins.source.jira.utils.JiraConfigHelper; -import org.opensearch.dataprepper.plugins.source.jira.utils.JiraContentType; import org.opensearch.dataprepper.plugins.source.source_crawler.model.ItemInfo; import org.springframework.util.CollectionUtils; import javax.inject.Named; import java.time.Instant; import java.util.ArrayList; -import java.util.HashMap; import java.util.List; import java.util.Map; -import java.util.Objects; import java.util.Queue; -import java.util.concurrent.ConcurrentHashMap; import java.util.concurrent.ConcurrentLinkedQueue; import java.util.regex.Matcher; import java.util.regex.Pattern; import java.util.stream.Collectors; import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.ISSUE_KEY; -import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.KEY; -import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.LIVE; -import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.PROJECT; import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.PROJECT_KEY; import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.UPDATED; -import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants._PROJECT; import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.CLOSING_ROUND_BRACKET; import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.DELIMITER; import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.GREATER_THAN_EQUALS; @@ -54,7 +46,6 @@ public class JiraService { public static final String CONTENT_TYPE = "ContentType"; private static final String SEARCH_RESULTS_FOUND = "searchResultsFound"; - private static final Map jiraProjectCache = new ConcurrentHashMap<>(); private final JiraSourceConfig jiraSourceConfig; private final JiraRestClient jiraRestClient; @@ -77,15 +68,8 @@ public JiraService(JiraSourceConfig jiraSourceConfig, JiraRestClient jiraRestCli public Queue getJiraEntities(JiraSourceConfig configuration, Instant timestamp) { log.trace("Started to fetch entities"); Queue itemInfoQueue = new ConcurrentLinkedQueue<>(); - jiraProjectCache.clear(); searchForNewTicketsAndAddToQueue(configuration, timestamp, itemInfoQueue); log.trace("Creating item information and adding in queue"); - jiraProjectCache.keySet().forEach(key -> { - Map metadata = new HashMap<>(); - metadata.put(CONTENT_TYPE, JiraContentType.PROJECT.getType()); - ItemInfo itemInfo = createItemInfo(_PROJECT + key, metadata); - itemInfoQueue.add(itemInfo); - }); return itemInfoQueue; } @@ -125,13 +109,6 @@ private void searchForNewTicketsAndAddToQueue(JiraSourceConfig configuration, In private void addItemsToQueue(List issueList, Queue itemInfoQueue) { issueList.forEach(issue -> { itemInfoQueue.add(JiraItemInfo.builder().withEventTime(Instant.now()).withIssueBean(issue).build()); - - if (Objects.nonNull(((Map) issue.getFields().get(PROJECT)).get(KEY))) { - String projectKey = ((Map) issue.getFields().get(PROJECT)).get(KEY).toString(); - if (!jiraProjectCache.containsKey(projectKey)) { - jiraProjectCache.put(projectKey, LIVE); - } - } }); } diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraIteratorTest.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraIteratorTest.java index b8560d69a4..b63a6fd17a 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraIteratorTest.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraIteratorTest.java @@ -15,6 +15,7 @@ import java.util.HashMap; import java.util.List; import java.util.Map; +import java.util.NoSuchElementException; import java.util.UUID; import static org.junit.jupiter.api.Assertions.assertFalse; @@ -98,8 +99,8 @@ void testItemInfoQueueNotEmpty() { jiraIterator.setCrawlerQWaitTimeMillis(1); assertTrue(jiraIterator.hasNext()); assertNotNull(jiraIterator.next()); - assertNotNull(jiraIterator.next()); assertFalse(jiraIterator.hasNext()); + assertThrows(NoSuchElementException.class, () -> jiraIterator.next()); } diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraServiceTest.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraServiceTest.java index cfa8a32733..8900936400 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraServiceTest.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraServiceTest.java @@ -142,7 +142,7 @@ public void testGetJiraEntities() throws JsonProcessingException { Instant timestamp = Instant.ofEpochSecond(0); Queue itemInfoQueue = jiraService.getJiraEntities(jiraSourceConfig, timestamp); - assertEquals(mockIssues.size() + 1, itemInfoQueue.size()); + assertEquals(mockIssues.size(), itemInfoQueue.size()); } @Test From 44094c5f8c4173f0d5bc4c47d502b18ba29216f7 Mon Sep 17 00:00:00 2001 From: Maxwell Brown Date: Wed, 6 Nov 2024 17:16:14 -0800 Subject: [PATCH 3/5] fix accountUrl with no ending / issue Signed-off-by: Maxwell Brown --- .../source/jira/rest/JiraRestClient.java | 15 ++++++- .../source/jira/rest/JiraRestClientTest.java | 44 +++++++++++++++++++ 2 files changed, 57 insertions(+), 2 deletions(-) diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClient.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClient.java index d87985c6fd..81322d9ebf 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClient.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClient.java @@ -71,7 +71,12 @@ public SearchResults getAllIssues(StringBuilder jql, int startAt, String url = configuration.getAccountUrl() + REST_API_SEARCH; if (configuration.getAuthType().equals(OAUTH2)) { - url = authConfig.getUrl() + REST_API_SEARCH; + String accountUrl = authConfig.getUrl(); + if (accountUrl.endsWith("/")) { + url = accountUrl + REST_API_SEARCH; + } else { + url = accountUrl + "/" + REST_API_SEARCH; + } } URI uri = UriComponentsBuilder.fromHttpUrl(url) @@ -92,7 +97,13 @@ public SearchResults getAllIssues(StringBuilder jql, int startAt, @Timed(TICKET_FETCH_LATENCY_TIMER) public String getIssue(String issueKey) { issuesRequestedCounter.increment(); - String url = authConfig.getUrl() + REST_API_FETCH_ISSUE + "/" + issueKey; + String accountUrl = authConfig.getUrl(); + String url; + if (accountUrl.endsWith("/")) { + url = accountUrl + REST_API_FETCH_ISSUE + "/" + issueKey; + } else { + url = accountUrl + "/" + REST_API_FETCH_ISSUE + "/" + issueKey; + } URI uri = UriComponentsBuilder.fromHttpUrl(url).buildAndExpand().toUri(); return invokeRestApi(uri, String.class).getBody(); } diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClientTest.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClientTest.java index c9bdb39625..066b3cd297 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClientTest.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClientTest.java @@ -19,6 +19,7 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; +import org.springframework.web.util.UriComponentsBuilder; import java.net.URI; import java.util.ArrayList; @@ -28,12 +29,22 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; +import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; +import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; +import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.FIFTY; +import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.MAX_RESULT; +import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.REST_API_FETCH_ISSUE; +import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.REST_API_SEARCH; +import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.START_AT; import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.BASIC; import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.OAUTH2; +import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.EXPAND_FIELD; +import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.EXPAND_VALUE; +import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.JQL_FIELD; @ExtendWith(MockitoExtension.class) public class JiraRestClientTest { @@ -99,6 +110,39 @@ void testInvokeRestApiTokenExpiredInterruptException() throws InterruptedExcepti testThread.interrupt(); } + @Test + void testNoSlashGetUrlGetIssue(){ + JiraRestClient jiraRestClient = new JiraRestClient(restTemplate, authConfig); + String issueKey = "key"; + when(authConfig.getUrl()).thenReturn("https://example.com/rest/api/2/issue/key"); + String url ="https://example.com/rest/api/2/issue/key" + "/" + REST_API_FETCH_ISSUE + "/" + issueKey; + URI uri = UriComponentsBuilder.fromHttpUrl(url).buildAndExpand().toUri(); + doReturn(new ResponseEntity<>("URI is correct", HttpStatus.OK)).when(restTemplate).getForEntity(eq(uri), any(Class.class)); + assertTrue(jiraRestClient.getIssue(issueKey).contains("URI is correct")); + } + + @Test + void testNoSlashGetUrlGetAllIssues() throws JsonProcessingException { + List issueType = new ArrayList<>(); + List issueStatus = new ArrayList<>(); + List projectKey = new ArrayList<>(); + issueType.add("Task"); + JiraSourceConfig jiraSourceConfig = JiraServiceTest.createJiraConfiguration(OAUTH2, issueType, issueStatus, projectKey); + JiraRestClient jiraRestClient = new JiraRestClient(restTemplate, authConfig); + SearchResults mockSearchResults = mock(SearchResults.class); + when(authConfig.getUrl()).thenReturn("https://example.com/rest/api/2/issue/key"); + String url ="https://example.com/rest/api/2/issue/key" + "/" + REST_API_SEARCH; + int startAt = 0; + URI uri = UriComponentsBuilder.fromHttpUrl(url) + .queryParam(MAX_RESULT, FIFTY) + .queryParam(START_AT, startAt) + .queryParam(JQL_FIELD, jql) + .queryParam(EXPAND_FIELD, EXPAND_VALUE) + .buildAndExpand().toUri(); + doReturn(new ResponseEntity<>(mockSearchResults, HttpStatus.OK)).when(restTemplate).getForEntity(eq(uri), any(Class.class)); + assertEquals(mockSearchResults, jiraRestClient.getAllIssues(jql, 0, jiraSourceConfig)); + } + @Test public void testGetAllIssuesOauth2() throws JsonProcessingException { List issueType = new ArrayList<>(); From ab27126603f777baaaa48ea0289e38f49591565f Mon Sep 17 00:00:00 2001 From: Maxwell Brown Date: Wed, 6 Nov 2024 17:48:17 -0800 Subject: [PATCH 4/5] better slash changes handling Signed-off-by: Maxwell Brown --- .../source/jira/rest/JiraRestClient.java | 19 +------- .../jira/rest/auth/JiraBasicAuthConfig.java | 10 ++++- .../source/jira/rest/JiraRestClientTest.java | 45 +------------------ .../rest/auth/JiraBasicAuthConfigTest.java | 10 ++++- 4 files changed, 21 insertions(+), 63 deletions(-) diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClient.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClient.java index 81322d9ebf..c87c7019e6 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClient.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClient.java @@ -21,7 +21,6 @@ import java.util.List; import static org.opensearch.dataprepper.logging.DataPrepperMarkers.NOISY; -import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.OAUTH2; import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.RETRY_ATTEMPT; import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.EXPAND_FIELD; import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.EXPAND_VALUE; @@ -69,15 +68,7 @@ public JiraRestClient(RestTemplate restTemplate, JiraAuthConfig authConfig) { public SearchResults getAllIssues(StringBuilder jql, int startAt, JiraSourceConfig configuration) { - String url = configuration.getAccountUrl() + REST_API_SEARCH; - if (configuration.getAuthType().equals(OAUTH2)) { - String accountUrl = authConfig.getUrl(); - if (accountUrl.endsWith("/")) { - url = accountUrl + REST_API_SEARCH; - } else { - url = accountUrl + "/" + REST_API_SEARCH; - } - } + String url = authConfig.getUrl() + REST_API_SEARCH; URI uri = UriComponentsBuilder.fromHttpUrl(url) .queryParam(MAX_RESULT, FIFTY) @@ -97,13 +88,7 @@ public SearchResults getAllIssues(StringBuilder jql, int startAt, @Timed(TICKET_FETCH_LATENCY_TIMER) public String getIssue(String issueKey) { issuesRequestedCounter.increment(); - String accountUrl = authConfig.getUrl(); - String url; - if (accountUrl.endsWith("/")) { - url = accountUrl + REST_API_FETCH_ISSUE + "/" + issueKey; - } else { - url = accountUrl + "/" + REST_API_FETCH_ISSUE + "/" + issueKey; - } + String url = authConfig.getUrl() + REST_API_FETCH_ISSUE + "/" + issueKey; URI uri = UriComponentsBuilder.fromHttpUrl(url).buildAndExpand().toUri(); return invokeRestApi(uri, String.class).getBody(); } diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfig.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfig.java index 751f56f6d1..3e7f1b0976 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfig.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfig.java @@ -13,7 +13,15 @@ public JiraBasicAuthConfig(JiraSourceConfig jiraSourceConfig) { @Override public String getUrl() { - return jiraSourceConfig.getAccountUrl(); + String accountUrl = jiraSourceConfig.getAccountUrl(); + + String url; + if (accountUrl.endsWith("/")) { + url = accountUrl; + } else { + url = accountUrl + "/"; + } + return url; } @Override diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClientTest.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClientTest.java index 066b3cd297..d79372c65b 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClientTest.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClientTest.java @@ -19,7 +19,6 @@ import org.springframework.http.ResponseEntity; import org.springframework.web.client.HttpClientErrorException; import org.springframework.web.client.RestTemplate; -import org.springframework.web.util.UriComponentsBuilder; import java.net.URI; import java.util.ArrayList; @@ -29,22 +28,12 @@ import static org.junit.jupiter.api.Assertions.assertEquals; import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertThrows; -import static org.junit.jupiter.api.Assertions.assertTrue; import static org.mockito.ArgumentMatchers.any; -import static org.mockito.ArgumentMatchers.eq; import static org.mockito.Mockito.doReturn; import static org.mockito.Mockito.mock; import static org.mockito.Mockito.when; -import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.FIFTY; -import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.MAX_RESULT; -import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.REST_API_FETCH_ISSUE; -import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.REST_API_SEARCH; -import static org.opensearch.dataprepper.plugins.source.jira.rest.JiraRestClient.START_AT; import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.BASIC; import static org.opensearch.dataprepper.plugins.source.jira.utils.Constants.OAUTH2; -import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.EXPAND_FIELD; -import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.EXPAND_VALUE; -import static org.opensearch.dataprepper.plugins.source.jira.utils.JqlConstants.JQL_FIELD; @ExtendWith(MockitoExtension.class) public class JiraRestClientTest { @@ -110,39 +99,6 @@ void testInvokeRestApiTokenExpiredInterruptException() throws InterruptedExcepti testThread.interrupt(); } - @Test - void testNoSlashGetUrlGetIssue(){ - JiraRestClient jiraRestClient = new JiraRestClient(restTemplate, authConfig); - String issueKey = "key"; - when(authConfig.getUrl()).thenReturn("https://example.com/rest/api/2/issue/key"); - String url ="https://example.com/rest/api/2/issue/key" + "/" + REST_API_FETCH_ISSUE + "/" + issueKey; - URI uri = UriComponentsBuilder.fromHttpUrl(url).buildAndExpand().toUri(); - doReturn(new ResponseEntity<>("URI is correct", HttpStatus.OK)).when(restTemplate).getForEntity(eq(uri), any(Class.class)); - assertTrue(jiraRestClient.getIssue(issueKey).contains("URI is correct")); - } - - @Test - void testNoSlashGetUrlGetAllIssues() throws JsonProcessingException { - List issueType = new ArrayList<>(); - List issueStatus = new ArrayList<>(); - List projectKey = new ArrayList<>(); - issueType.add("Task"); - JiraSourceConfig jiraSourceConfig = JiraServiceTest.createJiraConfiguration(OAUTH2, issueType, issueStatus, projectKey); - JiraRestClient jiraRestClient = new JiraRestClient(restTemplate, authConfig); - SearchResults mockSearchResults = mock(SearchResults.class); - when(authConfig.getUrl()).thenReturn("https://example.com/rest/api/2/issue/key"); - String url ="https://example.com/rest/api/2/issue/key" + "/" + REST_API_SEARCH; - int startAt = 0; - URI uri = UriComponentsBuilder.fromHttpUrl(url) - .queryParam(MAX_RESULT, FIFTY) - .queryParam(START_AT, startAt) - .queryParam(JQL_FIELD, jql) - .queryParam(EXPAND_FIELD, EXPAND_VALUE) - .buildAndExpand().toUri(); - doReturn(new ResponseEntity<>(mockSearchResults, HttpStatus.OK)).when(restTemplate).getForEntity(eq(uri), any(Class.class)); - assertEquals(mockSearchResults, jiraRestClient.getAllIssues(jql, 0, jiraSourceConfig)); - } - @Test public void testGetAllIssuesOauth2() throws JsonProcessingException { List issueType = new ArrayList<>(); @@ -167,6 +123,7 @@ public void testGetAllIssuesBasic() throws JsonProcessingException { JiraSourceConfig jiraSourceConfig = JiraServiceTest.createJiraConfiguration(BASIC, issueType, issueStatus, projectKey); JiraRestClient jiraRestClient = new JiraRestClient(restTemplate, authConfig); SearchResults mockSearchResults = mock(SearchResults.class); + when(authConfig.getUrl()).thenReturn("https://example.com"); doReturn(new ResponseEntity<>(mockSearchResults, HttpStatus.OK)).when(restTemplate).getForEntity(any(URI.class), any(Class.class)); SearchResults results = jiraRestClient.getAllIssues(jql, 0, jiraSourceConfig); assertNotNull(results); diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfigTest.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfigTest.java index 0028525127..917864592f 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfigTest.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfigTest.java @@ -8,6 +8,7 @@ import org.opensearch.dataprepper.plugins.source.jira.JiraSourceConfig; import static org.junit.jupiter.api.Assertions.assertEquals; +import static org.mockito.Mockito.when; @ExtendWith(MockitoExtension.class) public class JiraBasicAuthConfigTest { @@ -24,7 +25,14 @@ void setUp() { @Test void testGetUrl() { - assertEquals(jiraBasicAuthConfig.getUrl(), jiraSourceConfig.getAccountUrl()); + String url = "https://example.com"; + when(jiraSourceConfig.getAccountUrl()).thenReturn(url); + assertEquals(jiraBasicAuthConfig.getUrl(), url + '/'); + + String url2 = "https://example.com/"; + when(jiraSourceConfig.getAccountUrl()).thenReturn(url2); + assertEquals(jiraBasicAuthConfig.getUrl(), url2); + } @Test From c3c33289640a2d0bdebc1ad2187c9d98b65aea59 Mon Sep 17 00:00:00 2001 From: Maxwell Brown Date: Wed, 6 Nov 2024 18:01:09 -0800 Subject: [PATCH 5/5] better logic Signed-off-by: Maxwell Brown --- .../jira/rest/auth/JiraBasicAuthConfig.java | 15 ++++++--------- 1 file changed, 6 insertions(+), 9 deletions(-) diff --git a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfig.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfig.java index 3e7f1b0976..233cbf9f49 100644 --- a/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfig.java +++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfig.java @@ -6,22 +6,19 @@ public class JiraBasicAuthConfig implements JiraAuthConfig { private final JiraSourceConfig jiraSourceConfig; + private String accountUrl; public JiraBasicAuthConfig(JiraSourceConfig jiraSourceConfig) { this.jiraSourceConfig = jiraSourceConfig; + accountUrl = jiraSourceConfig.getAccountUrl(); + if (!accountUrl.endsWith("/")) { + accountUrl += "/"; + } } @Override public String getUrl() { - String accountUrl = jiraSourceConfig.getAccountUrl(); - - String url; - if (accountUrl.endsWith("/")) { - url = accountUrl; - } else { - url = accountUrl + "/"; - } - return url; + return accountUrl; } @Override