From 12de740173914632a03af56d0c3625760238f20a Mon Sep 17 00:00:00 2001
From: Maxwell Brown <55033421+Galactus22625@users.noreply.github.com>
Date: Mon, 11 Nov 2024 20:47:41 -0800
Subject: [PATCH] Jira source fixes (#5175)
* switch logs to trace
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
* remove Jira Project Cache (null pointer bug)
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
* fix accountUrl with no ending / issue
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
* better slash changes handling
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
* better logic
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
* fix tests
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
* Jira Iterator HasNext test
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
---------
Signed-off-by: Maxwell Brown <mxwelwbr@amazon.com>
---
.../plugins/source/jira/JiraIterator.java | 9 ++++--
.../plugins/source/jira/JiraService.java | 29 ++-----------------
.../source/jira/rest/JiraRestClient.java | 6 +---
.../jira/rest/auth/JiraBasicAuthConfig.java | 7 ++++-
.../plugins/source/jira/JiraIteratorTest.java | 15 +++++++++-
.../plugins/source/jira/JiraServiceTest.java | 2 +-
.../source/jira/rest/JiraRestClientTest.java | 1 +
.../jira/rest/auth/JiraAuthFactoryTest.java | 1 +
.../rest/auth/JiraBasicAuthConfigTest.java | 6 +++-
9 files changed, 39 insertions(+), 37 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..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;
@@ -38,7 +39,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;
}
@@ -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 1eaf67e1df..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<String, String> jiraProjectCache = new ConcurrentHashMap<>();
private final JiraSourceConfig jiraSourceConfig;
private final JiraRestClient jiraRestClient;
@@ -75,17 +66,10 @@ public JiraService(JiraSourceConfig jiraSourceConfig, JiraRestClient jiraRestCli
* @param timestamp timestamp.
*/
public Queue<ItemInfo> getJiraEntities(JiraSourceConfig configuration, Instant timestamp) {
- log.info("Started to fetch entities");
+ log.trace("Started to fetch entities");
Queue<ItemInfo> itemInfoQueue = new ConcurrentLinkedQueue<>();
- jiraProjectCache.clear();
searchForNewTicketsAndAddToQueue(configuration, timestamp, itemInfoQueue);
log.trace("Creating item information and adding in queue");
- jiraProjectCache.keySet().forEach(key -> {
- Map<String, Object> 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<IssueBean> issueList, Queue<ItemInfo> 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);
- }
- }
});
}
@@ -165,7 +142,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 +152,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<String> badFilters = new ArrayList<>();
Pattern regex = Pattern.compile("[^A-Z0-9]");
JiraConfigHelper.getProjectKeyFilter(configuration).forEach(projectFilter -> {
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..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,10 +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)) {
- url = authConfig.getUrl() + REST_API_SEARCH;
- }
+ String url = authConfig.getUrl() + REST_API_SEARCH;
URI uri = UriComponentsBuilder.fromHttpUrl(url)
.queryParam(MAX_RESULT, FIFTY)
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..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,14 +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() {
- return jiraSourceConfig.getAccountUrl();
+ return accountUrl;
}
@Override
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..9c47bf8e51 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,20 @@ void testItemInfoQueueNotEmpty() {
jiraIterator.setCrawlerQWaitTimeMillis(1);
assertTrue(jiraIterator.hasNext());
assertNotNull(jiraIterator.next());
- assertNotNull(jiraIterator.next());
+ }
+
+ @Test
+ void testItemInfoQueueEmpty(){
+ jiraIterator = createObjectUnderTest();
+ List<IssueBean> mockIssues = new ArrayList<>();
+ when(mockSearchResults.getIssues()).thenReturn(mockIssues);
+ when(mockSearchResults.getTotal()).thenReturn(0);
+ doReturn(mockSearchResults).when(jiraRestClient).getAllIssues(any(StringBuilder.class), anyInt(), any(JiraSourceConfig.class));
+
+ jiraIterator.initialize(Instant.ofEpochSecond(0));
+ jiraIterator.setCrawlerQWaitTimeMillis(1);
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<ItemInfo> itemInfoQueue = jiraService.getJiraEntities(jiraSourceConfig, timestamp);
- assertEquals(mockIssues.size() + 1, itemInfoQueue.size());
+ assertEquals(mockIssues.size(), itemInfoQueue.size());
}
@Test
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..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
@@ -123,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/JiraAuthFactoryTest.java b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraAuthFactoryTest.java
index d006f8df3f..566b3ae8ed 100644
--- a/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraAuthFactoryTest.java
+++ b/data-prepper-plugins/saas-source-plugins/jira-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraAuthFactoryTest.java
@@ -33,6 +33,7 @@ void testGetObjectOauth2() {
@Test
void testGetObjectBasicAuth() {
+ when(sourceConfig.getAccountUrl()).thenReturn("https://example.com");
assertInstanceOf(JiraBasicAuthConfig.class, jiraAuthFactory.getObject());
}
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..60dad25507 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 {
@@ -16,15 +17,18 @@ public class JiraBasicAuthConfigTest {
private JiraSourceConfig jiraSourceConfig;
private JiraBasicAuthConfig jiraBasicAuthConfig;
+ String url = "https://example.com";
@BeforeEach
void setUp() {
+ when(jiraSourceConfig.getAccountUrl()).thenReturn(url);
jiraBasicAuthConfig = new JiraBasicAuthConfig(jiraSourceConfig);
}
@Test
void testGetUrl() {
- assertEquals(jiraBasicAuthConfig.getUrl(), jiraSourceConfig.getAccountUrl());
+ assertEquals(jiraBasicAuthConfig.getUrl(), url + '/');
+
}
@Test