diff --git a/.github/skills/review-code/SKILL.md b/.github/skills/review-code/SKILL.md index f170200a4fd..675e5167a18 100644 --- a/.github/skills/review-code/SKILL.md +++ b/.github/skills/review-code/SKILL.md @@ -98,7 +98,17 @@ grep -rn "TenantBase\|tenant_id\|TenantContext" --include="*.java" $(git diff -- git diff --name-only HEAD~1 | grep -E "\.tsx$|\.ts$" | head -10 ``` -## Step 8 — Compile review +## Step 8 — Check common anti-patterns (learned from reviews) + +Apply these checks based on past review feedback: + +- **Root cause vs workaround**: If a bug is backend-originated, don't add frontend workarounds (onError handlers, fallback states). Fix the root cause in the correct layer. +- **String/value duplication**: When introducing new methods that share data with existing methods (e.g., logo filenames, config keys), extract the shared value to a private method or constant — never compute the same formatted string in multiple places. +- **Non-critical operations**: Startup operations that interact with external services (MinIO, S3, etc.) for non-critical assets (logos, thumbnails) should be best-effort: wrap in try-catch with `log.warn` so failures don't block application startup. +- **Test proportionality**: Don't require tests for small mechanical changes (1-line additions, parameter threading). Tests should be proportionate to the risk and complexity of the change. +- **Pre-existing issues**: Don't fix unrelated pre-existing issues (e.g., alt text, parameter naming) in a bug fix PR. Track them as follow-up. + +## Step 9 — Compile review Generate the Code Review Summary following the output format defined in `code-reviewer.agent.md`. diff --git a/openaev-api/src/main/java/io/openaev/integration/IntegrationFactory.java b/openaev-api/src/main/java/io/openaev/integration/IntegrationFactory.java index 5f4c88253e3..340f166aeda 100644 --- a/openaev-api/src/main/java/io/openaev/integration/IntegrationFactory.java +++ b/openaev-api/src/main/java/io/openaev/integration/IntegrationFactory.java @@ -25,11 +25,29 @@ public abstract class IntegrationFactory { protected abstract String getClassName(); + /** + * Ensures the catalog logo exists in MinIO. Called on every startup (best-effort), even when the + * catalog entry already exists in the database. Override in subclasses that upload a logo during + * {@link #insertCatalogEntry()}. + * + *
Default implementation is a no-op for factories that do not manage a catalog logo (e.g.
+ * built-in executors).
+ */
+ protected void ensureCatalogLogo() throws Exception {
+ // no-op by default
+ }
+
@Transactional(rollbackFor = Exception.class)
public void initialise() throws Exception {
String className = this.getClassName();
if (catalogConnectorService.findByFactoryClassName(className).isEmpty()) {
insertCatalogEntry();
+ } else {
+ try {
+ ensureCatalogLogo();
+ } catch (Exception e) {
+ log.warn("Failed to ensure catalog logo for {}: {}", className, e.getMessage());
+ }
}
runMigrations();
diff --git a/openaev-api/src/main/java/io/openaev/integration/impl/executors/caldera/CalderaExecutorIntegrationFactory.java b/openaev-api/src/main/java/io/openaev/integration/impl/executors/caldera/CalderaExecutorIntegrationFactory.java
index fe5418e7eef..224c20b3b93 100644
--- a/openaev-api/src/main/java/io/openaev/integration/impl/executors/caldera/CalderaExecutorIntegrationFactory.java
+++ b/openaev-api/src/main/java/io/openaev/integration/impl/executors/caldera/CalderaExecutorIntegrationFactory.java
@@ -83,13 +83,22 @@ protected void runMigrations() throws Exception {
calderaExecutorConfigurationMigration.migrate();
}
+ private String getLogoFilename() {
+ return "%s-logo.png".formatted(CALDERA_EXECUTOR_TYPE);
+ }
+
@Override
- protected void insertCatalogEntry() throws Exception {
- String logoFilename = "%s-logo.png".formatted(CALDERA_EXECUTOR_TYPE);
+ protected void ensureCatalogLogo() throws Exception {
fileService.uploadCatalogLogo(
FileService.CONNECTORS_LOGO_PATH,
- logoFilename,
+ getLogoFilename(),
getClass().getResourceAsStream("/img/icon-caldera.png"));
+ }
+
+ @Override
+ protected void insertCatalogEntry() throws Exception {
+ ensureCatalogLogo();
+ String logoFilename = getLogoFilename();
CatalogConnector connector = new CatalogConnector();
connector.setTitle("Caldera Executor");
connector.setSlug(CALDERA_EXECUTOR_TYPE);
diff --git a/openaev-api/src/main/java/io/openaev/integration/impl/executors/crowdstrike/CrowdStrikeExecutorIntegrationFactory.java b/openaev-api/src/main/java/io/openaev/integration/impl/executors/crowdstrike/CrowdStrikeExecutorIntegrationFactory.java
index 82a2d6334f4..7f0f6f82429 100644
--- a/openaev-api/src/main/java/io/openaev/integration/impl/executors/crowdstrike/CrowdStrikeExecutorIntegrationFactory.java
+++ b/openaev-api/src/main/java/io/openaev/integration/impl/executors/crowdstrike/CrowdStrikeExecutorIntegrationFactory.java
@@ -86,13 +86,22 @@ protected void runMigrations() throws Exception {
crowdStrikeExecutorConfigurationMigration.migrate();
}
+ private String getLogoFilename() {
+ return "%s-logo.png".formatted(CROWDSTRIKE_EXECUTOR_TYPE);
+ }
+
@Override
- protected void insertCatalogEntry() throws Exception {
- String logoFilename = "%s-logo.png".formatted(CROWDSTRIKE_EXECUTOR_TYPE);
+ protected void ensureCatalogLogo() throws Exception {
fileService.uploadCatalogLogo(
FileService.CONNECTORS_LOGO_PATH,
- logoFilename,
+ getLogoFilename(),
getClass().getResourceAsStream("/img/icon-crowdstrike.png"));
+ }
+
+ @Override
+ protected void insertCatalogEntry() throws Exception {
+ ensureCatalogLogo();
+ String logoFilename = getLogoFilename();
CatalogConnector connector = new CatalogConnector();
connector.setTitle("CrowdStrike Executor");
connector.setSlug(CROWDSTRIKE_EXECUTOR_TYPE);
diff --git a/openaev-api/src/main/java/io/openaev/integration/impl/executors/paloaltocortex/PaloAltoCortexExecutorIntegrationFactory.java b/openaev-api/src/main/java/io/openaev/integration/impl/executors/paloaltocortex/PaloAltoCortexExecutorIntegrationFactory.java
index dd6aebad4ed..f7438a7f451 100644
--- a/openaev-api/src/main/java/io/openaev/integration/impl/executors/paloaltocortex/PaloAltoCortexExecutorIntegrationFactory.java
+++ b/openaev-api/src/main/java/io/openaev/integration/impl/executors/paloaltocortex/PaloAltoCortexExecutorIntegrationFactory.java
@@ -80,13 +80,22 @@ protected void runMigrations() throws Exception {
// No
}
+ private String getLogoFilename() {
+ return "%s-logo.png".formatted(PALOALTOCORTEX_EXECUTOR_TYPE);
+ }
+
@Override
- protected void insertCatalogEntry() throws Exception {
- String logoFilename = "%s-logo.png".formatted(PALOALTOCORTEX_EXECUTOR_TYPE);
+ protected void ensureCatalogLogo() throws Exception {
fileService.uploadCatalogLogo(
FileService.CONNECTORS_LOGO_PATH,
- logoFilename,
+ getLogoFilename(),
getClass().getResourceAsStream("/img/icon-paloaltocortex.png"));
+ }
+
+ @Override
+ protected void insertCatalogEntry() throws Exception {
+ ensureCatalogLogo();
+ String logoFilename = getLogoFilename();
CatalogConnector connector = new CatalogConnector();
connector.setTitle("Palo Alto Cortex Executor");
connector.setSlug(PALOALTOCORTEX_EXECUTOR_TYPE);
diff --git a/openaev-api/src/main/java/io/openaev/integration/impl/executors/sentinelone/SentinelOneExecutorIntegrationFactory.java b/openaev-api/src/main/java/io/openaev/integration/impl/executors/sentinelone/SentinelOneExecutorIntegrationFactory.java
index 81a09efcd60..19b3f3690e2 100644
--- a/openaev-api/src/main/java/io/openaev/integration/impl/executors/sentinelone/SentinelOneExecutorIntegrationFactory.java
+++ b/openaev-api/src/main/java/io/openaev/integration/impl/executors/sentinelone/SentinelOneExecutorIntegrationFactory.java
@@ -84,13 +84,22 @@ protected void runMigrations() throws Exception {
sentinelOneExecutorConfigurationMigration.migrate();
}
+ private String getLogoFilename() {
+ return "%s-logo.png".formatted(SENTINELONE_EXECUTOR_TYPE);
+ }
+
@Override
- protected void insertCatalogEntry() throws Exception {
- String logoFilename = "%s-logo.png".formatted(SENTINELONE_EXECUTOR_TYPE);
+ protected void ensureCatalogLogo() throws Exception {
fileService.uploadCatalogLogo(
FileService.CONNECTORS_LOGO_PATH,
- logoFilename,
+ getLogoFilename(),
getClass().getResourceAsStream("/img/icon-sentinelone.png"));
+ }
+
+ @Override
+ protected void insertCatalogEntry() throws Exception {
+ ensureCatalogLogo();
+ String logoFilename = getLogoFilename();
CatalogConnector connector = new CatalogConnector();
connector.setTitle("SentinelOne Executor");
connector.setSlug(SENTINELONE_EXECUTOR_TYPE);
diff --git a/openaev-api/src/main/java/io/openaev/integration/impl/executors/tanium/TaniumExecutorIntegrationFactory.java b/openaev-api/src/main/java/io/openaev/integration/impl/executors/tanium/TaniumExecutorIntegrationFactory.java
index 4028cbe58d1..a148ac4bbb8 100644
--- a/openaev-api/src/main/java/io/openaev/integration/impl/executors/tanium/TaniumExecutorIntegrationFactory.java
+++ b/openaev-api/src/main/java/io/openaev/integration/impl/executors/tanium/TaniumExecutorIntegrationFactory.java
@@ -87,13 +87,22 @@ protected void runMigrations() throws Exception {
taniumExecutorConfigurationMigration.migrate();
}
+ private String getLogoFilename() {
+ return "%s-logo.png".formatted(TANIUM_EXECUTOR_TYPE);
+ }
+
@Override
- protected void insertCatalogEntry() throws Exception {
- String logoFilename = "%s-logo.png".formatted(TANIUM_EXECUTOR_TYPE);
+ protected void ensureCatalogLogo() throws Exception {
fileService.uploadCatalogLogo(
FileService.CONNECTORS_LOGO_PATH,
- logoFilename,
+ getLogoFilename(),
getClass().getResourceAsStream("/img/icon-tanium.png"));
+ }
+
+ @Override
+ protected void insertCatalogEntry() throws Exception {
+ ensureCatalogLogo();
+ String logoFilename = getLogoFilename();
CatalogConnector connector = new CatalogConnector();
connector.setTitle("Tanium Executor");
connector.setSlug(TANIUM_EXECUTOR_TYPE);
diff --git a/openaev-api/src/main/java/io/openaev/integration/impl/injectors/opencti/OpenCTIInjectorIntegrationFactory.java b/openaev-api/src/main/java/io/openaev/integration/impl/injectors/opencti/OpenCTIInjectorIntegrationFactory.java
index 2b980aed503..be92cb3a37b 100644
--- a/openaev-api/src/main/java/io/openaev/integration/impl/injectors/opencti/OpenCTIInjectorIntegrationFactory.java
+++ b/openaev-api/src/main/java/io/openaev/integration/impl/injectors/opencti/OpenCTIInjectorIntegrationFactory.java
@@ -79,13 +79,22 @@ protected void runMigrations() throws Exception {
openctiInjectorConfigurationMigration.migrate();
}
+ private String getLogoFilename() {
+ return "%s-logo.png".formatted(openCTIContract.TYPE);
+ }
+
@Override
- protected void insertCatalogEntry() throws Exception {
- String logoFilename = "%s-logo.png".formatted(openCTIContract.TYPE);
+ protected void ensureCatalogLogo() throws Exception {
fileService.uploadCatalogLogo(
FileService.CONNECTORS_LOGO_PATH,
- logoFilename,
+ getLogoFilename(),
getClass().getResourceAsStream("/img/icon-opencti.png"));
+ }
+
+ @Override
+ protected void insertCatalogEntry() throws Exception {
+ ensureCatalogLogo();
+ String logoFilename = getLogoFilename();
CatalogConnector connector = new CatalogConnector();
connector.setTitle(OPENCTI_INJECTOR_NAME);
connector.setSlug(openCTIContract.TYPE);
diff --git a/openaev-api/src/main/java/io/openaev/integration/impl/injectors/ovh/OvhInjectorIntegrationFactory.java b/openaev-api/src/main/java/io/openaev/integration/impl/injectors/ovh/OvhInjectorIntegrationFactory.java
index c264570ce5e..d63b148e461 100644
--- a/openaev-api/src/main/java/io/openaev/integration/impl/injectors/ovh/OvhInjectorIntegrationFactory.java
+++ b/openaev-api/src/main/java/io/openaev/integration/impl/injectors/ovh/OvhInjectorIntegrationFactory.java
@@ -75,14 +75,23 @@ protected void runMigrations() throws Exception {
ovhInjectorConfigurationMigration.migrate();
}
+ private String getLogoFilename() {
+ return "%s-logo.png".formatted(ovhSmsContract.getType());
+ }
+
@Override
- protected void insertCatalogEntry() throws Exception {
- CatalogConnector connector = new CatalogConnector();
- String logoFilename = "%s-logo.png".formatted(ovhSmsContract.getType());
+ protected void ensureCatalogLogo() throws Exception {
fileService.uploadCatalogLogo(
FileService.CONNECTORS_LOGO_PATH,
- logoFilename,
+ getLogoFilename(),
getClass().getResourceAsStream("/img/icon-ovh-sms.png"));
+ }
+
+ @Override
+ protected void insertCatalogEntry() throws Exception {
+ ensureCatalogLogo();
+ CatalogConnector connector = new CatalogConnector();
+ String logoFilename = getLogoFilename();
connector.setTitle("OVHCloud SMS Platform");
connector.setSlug(ovhSmsContract.getType());
connector.setLogoUrl(logoFilename);
diff --git a/openaev-api/src/main/java/io/openaev/migration/V5_35__Add_indexing_last_id.java b/openaev-api/src/main/java/io/openaev/migration/V5_35__Add_indexing_last_id.java
new file mode 100644
index 00000000000..d20e908c2e0
--- /dev/null
+++ b/openaev-api/src/main/java/io/openaev/migration/V5_35__Add_indexing_last_id.java
@@ -0,0 +1,18 @@
+package io.openaev.migration;
+
+import java.sql.Statement;
+import org.flywaydb.core.api.migration.BaseJavaMigration;
+import org.flywaydb.core.api.migration.Context;
+import org.springframework.stereotype.Component;
+
+@Component
+public class V5_35__Add_indexing_last_id extends BaseJavaMigration {
+
+ @Override
+ public void migrate(Context context) throws Exception {
+ try (Statement statement = context.getConnection().createStatement()) {
+ statement.execute(
+ "ALTER TABLE indexing_status ADD COLUMN IF NOT EXISTS indexing_last_id TEXT NULL");
+ }
+ }
+}
diff --git a/openaev-api/src/test/java/io/openaev/integration/local_fixtures/factory_throws/TestIntegrationFactoryInitThrows.java b/openaev-api/src/test/java/io/openaev/integration/local_fixtures/factory_throws/TestIntegrationFactoryInitThrows.java
index f89c91046c9..ed975b7cfaa 100644
--- a/openaev-api/src/test/java/io/openaev/integration/local_fixtures/factory_throws/TestIntegrationFactoryInitThrows.java
+++ b/openaev-api/src/test/java/io/openaev/integration/local_fixtures/factory_throws/TestIntegrationFactoryInitThrows.java
@@ -51,13 +51,22 @@ protected void runMigrations() throws Exception {
throw new RuntimeException("%s: deliberate throw!".formatted(this.getClassName()));
}
+ private String getLogoFilename() {
+ return "%s-logo.png".formatted(getClassName());
+ }
+
@Override
- protected void insertCatalogEntry() throws Exception {
- String logoFilename = "%s-logo.png".formatted(getClassName());
+ protected void ensureCatalogLogo() throws Exception {
fileService.uploadCatalogLogo(
FileService.CONNECTORS_LOGO_PATH,
- logoFilename,
+ getLogoFilename(),
getClass().getResourceAsStream("/img/icon-default.png"));
+ }
+
+ @Override
+ protected void insertCatalogEntry() throws Exception {
+ ensureCatalogLogo();
+ String logoFilename = getLogoFilename();
CatalogConnector connector = new CatalogConnector();
connector.setTitle("Test Integration Init Throws");
connector.setSlug(getClassName());
diff --git a/openaev-api/src/test/java/io/openaev/integration/local_fixtures/regular/TestIntegrationFactory.java b/openaev-api/src/test/java/io/openaev/integration/local_fixtures/regular/TestIntegrationFactory.java
index abbd294ca13..dff9b49141b 100644
--- a/openaev-api/src/test/java/io/openaev/integration/local_fixtures/regular/TestIntegrationFactory.java
+++ b/openaev-api/src/test/java/io/openaev/integration/local_fixtures/regular/TestIntegrationFactory.java
@@ -50,13 +50,22 @@ protected void runMigrations() throws Exception {
testIntegrationConfigurationMigration.migrate();
}
+ private String getLogoFilename() {
+ return "%s-logo.png".formatted(getClassName());
+ }
+
@Override
- protected void insertCatalogEntry() throws Exception {
- String logoFilename = "%s-logo.png".formatted(getClassName());
+ protected void ensureCatalogLogo() throws Exception {
fileService.uploadCatalogLogo(
FileService.CONNECTORS_LOGO_PATH,
- logoFilename,
+ getLogoFilename(),
getClass().getResourceAsStream("/img/icon-default.png"));
+ }
+
+ @Override
+ protected void insertCatalogEntry() throws Exception {
+ ensureCatalogLogo();
+ String logoFilename = getLogoFilename();
CatalogConnector connector = new CatalogConnector();
connector.setTitle("Test Integration");
connector.setSlug(getClassName());
diff --git a/openaev-model/src/main/java/io/openaev/database/model/IndexingStatus.java b/openaev-model/src/main/java/io/openaev/database/model/IndexingStatus.java
index 2c3cff7bcc8..4f185a25a95 100644
--- a/openaev-model/src/main/java/io/openaev/database/model/IndexingStatus.java
+++ b/openaev-model/src/main/java/io/openaev/database/model/IndexingStatus.java
@@ -23,4 +23,10 @@ public class IndexingStatus {
@Column(name = "indexing_status_indexing_date")
@JsonProperty("indexing_status_indexing_date")
private Instant lastIndexing;
+
+ /** Last processed entity ID within the current indexing timestamp window (compound cursor). */
+ @Getter
+ @Column(name = "indexing_last_id")
+ @JsonProperty("indexing_last_id")
+ private String lastId;
}
diff --git a/openaev-model/src/main/java/io/openaev/database/repository/InjectExpectationRepository.java b/openaev-model/src/main/java/io/openaev/database/repository/InjectExpectationRepository.java
index 05c7d4a0df7..d96bf4fdf87 100644
--- a/openaev-model/src/main/java/io/openaev/database/repository/InjectExpectationRepository.java
+++ b/openaev-model/src/main/java/io/openaev/database/repository/InjectExpectationRepository.java
@@ -381,6 +381,85 @@ LEFT JOIN LATERAL jsonb_array_elements(ie.inject_expectation_results::jsonb) AS
List The default implementation ignores {@code lastId} and falls back to the basic cursor,
+ * providing backward-compatible behaviour for handlers that do not need compound pagination.
+ * Override in handlers where the same-millisecond duplicate issue is observable (e.g.
+ * InjectExpectationHandler).
+ *
+ * @param from lower-bound timestamp (exclusive unless lastId is provided)
+ * @param lastId ID of the last successfully indexed entity at {@code from}; when non-null the
+ * query returns items at {@code from} with ID strictly greater than this value, plus all
+ * items strictly after {@code from}
+ * @param limit maximum number of records to fetch per batch
+ * @return list data to index
+ */
+ default List