Skip to content

Commit 35c8276

Browse files
san81EC2 Default User
san81
authored and
EC2 Default User
committed
Removing flaky TimzoneHelper utility and corresponding test case (opensearch-project#5591)
* Removing TimezoneHelper class and its corresponding test cases that are flaky. Changed the logic to make use of the ZonedDateTime conversion. Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com> * javadoc fixes Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com> * Making it on demand initialization Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com> * Test case to validate CQL construction with ZonedDateTime conversion Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com> --------- Signed-off-by: Santhosh Gandhe <1909520+san81@users.noreply.github.com>
1 parent d5b5496 commit 35c8276

22 files changed

Lines changed: 108 additions & 260 deletions

File tree

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/main/java/org/opensearch/dataprepper/plugins/source/confluence/ConfluenceClient.java‎

Lines changed: 1 addition & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -53,7 +53,6 @@ public class ConfluenceClient implements CrawlerClient {
5353

5454
private static final Logger log = LoggerFactory.getLogger(ConfluenceClient.class);
5555
private ObjectMapper objectMapper = new ObjectMapper();
56-
private Instant lastPollTime;
5756
private final ConfluenceService service;
5857
private final ConfluenceIterator confluenceIterator;
5958
private final ExecutorService executorService;
@@ -71,17 +70,11 @@ public ConfluenceClient(ConfluenceService service,
7170
}
7271

7372
@Override
74-
public Iterator<ItemInfo> listItems() {
73+
public Iterator<ItemInfo> listItems(Instant lastPollTime) {
7574
confluenceIterator.initialize(lastPollTime);
7675
return confluenceIterator;
7776
}
7877

79-
@Override
80-
public void setLastPollTime(Instant lastPollTime) {
81-
log.trace("Setting the lastPollTime: {}", lastPollTime);
82-
this.lastPollTime = lastPollTime;
83-
}
84-
8578
@VisibleForTesting
8679
public void injectObjectMapper(ObjectMapper objectMapper) {
8780
this.objectMapper = objectMapper;

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/main/java/org/opensearch/dataprepper/plugins/source/confluence/ConfluenceService.java‎

Lines changed: 14 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -26,7 +26,6 @@
2626

2727
import javax.inject.Named;
2828
import java.time.Instant;
29-
import java.time.LocalDateTime;
3029
import java.time.ZoneId;
3130
import java.time.format.DateTimeFormatter;
3231
import java.util.ArrayList;
@@ -60,8 +59,9 @@ public class ConfluenceService {
6059

6160

6261
public static final String CONTENT_TYPE = "ContentType";
62+
public static final String CQL_LAST_MODIFIED_DATE_FORMAT = "yyyy-MM-dd HH:mm";
6363
private static final String SEARCH_RESULTS_FOUND = "searchResultsFound";
64-
64+
private ZoneId confluenceServerZoneId = null;
6565
private final ConfluenceSourceConfig confluenceSourceConfig;
6666
private final ConfluenceRestClient confluenceRestClient;
6767
private final Counter searchResultsFoundCounter;
@@ -80,6 +80,7 @@ public ConfluenceService(ConfluenceSourceConfig confluenceSourceConfig,
8080
*
8181
* @param configuration the configuration.
8282
* @param timestamp timestamp.
83+
* @param itemInfoQueue queue for storing item information.
8384
*/
8485
public void getPages(ConfluenceSourceConfig configuration, Instant timestamp, Queue<ItemInfo> itemInfoQueue) {
8586
log.trace("Started to fetch entities");
@@ -91,8 +92,9 @@ public String getContent(String contentId) {
9192
return confluenceRestClient.getContent(contentId);
9293
}
9394

94-
public ConfluenceServerMetadata getConfluenceServerMetadata() {
95-
return confluenceRestClient.getConfluenceServerMetadata();
95+
private void initializeConfluenceServerMetadata() {
96+
ConfluenceServerMetadata confluenceServerMetadata = confluenceRestClient.getConfluenceServerMetadata();
97+
this.confluenceServerZoneId = confluenceServerMetadata.getDefaultTimeZone();
9698
}
9799

98100
/**
@@ -135,12 +137,13 @@ private void addItemsToQueue(List<ConfluenceItem> contentList, Queue<ItemInfo> i
135137

136138
/**
137139
* Method for creating Content Filter Criteria.
140+
* Made this method package private to be able to test with UnitTests
138141
*
139142
* @param configuration Input Parameter
140143
* @param ts Input Parameter
141144
* @return String Builder
142145
*/
143-
private StringBuilder createContentFilterCriteria(ConfluenceSourceConfig configuration, Instant ts) {
146+
StringBuilder createContentFilterCriteria(ConfluenceSourceConfig configuration, Instant ts) {
144147

145148
log.info("Creating content filter criteria");
146149
if (!CollectionUtils.isEmpty(ConfluenceConfigHelper.getSpacesNameIncludeFilter(configuration)) || !CollectionUtils.isEmpty(ConfluenceConfigHelper.getSpacesNameExcludeFilter(configuration))) {
@@ -151,8 +154,12 @@ private StringBuilder createContentFilterCriteria(ConfluenceSourceConfig configu
151154
validatePageTypeFilters(configuration);
152155
}
153156

154-
String formattedTimeStamp = LocalDateTime.ofInstant(ts, ZoneId.systemDefault())
155-
.format(DateTimeFormatter.ofPattern("yyyy-MM-dd HH:mm"));
157+
if (this.confluenceServerZoneId == null) {
158+
// initialize confluence server timezone
159+
initializeConfluenceServerMetadata();
160+
}
161+
162+
String formattedTimeStamp = ts.atZone(this.confluenceServerZoneId).format(DateTimeFormatter.ofPattern(CQL_LAST_MODIFIED_DATE_FORMAT));
156163
StringBuilder cQl = new StringBuilder(LAST_MODIFIED + GREATER_THAN + "\"" + formattedTimeStamp + "\"");
157164
if (!CollectionUtils.isEmpty(ConfluenceConfigHelper.getSpacesNameIncludeFilter(configuration))) {
158165
cQl.append(SPACE_IN).append(ConfluenceConfigHelper.getSpacesNameIncludeFilter(configuration).stream()

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/main/java/org/opensearch/dataprepper/plugins/source/confluence/ConfluenceSource.java‎

Lines changed: 1 addition & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -47,7 +47,6 @@ public class ConfluenceSource extends CrawlerSourcePlugin {
4747
private static final Logger log = LoggerFactory.getLogger(ConfluenceSource.class);
4848
private final ConfluenceSourceConfig confluenceSourceConfig;
4949
private final AtlassianAuthConfig jiraOauthConfig;
50-
private final ConfluenceService service;
5150

5251
@DataPrepperPluginConstructor
5352
public ConfluenceSource(final PluginMetrics pluginMetrics,
@@ -56,21 +55,18 @@ public ConfluenceSource(final PluginMetrics pluginMetrics,
5655
final PluginFactory pluginFactory,
5756
final AcknowledgementSetManager acknowledgementSetManager,
5857
Crawler crawler,
59-
PluginExecutorServiceProvider executorServiceProvider,
60-
final ConfluenceService service) {
58+
PluginExecutorServiceProvider executorServiceProvider) {
6159
super(PLUGIN_NAME, pluginMetrics, confluenceSourceConfig, pluginFactory, acknowledgementSetManager, crawler, executorServiceProvider);
6260
log.info("Creating Confluence Source Plugin");
6361
this.confluenceSourceConfig = confluenceSourceConfig;
6462
this.jiraOauthConfig = jiraOauthConfig;
65-
this.service = service;
6663
}
6764

6865
@Override
6966
public void start(Buffer<Record<Event>> buffer) {
7067
log.info("Starting Confluence Source Plugin... ");
7168
ConfluenceConfigHelper.validateConfig(confluenceSourceConfig);
7269
jiraOauthConfig.initCredentials();
73-
super.setServerMetadata(service.getConfluenceServerMetadata());
7470
super.start(buffer);
7571
}
7672

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/main/java/org/opensearch/dataprepper/plugins/source/confluence/models/ConfluenceServerMetadata.java‎

Lines changed: 1 addition & 12 deletions
Original file line numberDiff line numberDiff line change
@@ -14,20 +14,17 @@
1414
import com.fasterxml.jackson.annotation.JsonIgnoreProperties;
1515
import com.fasterxml.jackson.annotation.JsonProperty;
1616
import lombok.Getter;
17-
import org.opensearch.dataprepper.plugins.source.confluence.utils.TimezoneHelper;
18-
import org.opensearch.dataprepper.plugins.source.source_crawler.base.SourceServerMetadata;
1917
import org.slf4j.Logger;
2018
import org.slf4j.LoggerFactory;
2119

22-
import java.time.Duration;
2320
import java.time.ZoneId;
2421

2522
/**
2623
* The result of a SystemInfo API call.
2724
*/
2825
@Getter
2926
@JsonIgnoreProperties(ignoreUnknown = true)
30-
public class ConfluenceServerMetadata implements SourceServerMetadata {
27+
public class ConfluenceServerMetadata {
3128

3229
Logger log = LoggerFactory.getLogger(ConfluenceServerMetadata.class);
3330

@@ -37,12 +34,4 @@ public class ConfluenceServerMetadata implements SourceServerMetadata {
3734
@JsonProperty("defaultTimeZone")
3835
private ZoneId defaultTimeZone = ZoneId.of("UTC");
3936

40-
@Override
41-
public Duration getPollingTimezoneOffset() {
42-
Duration pollingTimezoneOffset = TimezoneHelper.getUTCTimezoneOffset(defaultTimeZone);
43-
log.info("Confluence server default timezone: {} with pollingTimezoneOffset: {}",
44-
defaultTimeZone, pollingTimezoneOffset);
45-
return pollingTimezoneOffset;
46-
}
47-
4837
}

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/main/java/org/opensearch/dataprepper/plugins/source/confluence/rest/ConfluenceRestClient.java‎

Lines changed: 4 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -83,8 +83,10 @@ public ConfluenceServerMetadata getConfluenceServerMetadata() {
8383
/**
8484
* Method to get all Contents in a paginated fashion.
8585
*
86-
* @param cql input parameter.
87-
* @param startAt the start at
86+
* @param cql input parameter.
87+
* @param startAt the start at
88+
* {@link ConfluenceSearchResults} confluence search results
89+
* @param paginationLinks the pagination links
8890
* @return InputStream input stream
8991
*/
9092
public ConfluenceSearchResults getAllContent(StringBuilder cql, int startAt,

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/main/java/org/opensearch/dataprepper/plugins/source/confluence/utils/ConfluenceConfigHelper.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -30,6 +30,7 @@ public class ConfluenceConfigHelper {
3030
/**
3131
* Get Content Types Filter from configuration.
3232
*
33+
* @param repositoryConfiguration repo config
3334
* @return List Content Type Filter.
3435
*/
3536
public static List<String> getContentTypeIncludeFilter(ConfluenceSourceConfig repositoryConfiguration) {
@@ -49,6 +50,7 @@ public static List<String> getContentTypeExcludeFilter(ConfluenceSourceConfig re
4950
/**
5051
* Get Space Filter Types from configuration.
5152
*
53+
* @param repositoryConfiguration repo config
5254
* @return List Space Filter.
5355
*/
5456
public static List<String> getSpacesNameIncludeFilter(ConfluenceSourceConfig repositoryConfiguration) {
@@ -70,6 +72,12 @@ public static List<String> getSpacesNameExcludeFilter(ConfluenceSourceConfig rep
7072
}
7173

7274

75+
/**
76+
* Validate Confluence Configuration
77+
*
78+
* @param config holding user pipeline yaml inputs
79+
* @return boolean indicating pipeline yaml inputs are valid or not
80+
*/
7381
public static boolean validateConfig(ConfluenceSourceConfig config) {
7482
if (config.getAccountUrl() == null) {
7583
throw new RuntimeException("Account URL is missing.");

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/main/java/org/opensearch/dataprepper/plugins/source/confluence/utils/TimezoneHelper.java‎

Lines changed: 0 additions & 25 deletions
This file was deleted.

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/test/java/org/opensearch/dataprepper/plugins/source/confluence/ConfluenceClientTest.java‎

Lines changed: 1 addition & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -63,14 +63,13 @@ public class ConfluenceClientTest {
6363
@Test
6464
void testConstructor() {
6565
ConfluenceClient confluenceClient = new ConfluenceClient(confluenceService, confluenceIterator, executorServiceProvider, confluenceSourceConfig);
66-
confluenceClient.setLastPollTime(Instant.ofEpochSecond(1234L));
6766
assertNotNull(confluenceClient);
6867
}
6968

7069
@Test
7170
void testListItems() {
7271
ConfluenceClient confluenceClient = new ConfluenceClient(confluenceService, confluenceIterator, executorServiceProvider, confluenceSourceConfig);
73-
assertNotNull(confluenceClient.listItems());
72+
assertNotNull(confluenceClient.listItems(Instant.ofEpochSecond(1234L)));
7473
}
7574

7675

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/test/java/org/opensearch/dataprepper/plugins/source/confluence/ConfluenceIteratorTest.java‎

Lines changed: 0 additions & 7 deletions
Original file line numberDiff line numberDiff line change
@@ -16,7 +16,6 @@
1616
import org.mockito.Mock;
1717
import org.mockito.junit.jupiter.MockitoExtension;
1818
import org.opensearch.dataprepper.metrics.PluginMetrics;
19-
import org.opensearch.dataprepper.plugins.source.confluence.models.ConfluenceSearchResults;
2019
import org.opensearch.dataprepper.plugins.source.confluence.rest.ConfluenceRestClient;
2120
import org.opensearch.dataprepper.plugins.source.source_crawler.base.PluginExecutorServiceProvider;
2221

@@ -26,16 +25,11 @@
2625
import static org.junit.jupiter.api.Assertions.assertFalse;
2726
import static org.junit.jupiter.api.Assertions.assertNotNull;
2827
import static org.junit.jupiter.api.Assertions.assertThrows;
29-
import static org.mockito.ArgumentMatchers.any;
30-
import static org.mockito.ArgumentMatchers.anyInt;
31-
import static org.mockito.Mockito.doReturn;
3228
import static org.mockito.Mockito.spy;
3329

3430
@ExtendWith(MockitoExtension.class)
3531
public class ConfluenceIteratorTest {
3632

37-
@Mock
38-
private ConfluenceSearchResults mockConfluenceSearchResults;
3933
@Mock
4034
private ConfluenceRestClient confluenceRestClient;
4135
private ConfluenceService confluenceService;
@@ -59,7 +53,6 @@ void testInitialization() {
5953
confluenceIterator = createObjectUnderTest();
6054
assertNotNull(confluenceIterator);
6155
confluenceIterator.initialize(Instant.ofEpochSecond(0));
62-
doReturn(mockConfluenceSearchResults).when(confluenceRestClient).getAllContent(any(StringBuilder.class), anyInt(), any());
6356
assertFalse(confluenceIterator.hasNext());
6457
}
6558

‎data-prepper-plugins/saas-source-plugins/confluence-source/src/test/java/org/opensearch/dataprepper/plugins/source/confluence/ConfluenceServiceTest.java‎

Lines changed: 31 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,8 @@
1616
import org.junit.jupiter.api.AfterEach;
1717
import org.junit.jupiter.api.Test;
1818
import org.junit.jupiter.api.extension.ExtendWith;
19+
import org.junit.jupiter.params.ParameterizedTest;
20+
import org.junit.jupiter.params.provider.ValueSource;
1921
import org.mockito.Mock;
2022
import org.mockito.junit.jupiter.MockitoExtension;
2123
import org.opensearch.dataprepper.metrics.PluginMetrics;
@@ -25,6 +27,7 @@
2527
import org.opensearch.dataprepper.plugins.source.confluence.models.ConfluenceItem;
2628
import org.opensearch.dataprepper.plugins.source.confluence.models.ConfluencePaginationLinks;
2729
import org.opensearch.dataprepper.plugins.source.confluence.models.ConfluenceSearchResults;
30+
import org.opensearch.dataprepper.plugins.source.confluence.models.ConfluenceServerMetadata;
2831
import org.opensearch.dataprepper.plugins.source.confluence.models.SpaceItem;
2932
import org.opensearch.dataprepper.plugins.source.confluence.rest.ConfluenceRestClient;
3033
import org.opensearch.dataprepper.plugins.source.confluence.utils.MockPluginConfigVariableImpl;
@@ -37,6 +40,8 @@
3740
import java.io.IOException;
3841
import java.io.InputStream;
3942
import java.time.Instant;
43+
import java.time.ZoneId;
44+
import java.time.format.DateTimeFormatter;
4045
import java.util.ArrayList;
4146
import java.util.HashMap;
4247
import java.util.List;
@@ -49,6 +54,7 @@
4954
import static org.junit.jupiter.api.Assertions.assertEquals;
5055
import static org.junit.jupiter.api.Assertions.assertNotNull;
5156
import static org.junit.jupiter.api.Assertions.assertThrows;
57+
import static org.junit.jupiter.api.Assertions.assertTrue;
5258
import static org.mockito.ArgumentMatchers.any;
5359
import static org.mockito.ArgumentMatchers.anyInt;
5460
import static org.mockito.ArgumentMatchers.anyString;
@@ -57,6 +63,7 @@
5763
import static org.mockito.Mockito.spy;
5864
import static org.mockito.Mockito.when;
5965
import static org.opensearch.dataprepper.plugins.source.atlassian.rest.auth.AtlassianOauthConfig.ACCESSIBLE_RESOURCES;
66+
import static org.opensearch.dataprepper.plugins.source.confluence.ConfluenceService.CQL_LAST_MODIFIED_DATE_FORMAT;
6067
import static org.opensearch.dataprepper.plugins.source.confluence.utils.Constants.BASIC;
6168
import static org.opensearch.dataprepper.plugins.source.confluence.utils.Constants.OAUTH2;
6269

@@ -70,6 +77,8 @@ public class ConfluenceServiceTest {
7077
private static final Logger log = LoggerFactory.getLogger(ConfluenceServiceTest.class);
7178
@Mock
7279
private ConfluenceRestClient confluenceRestClient;
80+
@Mock
81+
private ConfluenceServerMetadata confluenceServerMetadata;
7382
private final PluginExecutorServiceProvider executorServiceProvider = new PluginExecutorServiceProvider();
7483
private final PluginMetrics pluginMetrics = PluginMetrics.fromNames("confluenceService", "aws");
7584

@@ -198,6 +207,8 @@ public void testGetPages() throws JsonProcessingException {
198207
when(paginationLinks.getNext()).thenReturn(null);
199208

200209
doReturn(mockConfluenceSearchResults).when(confluenceRestClient).getAllContent(any(StringBuilder.class), anyInt(), any());
210+
doReturn(confluenceServerMetadata).when(confluenceRestClient).getConfluenceServerMetadata();
211+
doReturn(ZoneId.of("UTC")).when(confluenceServerMetadata).getDefaultTimeZone();
201212

202213
Instant timestamp = Instant.ofEpochSecond(0);
203214
Queue<ItemInfo> itemInfoQueue = new ConcurrentLinkedQueue<>();
@@ -223,6 +234,8 @@ public void buildIssueItemInfoMultipleFutureThreads() throws JsonProcessingExcep
223234
when(mockConfluenceSearchResults.getResults()).thenReturn(mockIssues);
224235

225236
doReturn(mockConfluenceSearchResults).when(confluenceRestClient).getAllContent(any(StringBuilder.class), anyInt(), any());
237+
doReturn(confluenceServerMetadata).when(confluenceRestClient).getConfluenceServerMetadata();
238+
doReturn(ZoneId.of("UTC")).when(confluenceServerMetadata).getDefaultTimeZone();
226239

227240
Instant timestamp = Instant.ofEpochSecond(0);
228241
Queue<ItemInfo> itemInfoQueue = new ConcurrentLinkedQueue<>();
@@ -263,6 +276,24 @@ public void testGetPagesException() throws JsonProcessingException {
263276
assertThrows(RuntimeException.class, () -> confluenceService.getPages(confluenceSourceConfig, timestamp, itemInfoQueue));
264277
}
265278

279+
@ParameterizedTest
280+
@ValueSource(strings = {"America/Los_Angeles", "America/New_York", "Asia/Kolkata"})
281+
public void testCreateContentFilterCriteria(String confluenceServerTimezone) throws JsonProcessingException {
282+
List<String> pageType = new ArrayList<>();
283+
List<String> spaceKey = new ArrayList<>();
284+
ConfluenceSourceConfig confluenceSourceConfig = createConfluenceConfiguration(BASIC, pageType, spaceKey);
285+
doReturn(confluenceServerMetadata).when(confluenceRestClient).getConfluenceServerMetadata();
286+
ZoneId confluenceZoneId = ZoneId.of(confluenceServerTimezone);
287+
doReturn(confluenceZoneId).when(confluenceServerMetadata).getDefaultTimeZone();
288+
ConfluenceService confluenceService = new ConfluenceService(confluenceSourceConfig, confluenceRestClient, pluginMetrics);
289+
Instant pollingTime = Instant.now();
290+
String formattedZonedPollingTime = pollingTime.atZone(confluenceZoneId)
291+
.format(DateTimeFormatter.ofPattern(CQL_LAST_MODIFIED_DATE_FORMAT));
292+
StringBuilder contentFilterCriteria = confluenceService.createContentFilterCriteria(confluenceSourceConfig, pollingTime);
293+
assertNotNull(contentFilterCriteria);
294+
assertTrue(contentFilterCriteria.toString().contains(formattedZonedPollingTime));
295+
}
296+
266297

267298
private ConfluenceItem createConfluenceItemBean() {
268299
ConfluenceItem confluenceItem = new ConfluenceItem();

0 commit comments

Comments
 (0)