Split add and update schema apis

Signed-off-by: Andrea Lamparelli <[email protected]>

docs/site/content/en/openapi/openapi.yaml

@@ -1283,6 +1283,25 @@ paths:
             application/json:
               schema:
                 $ref: "#/components/schemas/SchemaQueryResult"
+    put:
+      tags:
+      - Schema
+      description: Update an existing Schema
+      operationId: update
+      requestBody:
+        content:
+          application/json:
+            schema:
+              $ref: "#/components/schemas/Schema"
+      responses:
+        "200":
+          description: Schema updated successfully
+          content:
+            application/json:
+              schema:
+                format: int32
+                type: integer
+              example: 103
     post:
       tags:
       - Schema
@@ -1294,8 +1313,8 @@ paths:
             schema:
               $ref: "#/components/schemas/Schema"
       responses:
-        "200":
-          description: Import a new Schema
+        "201":
+          description: New schema created successfully
           content:
             application/json:
               schema:

horreum-api/src/main/java/io/hyperfoil/tools/horreum/api/data/Schema.java

@@ -66,4 +66,8 @@ public String toString() {
                     '}';
         }
     }
+
+    public void clearIds() {
+        id = null;
+    }
 }

horreum-api/src/main/java/io/hyperfoil/tools/horreum/api/services/SchemaService.java

@@ -9,6 +9,7 @@
 import jakarta.ws.rs.DELETE;
 import jakarta.ws.rs.GET;
 import jakarta.ws.rs.POST;
+import jakarta.ws.rs.PUT;
 import jakarta.ws.rs.Path;
 import jakarta.ws.rs.PathParam;
 import jakarta.ws.rs.Produces;
@@ -26,6 +27,7 @@
 import org.eclipse.microprofile.openapi.annotations.responses.APIResponseSchema;
 import org.eclipse.microprofile.openapi.annotations.responses.APIResponses;
 import org.eclipse.microprofile.openapi.annotations.tags.Tag;
+import org.jboss.resteasy.reactive.ResponseStatus;
 
 import com.fasterxml.jackson.annotation.JsonProperty;
 import com.fasterxml.jackson.databind.node.ObjectNode;
@@ -67,12 +69,16 @@ public interface SchemaService {
     int idByUri(@PathParam("uri") String uri);
 
     @POST
-    @Consumes(MediaType.APPLICATION_JSON)
-    @Produces(MediaType.APPLICATION_JSON)
     @Operation(description = "Save a new Schema")
-    @APIResponse(responseCode = "200", description = "Import a new Schema", content = @Content(schema = @org.eclipse.microprofile.openapi.annotations.media.Schema(type = SchemaType.INTEGER, implementation = Integer.class), example = "103"))
+    @ResponseStatus(201)
+    @APIResponse(responseCode = "201", description = "New schema created successfully", content = @Content(schema = @org.eclipse.microprofile.openapi.annotations.media.Schema(type = SchemaType.INTEGER, implementation = Integer.class), example = "103"))
     Integer add(Schema schema);
 
+    @PUT
+    @Operation(description = "Update an existing Schema")
+    @APIResponse(responseCode = "200", description = "Schema updated successfully", content = @Content(schema = @org.eclipse.microprofile.openapi.annotations.media.Schema(type = SchemaType.INTEGER, implementation = Integer.class), example = "103"))
+    Integer update(Schema schema);
+
     @GET
     @Operation(description = "Retrieve a paginated list of Schemas with available count")
     @Parameters(value = {

horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/SchemaServiceImpl.java

@@ -3,7 +3,17 @@
 import java.io.ByteArrayInputStream;
 import java.io.InputStream;
 import java.nio.charset.StandardCharsets;
-import java.util.*;
+import java.util.ArrayList;
+import java.util.Arrays;
+import java.util.Collection;
+import java.util.Collections;
+import java.util.HashMap;
+import java.util.HashSet;
+import java.util.List;
+import java.util.Map;
+import java.util.Objects;
+import java.util.Set;
+import java.util.TreeMap;
 import java.util.function.Function;
 import java.util.function.Predicate;
 import java.util.stream.Collectors;
@@ -146,29 +156,65 @@ public int idByUri(String uri) {
     @Transactional
     @Override
     public Integer add(Schema schemaDTO) {
-        verifyNewSchema(schemaDTO);
+        // we are creating a new schema, ensure all ids are cleaned up
+        schemaDTO.clearIds();
+
+        log.debugf("Creating new schema: %s", schemaDTO.toString());
+        return addOrUpdateSchema(schemaDTO);
+    }
+
+    @RolesAllowed(Roles.TESTER)
+    @WithRoles
+    @Transactional
+    @Override
+    public Integer update(Schema schemaDTO) {
+        // check the schema already exists in the db
+        if (schemaDTO.id == null || SchemaDAO.findById(schemaDTO.id) == null) {
+            throw ServiceException.notFound("Missing schema id or schema with id " + schemaDTO.id + " does not exist");
+        }
+
+        log.debugf("Updating schema (%d): %s", schemaDTO.id, schemaDTO.toString());
+        return addOrUpdateSchema(schemaDTO);
+    }
+
+    /**
+     * Create or update a Schema entity by performing the appropriate validation logics
+     * @param schemaDTO the DTO that should be mapped to the database
+     * @return the db Schema id
+     */
+    private int addOrUpdateSchema(Schema schemaDTO) {
+        // whether we should trigger a schema synchronization
+        // this happens when creating new schemas or, updating uri or JSON schema
+        boolean syncSchemas = false;
+
+        validateSchema(schemaDTO);
+
         // Note: isEmpty is true for all non-object and non-array nodes
         if (schemaDTO.schema != null && schemaDTO.schema.isEmpty()) {
             schemaDTO.schema = null;
         }
+
         SchemaDAO schema = SchemaMapper.to(schemaDTO);
         if (schemaDTO.id != null && schemaDTO.id > 0) {
-            SchemaDAO existing = SchemaDAO.findById(schema.id);
-            if (existing == null)
-                throw ServiceException.badRequest("An id was given, but it does not exist.");
+            // update existing Schema
+            SchemaDAO existing = (SchemaDAO) SchemaDAO.findByIdOptional(schema.id)
+                    .orElseThrow(() -> ServiceException.badRequest("Cannot find schema with id " + schemaDTO.id));
             em.merge(schema);
             em.flush();
-            if (!Objects.equals(schema.uri, existing.uri) ||
-                    Objects.equals(schema.schema, existing.schema)) {
-                newOrUpdatedSchema(schema);
+            if (!Objects.equals(schema.uri, existing.uri) || Objects.equals(schema.schema, existing.schema)) {
+                syncSchemas = true;
             }
         } else {
-            schema.id = null;
+            // persist new Schema
+            syncSchemas = true;
             schema.persist();
             em.flush();
+        }
+
+        if (syncSchemas) {
             newOrUpdatedSchema(schema);
         }
-        log.debugf("Added schema %s (%d), URI %s", schema.name, schema.id, schema.uri);
+
         return schema.id;
     }
 
@@ -177,7 +223,7 @@ private void newOrUpdatedSchema(SchemaDAO schema) {
         Util.registerTxSynchronization(tm, txStatus -> mediator.queueSchemaSync(schema.id));
     }
 
-    private void verifyNewSchema(Schema schemaDTO) {
+    private void validateSchema(Schema schemaDTO) {
         if (schemaDTO.uri == null || Arrays.stream(ALL_URNS).noneMatch(scheme -> schemaDTO.uri.startsWith(scheme + ":"))) {
             throw ServiceException
                     .badRequest("Please use URI starting with one of these schemes: " + Arrays.toString(ALL_URNS));
@@ -866,13 +912,13 @@ public void importSchema(ObjectNode node) {
                 em.merge(SchemaMapper.to(importSchema));
                 newSchema = false;
             } else {
-                verifyNewSchema(importSchema);
+                validateSchema(importSchema);
                 schema = SchemaMapper.to(importSchema);
                 schema.id = null;
                 schema.persist();
             }
         } else {
-            verifyNewSchema(importSchema);
+            validateSchema(importSchema);
             schema = SchemaMapper.to(importSchema);
             em.persist(schema);
         }

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/BaseServiceTest.java

@@ -505,9 +505,8 @@ protected Schema createExampleSchema(String name, String className, String displ
         schema.access = Access.PUBLIC;
         schema.name = name + "." + displayName;
         schema.uri = "urn:" + className + ":" + displayName + ":1.0";
-        Integer id = jsonRequest().body(schema).post("/api/schema").then()
-                .statusCode(200).extract().as(Integer.class);
-        schema.id = id;
+        schema.id = jsonRequest().body(schema).post("/api/schema").then()
+                .statusCode(201).extract().as(Integer.class);
 
         if (label) {
             addLabel(schema, "value", null, new Extractor("value", "$.value", false));
@@ -526,7 +525,7 @@ protected Schema createSchema(String name, String uri, JsonNode jsonSchema) {
         schema.name = name;
         schema.uri = uri;
         schema.schema = jsonSchema;
-        return addOrUpdateSchema(schema);
+        return addSchema(schema);
     }
 
     protected Schema getSchema(int id, String token) {
@@ -538,8 +537,15 @@ protected Schema getSchema(int id, String token) {
                 .extract().as(Schema.class);
     }
 
-    protected Schema addOrUpdateSchema(Schema schema) {
+    protected Schema addSchema(Schema schema) {
         Response response = jsonRequest().body(schema).post("/api/schema");
+        response.then().statusCode(201);
+        schema.id = Integer.parseInt(response.body().asString());
+        return schema;
+    }
+
+    protected Schema updateSchema(Schema schema) {
+        Response response = jsonRequest().body(schema).put("/api/schema");
         response.then().statusCode(200);
         schema.id = Integer.parseInt(response.body().asString());
         return schema;
@@ -951,7 +957,7 @@ protected void populateDataFromFiles() throws IOException {
                 readFile(p.resolve("acme_benchmark_schema.json").toFile()), Schema.class);
         assertEquals("dev-team", s.owner);
         s.owner = "foo-team";
-        s = addOrUpdateSchema(s);
+        s = addSchema(s);
 
         Label l = new ObjectMapper().readValue(
                 readFile(p.resolve("throughput_label.json").toFile()), Label.class);

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/DatasetServiceTest.java

@@ -465,7 +465,7 @@ public void testEverythingPrivate() throws InterruptedException {
         schema.access = Access.PRIVATE;
         schema.name = "private-schema";
         schema.uri = "urn:private";
-        addOrUpdateSchema(schema);
+        addSchema(schema);
         postLabel(schema, "value", null, l -> l.access = Access.PRIVATE, new Extractor("value", "$.value", false));
 
         Test test = createExampleTest("private-test");

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/RunServiceTest.java

@@ -1001,7 +1001,7 @@ public void testJavascriptExecution() throws InterruptedException {
         schema.name = "Dummy";
         schema.owner = test.owner;
         schema.access = Access.PUBLIC;
-        schema = addOrUpdateSchema(schema);
+        schema = addSchema(schema);
 
         long now = System.currentTimeMillis();
         String ts = String.valueOf(now);
@@ -1046,7 +1046,7 @@ public void runExperiment() throws InterruptedException {
             schema.name = "test";
             schema.owner = test.owner;
             schema.access = Access.PUBLIC;
-            schema = addOrUpdateSchema(schema);
+            schema = addSchema(schema);
 
             //2. Define schema labels
             Label lblCpu = new Label();

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/SchemaServiceNoRestTest.java

@@ -115,7 +115,7 @@ void testUpdateSchema() {
         schema.id = id;
         schema.name = "urn:dummy:schema:v1";
 
-        int afterUpdateId = schemaService.add(schema);
+        int afterUpdateId = schemaService.update(schema);
         assertEquals(id, afterUpdateId);
 
         SchemaDAO savedSchema = SchemaDAO.findById(id);
@@ -132,9 +132,9 @@ void testUpdateNotFoundSchema() {
         schema.id = 9999;
 
         // try to update a not existing schema
-        ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.add(schema));
-        assertEquals("An id was given, but it does not exist.", thrown.getMessage());
-        assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), thrown.getResponse().getStatus());
+        ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.update(schema));
+        assertEquals("Missing schema id or schema with id 9999 does not exist", thrown.getMessage());
+        assertEquals(Response.Status.NOT_FOUND.getStatusCode(), thrown.getResponse().getStatus());
     }
 
     @org.junit.jupiter.api.Test

horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/SchemaServiceTest.java

@@ -107,9 +107,10 @@ void testCreateSchemaWithInvalidId() {
         schema.uri = "urn:invalid-id:schema";
         schema.id = 9999; // does not exist
 
+        // expect 201 as the id is cleared up when creating new schema
         jsonRequest().body(schema).post("/api/schema")
                 .then()
-                .statusCode(400);
+                .statusCode(201);
     }
 
     @org.junit.jupiter.api.Test
@@ -232,7 +233,7 @@ void testValidateRun() throws IOException, InterruptedException {
 
         allowAnySchema.schema = allowNone.deepCopy();
         ((ObjectNode) allowAnySchema.schema).set("$id", allowAny.path("$id").deepCopy());
-        addOrUpdateSchema(allowAnySchema);
+        updateSchema(allowAnySchema);
 
         Schema.ValidationEvent runValidation2 = runValidations.poll(10, TimeUnit.SECONDS);
         assertNotNull(runValidation2);
@@ -269,7 +270,7 @@ void testEditSchema() {
 
         schema.name = "Different name";
         schema.description = "Bla bla";
-        schema = addOrUpdateSchema(schema);
+        schema = updateSchema(schema);
         checkEntities(labelId, transformerId);
         //check labels and transformers using the rest interface as well
         labels = jsonRequest().get("/api/schema/" + schema.id + "/labels")
@@ -283,13 +284,13 @@ void testEditSchema() {
 
         schema.uri = "http://example.com/otherschema";
         schema.description = null;
-        addOrUpdateSchema(schema);
+        updateSchema(schema);
         checkEntities(labelId, transformerId);
 
         // back to original
         schema.name = "My schema";
         schema.uri = "urn:my:schema";
-        schema = addOrUpdateSchema(schema);
+        schema = updateSchema(schema);
         assertEquals("urn:my:schema", schema.uri);
         checkEntities(labelId, transformerId);
 
@@ -659,7 +660,7 @@ void testChangeUriForReferencedSchema() {
 
         // update the schema uri afterward
         schema.uri = "urn:new-dummy:schema";
-        Schema updatedSchema = addOrUpdateSchema(schema);
+        Schema updatedSchema = updateSchema(schema);
         assertNotNull(updatedSchema);
         assertEquals(schema.id, updatedSchema.id);
 

horreum-web/src/domain/schemas/Schema.tsx

@@ -1,5 +1,5 @@
 import {useEffect, useState, useRef, useContext} from "react"
-import { useParams } from "react-router-dom"
+import { useNavigate, useParams } from "react-router-dom"
 import { useSelector } from "react-redux"
 
 import {
@@ -43,9 +43,6 @@ import {AppContextType} from "../../context/@types/appContextTypes";
 import { TimeoutBanner, TimeoutBannerProps } from "../../Banner"
 import ExportButton from "../../components/ExportButton";
 
-type SchemaParams = {
-    schemaId: string
-}
 
 type GeneralProps = {
     schema: SchemaDef | undefined
@@ -205,6 +202,7 @@ function General(props: GeneralProps) {
 }
 
 export default function Schema() {
+    const navigate = useNavigate()
     const { alerting } = useContext(AppContext) as AppContextType;
     const { schemaId } = useParams<any>()
     const [schemaIdVal, setSchemaIdVal] = useState(schemaId === "_new" ? -1 : Number.parseInt(schemaId ?? "-1"))
@@ -224,11 +222,13 @@ export default function Schema() {
             setLoading(true)
             getSchema(schemaIdVal, alerting)
                 .then(setSchema)
+                .then(() => navigate("/schema/" + schemaIdVal, {replace: false}))
                 .finally(() => setLoading(false))
         } else {
             setLoading(false)
         }
     }, [schemaIdVal])
+
     useEffect(() => {
         document.title = (schemaIdVal < 0 ? "New schema" : schema?.name || "(unknown schema)") + " | Horreum"
         setModifiedSchema(schema)
@@ -258,7 +258,7 @@ export default function Schema() {
             schema: editorSchema ? JSON.parse(editorSchema) : null,
         } as SchemaDef
 
-        return schemaApi.add(newSchema)
+        return (newSchema.id > 0 ? schemaApi.update(newSchema) : schemaApi.add(newSchema))
             .then(id=>  id,
                 error => alerting.dispatchError(error, "SAVE_SCHEMA", "Failed to save schema")
             ).then(id => {