Jira source fixes (opensearch-project#5175)

* switch logs to trace Signed-off-by: Maxwell Brown <[email protected]> * remove Jira Project Cache (null pointer bug) Signed-off-by: Maxwell Brown <[email protected]> * fix accountUrl with no ending / issue Signed-off-by: Maxwell Brown <[email protected]> * better slash changes handling Signed-off-by: Maxwell Brown <[email protected]> * better logic Signed-off-by: Maxwell Brown <[email protected]> * fix tests Signed-off-by: Maxwell Brown <[email protected]> * Jira Iterator HasNext test Signed-off-by: Maxwell Brown <[email protected]> --------- Signed-off-by: Maxwell Brown <[email protected]>
shenkw1 · Nov 12, 2024 · 12de740 · 12de740
1 parent 05aaf65
commit 12de740
Show file tree

Hide file tree

Showing 9 changed files with 39 additions and 37 deletions.
diff --git a/...ira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraIterator.java b/...ira-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/...jira-source/src/main/java/org/opensearch/dataprepper/plugins/source/jira/JiraService.java b/...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/...rce/src/main/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClient.java b/...rce/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/...in/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfig.java b/...in/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/...source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraIteratorTest.java b/...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/...-source/src/test/java/org/opensearch/dataprepper/plugins/source/jira/JiraServiceTest.java b/...-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/...src/test/java/org/opensearch/dataprepper/plugins/source/jira/rest/JiraRestClientTest.java b/...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/...st/java/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraAuthFactoryTest.java b/...st/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/...ava/org/opensearch/dataprepper/plugins/source/jira/rest/auth/JiraBasicAuthConfigTest.java b/...ava/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
-Original file line number
+Diff line change
@@ Expand Up / @@ -33,6 +33,7 @@ void testGetObjectOauth2() { @@
         @Test
         void testGetObjectBasicAuth() {
+            when(sourceConfig.getAccountUrl()).thenReturn("https://example.com");
             assertInstanceOf(JiraBasicAuthConfig.class, jiraAuthFactory.getObject());
         }
@@ Expand Down @@