pefernan commented on code in PR #1962:
URL:
https://github.com/apache/incubator-kie-kogito-apps/pull/1962#discussion_r1463044042
##########
data-audit/data-audit-common/src/main/java/org/kie/kogito/app/audit/api/SubsystemConstants.java:
##########
@@ -25,6 +25,8 @@ private SubsystemConstants() {
}
public static final String DATA_AUDIT_PATH = "/data-audit";
+ public static final String DATA_AUDIT_QUERY_PATH = "/data-audit/q";
Review Comment:
```suggestion
public static final String DATA_AUDIT_QUERY_PATH = DATA_AUDIT_PATH +
"/q";
```
##########
data-audit/data-audit-common/src/main/java/org/kie/kogito/app/audit/graphql/GraphQLSchemaManager.java:
##########
@@ -77,14 +95,18 @@ private GraphQLSchemaManager() {
ServiceLoader.load(GraphQLSchemaQueryProvider.class,
classLoader).forEach(queryProvider -> {
graphqlSchemas.addAll(List.of(queryProvider.graphQLQueryExtension()));
- for (GraphQLSchemaQuery<?> query : queryProvider.queries()) {
+ for (GraphQLSchemaQuery query :
queryProvider.queries(dataAuditContext)) {
runtimeWiringBuilder.type("Query", builder ->
builder.dataFetcher(query.name(), query::fetch));
}
});
+ List<InputStream> data = new ArrayList<>();
+
data.addAll(graphqlSchemas.stream().map(this::toInputStream).collect(Collectors.toList()));
+
data.addAll(this.graphQLdefinitions.values().stream().map(String::getBytes).map(ByteArrayInputStream::new).collect(Collectors.toList()));
Review Comment:
```suggestion
data.addAll(this.graphQLdefinitions.values().stream().map(String::getBytes).map(ByteArrayInputStream::new).toList());
```
##########
data-audit/data-audit-common/src/main/java/org/kie/kogito/app/audit/graphql/GraphQLSchemaManager.java:
##########
@@ -103,22 +125,52 @@ private GraphQLSchemaManager() {
SchemaGenerator schemaGenerator = new SchemaGenerator();
// we merge the query object
typeDefinitionRegistry.add(ObjectTypeDefinition.newObjectTypeDefinition().name("Query").fieldDefinitions(queryDefinitions).build());
- graphQLSchema =
schemaGenerator.makeExecutableSchema(typeDefinitionRegistry, runtimeWiring);
+ GraphQLSchema newGraphQLSchema =
schemaGenerator.makeExecutableSchema(typeDefinitionRegistry, runtimeWiring);
+ GraphQL newGraphQL = GraphQL.newGraphQL(newGraphQLSchema).build();
+ // we got to this point then everything is validated
+ this.graphQLSchema = newGraphQLSchema;
+ this.graphQL = newGraphQL;
+ LOGGER.debug("Succesfuly rebuilding graphQL definitions");
}
- private TypeDefinitionRegistry readDefintionRegistry(String graphQLSchema)
{
- try (InputStream is =
Thread.currentThread().getContextClassLoader().getResourceAsStream(graphQLSchema))
{
- SchemaParser schemaParser = new SchemaParser();
- return schemaParser.parse(is);
+ private InputStream toInputStream(String classpathFile) {
+ try (InputStream is =
Thread.currentThread().getContextClassLoader().getResourceAsStream(classpathFile))
{
+ return new ByteArrayInputStream(is.readAllBytes());
} catch (IOException e) {
LOGGER.error("could not find or process {}", graphQLSchema, e);
- return new TypeDefinitionRegistry();
+ return new ByteArrayInputStream(new byte[0]);
}
}
+ private TypeDefinitionRegistry readDefintionRegistry(InputStream
inputStream) {
Review Comment:
```suggestion
private TypeDefinitionRegistry readDefinitionRegistry(InputStream
inputStream) {
```
##########
data-audit/kogito-addons-data-audit-quarkus/src/main/java/org/kie/kogito/app/audit/quarkus/GraphQLJPADataAuditRouter.java:
##########
@@ -46,22 +53,50 @@ public class GraphQLJPADataAuditRouter {
@Inject
DataAuditContextFactory dataAuditContextFactory;
+ private DataAuditQueryService dataAuditQueryService;
+
+ private DataAuditStoreProxyService dataAuditStoreProxyService;
+
@PostConstruct
public void init() {
- graphQL =
GraphQL.newGraphQL(DataAuditQueryService.newAuditQuerySerice().getGraphQLSchema()).build();
- graphQLHandler = GraphQLHandler.create(graphQL, new
GraphQLHandlerOptions());
+
graphQLSchemaManagerInstance().rebuildDefinitions(dataAuditContextFactory.newDataAuditContext());
+ dataAuditQueryService = DataAuditQueryService.newAuditQuerySerice();
+ dataAuditStoreProxyService =
DataAuditStoreProxyService.newAuditStoreService();
+ graphQLHandler =
GraphQLHandler.create(dataAuditQueryService.getGraphQL(), new
GraphQLHandlerOptions());
}
- @Route(path = DATA_AUDIT_PATH, type = Route.HandlerType.BLOCKING, order =
2, methods = { GET })
+ @Route(path = DATA_AUDIT_QUERY_PATH, type = Route.HandlerType.BLOCKING,
order = 2, methods = { GET })
public void blockingGraphQLHandlerGet(RoutingContext rc) {
graphQLHandler.beforeExecute(this::beforeExecuteHTTP).handle(rc);
}
- @Route(path = DATA_AUDIT_PATH, type = Route.HandlerType.BLOCKING, order =
2, methods = { POST })
+ @Route(path = DATA_AUDIT_QUERY_PATH, type = Route.HandlerType.BLOCKING,
order = 2, methods = { POST })
public void blockingGraphQLHandlerPost(RoutingContext rc) {
graphQLHandler.beforeExecute(this::beforeExecuteHTTP).handle(rc);
}
+ @Route(path = DATA_AUDIT_REGISTRY_PATH, type = Route.HandlerType.BLOCKING,
order = 2, methods = { POST })
+ public void blockingRegistryHandlerPost(RoutingContext rc) {
+ try {
+ JsonObject jsonObject = rc.body().asJsonObject();
+ DataAuditQuery dataAuditQuery = new DataAuditQuery();
+ dataAuditQuery.setIdentifier(jsonObject.getString("identifier"));
+
dataAuditQuery.setGraphQLDefinition(jsonObject.getString("graphQLDefinition"));
+ dataAuditQuery.setQuery(jsonObject.getString("query"));
+
dataAuditStoreProxyService.storeQuery(dataAuditContextFactory.newDataAuditContext(),
dataAuditQuery);
+ graphQLHandler =
GraphQLHandler.create(dataAuditQueryService.getGraphQL(), new
GraphQLHandlerOptions());
+ rc.response().setStatusCode(200).end();
+ } catch (Exception e) {
+ rc.response().setStatusCode(400).end();
Review Comment:
I'd add some message explaining what failed here.
##########
data-audit/data-audit-common/src/main/java/org/kie/kogito/app/audit/api/SubsystemConstants.java:
##########
@@ -25,6 +25,8 @@ private SubsystemConstants() {
}
public static final String DATA_AUDIT_PATH = "/data-audit";
+ public static final String DATA_AUDIT_QUERY_PATH = "/data-audit/q";
+ public static final String DATA_AUDIT_REGISTRY_PATH = "/data-audit/r";
Review Comment:
```suggestion
public static final String DATA_AUDIT_REGISTRY_PATH = DATA_AUDIT_PATH +
"/r";
```
##########
data-audit/data-audit-common/src/main/java/org/kie/kogito/app/audit/graphql/GraphQLSchemaManager.java:
##########
@@ -77,14 +95,18 @@ private GraphQLSchemaManager() {
ServiceLoader.load(GraphQLSchemaQueryProvider.class,
classLoader).forEach(queryProvider -> {
graphqlSchemas.addAll(List.of(queryProvider.graphQLQueryExtension()));
- for (GraphQLSchemaQuery<?> query : queryProvider.queries()) {
+ for (GraphQLSchemaQuery query :
queryProvider.queries(dataAuditContext)) {
runtimeWiringBuilder.type("Query", builder ->
builder.dataFetcher(query.name(), query::fetch));
}
});
+ List<InputStream> data = new ArrayList<>();
+
data.addAll(graphqlSchemas.stream().map(this::toInputStream).collect(Collectors.toList()));
Review Comment:
```suggestion
data.addAll(graphqlSchemas.stream().map(this::toInputStream).toList());
```
##########
data-audit/kogito-addons-data-audit-jpa/kogito-addons-data-audit-jpa-common/src/main/resources/db/data-audit/postgresql/V1.1.0__Add
Query audit dynamic registering.sql:
##########
@@ -0,0 +1,2 @@
+create table Audit_Query (identifier varchar(255) not null,
graph_ql_definition varchar(255), query varchar(255)) primary key (identifier));
Review Comment:
I think that `varchar(255)` might be too small to store complex queries
(graphql or sql), I'm hitting this issue when trying to store SQL queries with
JOINS or column aliases, which seems a common use.
##########
data-audit/data-audit-common/src/main/java/org/kie/kogito/app/audit/api/DataAuditStoreProxyService.java:
##########
@@ -81,4 +82,10 @@ public static DataAuditStoreProxyService
newAuditStoreService() {
LOGGER.debug("Creating new Data Audit Store proxy service with {}",
service);
return new DataAuditStoreProxyService(service);
}
+
+ public void storeQuery(DataAuditContext newDataAuditContext,
DataAuditQuery dataAuditQuery) {
Review Comment:
We should improve this code a bit.
- We must check the format of the query before persisting into DDBB. Right
now it's possible to store a malformed query in DDBB but it will fail when
registering in `SchemaManager`.
- If the query has been successfully registered in the `SchemaManager` we
should unregister it if there's any error when storing in DDBB. I found this
while trying to register this query:
```
{
"identifier": "GetAllStates",
"graphQLDefinition": "type AllStates {eventId: String,
processInstanceId: String, processId: String, state: String, eventType: String,
eventDate: DateTime } type Query {GetAllStates(pagination:Pagination) :
[AllStates]}",
"query": "SELECT log.event_id as eventId, log.event_date as eventDate,
log.event_type as eventType, log.process_id as processId,
log.process_instance_id as processInstanceId, log.state as state FROM
Process_Instance_State_Log log group by log.event_id, log.event_type,
log.event_date, log.process_id, log.process_instance_id, log.state order by
processInstanceId, eventDate"
}
```
The `SchemaManager` is able to register the query but there's an
SQLException happening due to the column size, so the query is correctly
registered but not stored in DDBB.
--
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]
