Copilot commented on code in PR #2298:
URL:
https://github.com/apache/incubator-kie-kogito-apps/pull/2298#discussion_r2798327463
##########
data-index/data-index-service/data-index-service-common/src/main/java/org/kie/kogito/index/service/graphql/GraphQLProtoSchemaMapper.java:
##########
@@ -118,6 +119,11 @@ public void onDomainModelRegisteredEvent(@Observes
DomainModelRegisteredEvent ev
codeBuilder.dataFetcher(coordinates("Query",
rootType.getName()),
schemaManager.getDomainModelDataFetcher(event.getProcessId()));
codeBuilder.dataFetcher(coordinates("Subscription",
rootType.getName() + "Added"),
schemaManager.getDomainModelAddedDataFetcher(event.getProcessId()));
codeBuilder.dataFetcher(coordinates("Subscription",
rootType.getName() + "Updated"),
schemaManager.getDomainModelUpdatedDataFetcher(event.getProcessId()));
+ // graphql-java 24.x LightDataFetcher optimization bypasses
instrumentDataFetcher,
+ // so PropertyDataFetcher is no longer intercepted for
JsonNode-based sources (MongoDB).
+ // Set default data fetcher factory to JsonPropertyDataFetcher
which handles both
+ // JsonNode (MongoDB) and POJO (PostgreSQL) sources.
+ codeBuilder.defaultDataFetcher(env -> new
JsonPropertyDataFetcher());
Review Comment:
defaultDataFetcher is configured with `env -> new
JsonPropertyDataFetcher()`, which will allocate a new DataFetcher instance each
time the factory is invoked. JsonPropertyDataFetcher appears stateless, so
reuse a single instance (e.g., a static final) to avoid unnecessary allocations
when rebuilding the schema/code registry.
##########
data-audit/kogito-addons-data-audit-jpa/kogito-addons-data-audit-jpa-common/src/main/java/org/kie/kogito/app/audit/jpa/queries/mapper/PojoMapper.java:
##########
@@ -46,14 +51,60 @@ public PojoMapper(Class<T> clazz) {
@Override
public List<T> produce(List<Object[]> data) {
List<T> transformed = new ArrayList<>();
+ Class<?>[] paramTypes = defaultConstructor.getParameterTypes();
for (Object[] row : data) {
try {
- transformed.add(defaultConstructor.newInstance(row));
+ // Hibernate 7 changed default return types for native queries
+ // (e.g. OffsetDateTime instead of java.util.Date, Long
instead of Integer).
+ // Convert each value to match the constructor's declared
parameter types.
+ Object[] convertedRow = convertTypes(row, paramTypes);
+ transformed.add(defaultConstructor.newInstance(convertedRow));
} catch (InstantiationException | IllegalAccessException |
IllegalArgumentException | InvocationTargetException e) {
LOGGER.error("Could not transform data", e);
}
}
return transformed;
}
+ private static Object[] convertTypes(Object[] row, Class<?>[] paramTypes) {
+ Object[] result = new Object[row.length];
+ for (int i = 0; i < row.length; i++) {
+ result[i] = (i < paramTypes.length) ? convertValue(row[i],
paramTypes[i]) : row[i];
+ }
+ return result;
+ }
+
+ private static Object convertValue(Object value, Class<?> targetType) {
+ if (value == null || targetType.isInstance(value)) {
+ return value;
+ }
+ // Hibernate 7 returns java.time types instead of java.util.Date
+ if (targetType == Date.class) {
+ if (value instanceof OffsetDateTime) {
+ return Date.from(((OffsetDateTime) value).toInstant());
+ }
+ if (value instanceof Instant) {
+ return Date.from((Instant) value);
+ }
+ if (value instanceof LocalDateTime) {
+ return Date.from(((LocalDateTime)
value).atZone(ZoneId.systemDefault()).toInstant());
+ }
Review Comment:
PojoMapper converts LocalDateTime -> Date using ZoneId.systemDefault(),
which makes native-query temporal mapping depend on the host timezone and can
shift timestamps between environments. This should use a deterministic zone
(likely UTC, consistent with DateTimeUtil and the other mappers) when
converting LocalDateTime to an Instant.
##########
jobs-service/jobs-service-inmemory/src/main/resources/application.properties:
##########
@@ -20,6 +20,9 @@
#Flyway - It's safe to enable Flyway by default for in-memory storage
quarkus.flyway.migrate-at-start=true
quarkus.flyway.baseline-on-migrate=true
+# Disable Flyway validation to avoid conflicts with data-index migrations
applied
+# to the shared dev-services PostgreSQL container by earlier modules in the
build.
+quarkus.flyway.validate-on-migrate=false
Review Comment:
Disabling `quarkus.flyway.validate-on-migrate` in main
application.properties reduces protection against accidental schema drift at
runtime. If this is only needed to avoid CI/dev-services cross-module
conflicts, scope it to a specific profile (e.g. %test or %dev) rather than
applying it for all runs of jobs-service-inmemory.
##########
data-index/data-index-springboot/kogito-addons-springboot-data-index-persistence/kogito-addons-springboot-data-index-persistence-jpa-parent/integration-tests-process/src/main/resources/application.properties:
##########
@@ -26,3 +26,4 @@ kie.flyway.enabled=true
# Disabling Spring-Boot Flyway to avoid unnecessary Data Base initialization
spring.flyway.enabled=false
+
Review Comment:
Trailing blank line added here looks accidental and creates diff noise.
Consider removing it.
```suggestion
```
--
This is an automated message from the Apache Git Service.
To respond to the message, please log on to GitHub and use the
URL above to go to the specific comment.
To unsubscribe, e-mail: [email protected]
For queries about this service, please contact Infrastructure at:
[email protected]
---------------------------------------------------------------------
To unsubscribe, e-mail: [email protected]
For additional commands, e-mail: [email protected]
