Importsql v2 #41

Nefarious46 · 2025-03-26T10:13:48Z

Contains the following features and objects.

ImportSQL
Media objects for handling log_group, stream and host
SQL objects for Host and Capture
SQLMedia object to print querys using jOOQ
Changed object attributes to method input parameters

src/main/java/com/teragrep/cfe_41/importsql/Importsql.java

src/main/java/com/teragrep/cfe_41/importsql/SQLCapture.java

src/main/java/com/teragrep/cfe_41/importsql/SQLHost.java

src/main/java/com/teragrep/cfe_41/importsql/SQLTemplate.java

src/test/java/com/teragrep/cfe_41/captureGroup/PartialCaptureResponseTest.java

kortemik

there is a mutable pathogen in the code: StringWriter(). please see comments and redesign.

kortemik · 2025-04-09T09:31:49Z

src/main/java/com/teragrep/cfe_41/captureGroup/CaptureGroupResponse.java

+            partialCaptureGroupRespons.add(new PartialCaptureGroupResponse(key.asJsonObject()));
        }
-        return partialCaptureResponses;
+        return partialCaptureGroupRespons;


partialCaptureGroupRespons -> partialCaptureGroupResponses

kortemik · 2025-04-09T11:39:04Z

src/main/java/com/teragrep/cfe_41/importsql/SQLHost.java

+        this.hostRequest = hostRequest;
+    }
+
+    public List<SQLMediaHost> asSQLHosts(final String captureGroupName) throws IOException {


object name SQLHost and methodName asSQLHost are not coherent. is the responsibility design correct?

renamed to asSqlMediaHosts since it requests hosts for current group and returns them as list of SqlMediaHosts

kortemik · 2025-04-09T11:40:20Z

src/main/java/com/teragrep/cfe_41/importsql/SQLCapture.java

+    }
+
+    // Gets all the capture and returns a list for sql
+    public List<SQLMediaCapture> asSQLCaptures(final String captureGroupName) throws IOException {


object name SQLCapture and method name asSQLCaptures are not coherent. please check the responsibility design. what is the responsibility of SQLCapture?

renamed to asSqlMediaCaptures since it requests captures for current group and returns them as list of SqlMediaCaptures.

kortemik · 2025-04-09T11:45:24Z

src/main/java/com/teragrep/cfe_41/importsql/Importsql.java

+        if (o == null || getClass() != o.getClass()) {
+            return false;
+        }
+        final Importsql importsql = (Importsql) o;


use name that

kortemik · 2025-04-09T11:51:13Z

src/main/java/com/teragrep/cfe_41/media/SQLMedia.java

+    public String asSql() {
+        final SQLTemplate template = new SQLTemplate();
+        query.append(template.endTemplate());
+        return query.toString();


do call asSql() two times, what do you get? a mutation i'd guess!

SQLTemplate removed as a whole.

kortemik · 2025-04-09T12:28:42Z

src/test/java/com/teragrep/cfe_41/sink/SinkRequestImplTest.java

        ApiConfig apiConfig = Assertions.assertDoesNotThrow(() -> new ApiConfig(cfg.asMap()));

-        SinkRequest fakeRequest = new SinkRequest("flow1", "protocol1", apiConfig);
+        SinkRequestImpl fakeRequest = new SinkRequestImpl(apiConfig);


use interface instead of implementation for type

kortemik · 2025-04-09T12:29:02Z

src/test/java/com/teragrep/cfe_41/importsql/SQLMediaHostTest.java

+
+        SQLStatementMedia expected = sqlMediaExpected.withHost("fake-fqhost", "captureGroup1");
+
+        Assertions.assertEquals(expected.asSql(), actual.asSql());


run actual.asSql() once before testing equality

kortemik · 2025-04-09T12:31:14Z

src/test/java/com/teragrep/cfe_41/importsql/SQLMediaCaptureTest.java

+        SQLStatementMedia expected = sqlMediaExpected
+                .withStream("captureGroup1", "fake-index", "fake-sourcetype", "fake-tag");
+
+        Assertions.assertEquals(actual.asSql(), expected.asSql());


wrong order of arguments. first expected, then actual.
run actual.asSql() once before testing equality.

kortemik · 2025-04-09T12:32:09Z

src/test/java/com/teragrep/cfe_41/importsql/SQLMediaCaptureGroupTest.java

+        SQLStatementMedia actual = sqlMediaCaptureGroup.asSql(sqlMediaActual);
+        SQLStatementMedia expected = sqlMediaExpected.withLogGroup("captureGroup1");
+
+        Assertions.assertEquals(actual.asSql(), expected.asSql());


wrong order of arguments. first expected then actual.
run actual.asSql() once before testing equality

kortemik · 2025-04-09T12:33:38Z

src/test/java/com/teragrep/cfe_41/importsql/ImportsqlTest.java

-        Assertions.assertFalse(captureStoragesArray.isEmpty());
-        // Asserting that contents equal to each other
-        Assertions.assertEquals(actualCaptureStorageResponse.hashCode(), expectedCaptureStorageResponse.hashCode());
+        String actual = Assertions.assertDoesNotThrow(() -> importsql.toString());


run toString() and see what happens

…sted in PR teragrep#41

eemhu · 2025-09-17T10:57:39Z

src/main/java/com/teragrep/cfe_41/captureGroup/CaptureGroup.java

+
+import com.teragrep.cfe_41.Stored;
+
+public interface CaptureGroup extends Stored {


Perhaps the method names here could be simplified:

groupName -> name

captureGroupType -> type

eemhu · 2025-09-17T10:59:56Z

src/main/java/com/teragrep/cfe_41/captureGroup/CaptureGroupResponse.java

-    public List<PartialCaptureResponse> partialCaptureResponses() {
-        final List<PartialCaptureResponse> partialCaptureResponses = new ArrayList<>();
+    public List<PartialCaptureGroupResponse> partialCaptureResponses() {
+        final List<PartialCaptureGroupResponse> partialCaptureGroupResponse = new ArrayList<>();


response -> responses

eemhu · 2025-09-17T12:25:10Z

src/main/java/com/teragrep/cfe_41/sink/SinkRequestImpl.java

+
+    @Override
+    public SinkResponse sinkResponse(final String flowName, final String protocolType) throws IOException {
+        final JsonArray sinksArray = new Response(new RequestData("/sink", apiConfig).doRequest()).asJsonArray();


should this response be validated?

eemhu · 2025-09-17T12:40:08Z

src/test/java/com/teragrep/cfe_41/importsql/SQLMediaCaptureGroupTest.java

+        SQLMedia sqlMediaActual = new SQLMedia();
+        SQLMedia sqlMediaExpected = new SQLMedia();
+
+        SQLStatementMedia actual = sqlMediaCaptureGroup.asSql(sqlMediaActual);


I think the expected values should be hardcoded instead of using the implementations

Nefarious46 added 5 commits March 25, 2025 23:31

importSQL, SQL files and SQL Media files.

5466a1e

Changes required by checkstyle

4f2dc08

Importsql objects can now be faked for tests.

4668230

Minor renames and unit tests on Media and SQL objects.

82e1201

Checkstyle fixes

7ee80a2

Nefarious46 added the enhancement New feature or request label Mar 26, 2025

Nefarious46 requested a review from eemhu March 26, 2025 10:13