Skip to content

Commit d0c5b82

Browse files
committed
added tests
Signed-off-by: Kai Huang <[email protected]>
1 parent bdddb84 commit d0c5b82

File tree

3 files changed

+276
-48
lines changed

3 files changed

+276
-48
lines changed

legacy/src/main/java/org/opensearch/sql/legacy/executor/join/ElasticJoinExecutor.java

Lines changed: 12 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -52,19 +52,19 @@ public abstract class ElasticJoinExecutor extends ElasticHitsExecutor {
5252
private final Set<String> aliasesOnReturn;
5353
private final boolean allFieldsReturn;
5454
protected final String[] indices;
55-
protected final JoinRequestBuilder requestBuilder;
55+
protected final JoinRequestBuilder requestBuilder; // ✅ Added to store request builder
5656

5757
protected ElasticJoinExecutor(Client client, JoinRequestBuilder requestBuilder) {
5858
metaResults = new MetaSearchResult();
5959
aliasesOnReturn = new HashSet<>();
6060
List<Field> firstTableReturnedField = requestBuilder.getFirstTable().getReturnedFields();
6161
List<Field> secondTableReturnedField = requestBuilder.getSecondTable().getReturnedFields();
6262
allFieldsReturn =
63-
(firstTableReturnedField == null || firstTableReturnedField.size() == 0)
64-
&& (secondTableReturnedField == null || secondTableReturnedField.size() == 0);
63+
(firstTableReturnedField == null || firstTableReturnedField.size() == 0)
64+
&& (secondTableReturnedField == null || secondTableReturnedField.size() == 0);
6565
indices = getIndices(requestBuilder);
6666
this.client = client;
67-
this.requestBuilder = requestBuilder;
67+
this.requestBuilder = requestBuilder; // ✅ Store request builder for hint access
6868
}
6969

7070
public void sendResponse(RestChannel channel) throws IOException {
@@ -86,15 +86,16 @@ public void sendResponse(RestChannel channel) throws IOException {
8686
throw e;
8787
}
8888
LOG.debug(
89-
"[MCB] Successfully send response with size of {}. Thread id = {}",
90-
len,
91-
Thread.currentThread().getId());
89+
"[MCB] Successfully send response with size of {}. Thread id = {}",
90+
len,
91+
Thread.currentThread().getId());
9292
}
9393

9494
public void run() throws IOException, SqlParseException {
9595
try {
9696
long timeBefore = System.currentTimeMillis();
9797

98+
// ✅ Extract JOIN_TIME_OUT hint and create PIT with custom keepalive
9899
TimeValue customKeepAlive = extractJoinTimeoutFromHints();
99100
pit = customKeepAlive != null
100101
? new PointInTimeHandlerImpl(client, indices, customKeepAlive)
@@ -175,8 +176,10 @@ TimeValue findJoinTimeoutInHints(List<Hint> hints) {
175176
Object[] params = hint.getParams();
176177
if (params != null && params.length > 0) {
177178
Integer timeoutSeconds = (Integer) params[0];
178-
LOG.info("Found JOIN_TIME_OUT hint: {} seconds, applying to PIT keepalive", timeoutSeconds);
179-
return TimeValue.timeValueSeconds(timeoutSeconds);
179+
if (timeoutSeconds != null) {
180+
LOG.info("Found JOIN_TIME_OUT hint: {} seconds, applying to PIT keepalive", timeoutSeconds);
181+
return TimeValue.timeValueSeconds(timeoutSeconds);
182+
}
180183
}
181184
}
182185
}

legacy/src/test/java/org/opensearch/sql/legacy/executor/join/ElasticJoinExecutorTest.java

Lines changed: 53 additions & 30 deletions
Original file line numberDiff line numberDiff line change
@@ -6,79 +6,69 @@
66
package org.opensearch.sql.legacy.executor.join;
77

88
import static org.junit.Assert.*;
9-
import static org.mockito.ArgumentMatchers.any;
10-
import static org.mockito.ArgumentMatchers.eq;
119
import static org.mockito.Mockito.*;
1210

1311
import java.util.ArrayList;
1412
import java.util.Arrays;
13+
import java.util.Collections;
1514
import java.util.List;
15+
import lombok.SneakyThrows;
1616
import org.junit.Before;
1717
import org.junit.Test;
18-
import org.junit.runner.RunWith;
1918
import org.mockito.Mock;
20-
import org.mockito.junit.MockitoJUnitRunner;
19+
import org.mockito.MockitoAnnotations;
2120
import org.opensearch.common.unit.TimeValue;
21+
import org.opensearch.search.SearchHit;
22+
import org.opensearch.sql.legacy.domain.Field;
2223
import org.opensearch.sql.legacy.domain.Select;
2324
import org.opensearch.sql.legacy.domain.hints.Hint;
2425
import org.opensearch.sql.legacy.domain.hints.HintType;
25-
import org.opensearch.sql.legacy.pit.PointInTimeHandlerImpl;
2626
import org.opensearch.sql.legacy.query.join.JoinRequestBuilder;
2727
import org.opensearch.sql.legacy.query.join.TableInJoinRequestBuilder;
2828
import org.opensearch.transport.client.Client;
2929

30-
@RunWith(MockitoJUnitRunner.class)
3130
public class ElasticJoinExecutorTest {
3231

33-
@Mock
34-
private Client mockClient;
35-
36-
@Mock
37-
private JoinRequestBuilder mockJoinRequestBuilder;
38-
39-
@Mock
40-
private TableInJoinRequestBuilder mockFirstTable;
41-
42-
@Mock
43-
private TableInJoinRequestBuilder mockSecondTable;
44-
45-
@Mock
46-
private Select mockFirstSelect;
47-
48-
@Mock
49-
private Select mockSecondSelect;
32+
@Mock private Client mockClient;
33+
@Mock private JoinRequestBuilder mockJoinRequestBuilder;
34+
@Mock private TableInJoinRequestBuilder mockFirstTable;
35+
@Mock private TableInJoinRequestBuilder mockSecondTable;
36+
@Mock private Select mockFirstSelect;
37+
@Mock private Select mockSecondSelect;
5038

5139
private TestableElasticJoinExecutor executor;
5240

53-
// Concrete test implementation to access private methods
41+
// Concrete test implementation to access private methods and test behavior
5442
private static class TestableElasticJoinExecutor extends ElasticJoinExecutor {
5543

5644
public TestableElasticJoinExecutor(Client client, JoinRequestBuilder requestBuilder) {
5745
super(client, requestBuilder);
5846
}
5947

6048
@Override
61-
protected List<org.opensearch.search.SearchHit> innerRun() {
49+
protected List<SearchHit> innerRun() {
6250
return new ArrayList<>();
6351
}
6452

6553
// Expose private methods for testing
6654
public TimeValue testExtractJoinTimeoutFromHints() {
67-
return super.extractJoinTimeoutFromHints();
55+
return extractJoinTimeoutFromHints();
6856
}
6957

7058
public TimeValue testGetJoinTimeoutFromTable(TableInJoinRequestBuilder table) {
71-
return super.getJoinTimeoutFromTable(table);
59+
return getJoinTimeoutFromTable(table);
7260
}
7361

7462
public TimeValue testFindJoinTimeoutInHints(List<Hint> hints) {
75-
return super.findJoinTimeoutInHints(hints);
63+
return findJoinTimeoutInHints(hints);
7664
}
7765
}
7866

7967
@Before
8068
public void setUp() {
81-
// Setup mock chain
69+
MockitoAnnotations.initMocks(this);
70+
71+
// Setup mock chain for constructor requirements
8272
when(mockJoinRequestBuilder.getFirstTable()).thenReturn(mockFirstTable);
8373
when(mockJoinRequestBuilder.getSecondTable()).thenReturn(mockSecondTable);
8474
when(mockFirstTable.getOriginalSelect()).thenReturn(mockFirstSelect);
@@ -284,7 +274,7 @@ public void testExtractJoinTimeoutFromHints_ExceptionHandling() {
284274

285275
@Test
286276
public void testBoundaryValues() {
287-
// Test various timeout values
277+
// Test various timeout values that might be used in real scenarios
288278
int[] testValues = {1, 30, 60, 90, 300, 3600, 7200}; // 1s to 2hrs
289279

290280
for (int timeoutSeconds : testValues) {
@@ -301,6 +291,39 @@ public void testBoundaryValues() {
301291
}
302292
}
303293

294+
@Test
295+
public void testTypicalJoinTimeoutScenarios() {
296+
// Test common JOIN_TIME_OUT scenarios
297+
int[] commonTimeouts = {90, 180, 300, 600}; // 1.5min, 3min, 5min, 10min
298+
299+
for (int timeout : commonTimeouts) {
300+
// Arrange
301+
List<Hint> hints = createJoinTimeoutHints(timeout);
302+
when(mockFirstSelect.getHints()).thenReturn(hints);
303+
when(mockSecondSelect.getHints()).thenReturn(new ArrayList<>());
304+
305+
// Act
306+
TimeValue result = executor.testExtractJoinTimeoutFromHints();
307+
308+
// Assert
309+
assertNotNull("Should extract JOIN_TIME_OUT(" + timeout + ")", result);
310+
assertEquals("Should extract " + timeout + " seconds", timeout, result.getSeconds());
311+
}
312+
}
313+
314+
@Test
315+
public void testConstructorStoresRequestBuilder() {
316+
// Verify that the constructor properly stores the request builder for hint access
317+
assertNotNull("Executor should be created successfully", executor);
318+
319+
// Test that the executor can access hints through the stored request builder
320+
List<Hint> hints = createJoinTimeoutHints(90);
321+
when(mockFirstSelect.getHints()).thenReturn(hints);
322+
323+
TimeValue result = executor.testExtractJoinTimeoutFromHints();
324+
assertNotNull("Should be able to access hints through stored request builder", result);
325+
}
326+
304327
// Helper Methods
305328

306329
private List<Hint> createJoinTimeoutHints(int timeoutSeconds) {

0 commit comments

Comments
 (0)