Skip to content

Commit

Permalink
Split add and update schema labels apis
Browse files Browse the repository at this point in the history 
Signed-off-by: Andrea Lamparelli <[email protected]>
  • Loading branch information
@lampajr
lampajr committed Mar 7, 2025
1 parent a12d640 commit e5ff803
Show file tree
Hide file tree
Showing 7 changed files with 117 additions and 47 deletions.
35 changes: 32 additions & 3 deletions docs/site/content/en/openapi/openapi.yaml
View file
Original file line number Diff line number Diff line change
Expand Up @@ -1567,12 +1567,41 @@ paths:
type: array
items:
$ref: "#/components/schemas/Label"
put:
tags:
- Schema
description: Update existing Label for a Schema (Label id only required when
updating existing one)
operationId: updateLabel
parameters:
- name: schemaId
in: path
description: Schema ID
required: true
schema:
format: int32
type: integer
example: 101
requestBody:
content:
application/json:
schema:
$ref: "#/components/schemas/Label"
required: true
responses:
"200":
description: Schema updated successfully
content:
application/json:
schema:
format: int32
type: integer
post:
tags:
- Schema
description: Save new or update existing Label for a Schema (Label id only required
when updating existing one)
operationId: addOrUpdateLabel
operationId: addLabel
parameters:
- name: schemaId
in: path
Expand All @@ -1589,8 +1618,8 @@ paths:
$ref: "#/components/schemas/Label"
required: true
responses:
"200":
description: OK
"201":
description: New schema created successfully
content:
application/json:
schema:
Expand Down
18 changes: 17 additions & 1 deletion horreum-api/src/main/java/io/hyperfoil/tools/horreum/api/services/SchemaService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -175,10 +175,26 @@ int addOrUpdateTransformer(@PathParam("schemaId") int schemaId,
@Path("{schemaId}/labels")
@Consumes(MediaType.APPLICATION_JSON)
@Operation(description = "Save new or update existing Label for a Schema (Label id only required when updating existing one)")
@ResponseStatus(201)
@Parameters(value = {
@Parameter(name = "schemaId", description = "Schema ID", example = "101"),
})
@APIResponses(value = {
@APIResponse(responseCode = "201", description = "New schema created successfully", content = @Content(mediaType = MediaType.APPLICATION_JSON, schema = @org.eclipse.microprofile.openapi.annotations.media.Schema(implementation = Integer.class)))
})
Integer addLabel(@PathParam("schemaId") int schemaId, @RequestBody(required = true) Label label);

@PUT
@Path("{schemaId}/labels")
@Consumes(MediaType.APPLICATION_JSON)
@Operation(description = "Update existing Label for a Schema (Label id only required when updating existing one)")
@Parameters(value = {
@Parameter(name = "schemaId", description = "Schema ID", example = "101"),
})
Integer addOrUpdateLabel(@PathParam("schemaId") int schemaId, @RequestBody(required = true) Label label);
@APIResponses(value = {
@APIResponse(responseCode = "200", description = "Schema updated successfully", content = @Content(mediaType = MediaType.APPLICATION_JSON, schema = @org.eclipse.microprofile.openapi.annotations.media.Schema(implementation = Integer.class)))
})
Integer updateLabel(@PathParam("schemaId") int schemaId, @RequestBody(required = true) Label label);

@DELETE
@Path("{schemaId}/labels/{labelId}")
Expand Down
69 changes: 47 additions & 22 deletions horreum-backend/src/main/java/io/hyperfoil/tools/horreum/svc/SchemaServiceImpl.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -62,6 +62,7 @@
import io.hyperfoil.tools.horreum.entity.ValidationErrorDAO;
import io.hyperfoil.tools.horreum.entity.data.DatasetDAO;
import io.hyperfoil.tools.horreum.entity.data.LabelDAO;
import io.hyperfoil.tools.horreum.entity.data.LabelValueDAO;
import io.hyperfoil.tools.horreum.entity.data.RunDAO;
import io.hyperfoil.tools.horreum.entity.data.SchemaDAO;
import io.hyperfoil.tools.horreum.entity.data.TransformerDAO;
Expand Down Expand Up @@ -710,33 +711,43 @@ public List<Label> labels(int schemaId) {
@WithRoles
@Transactional
@Override
public Integer addOrUpdateLabel(int schemaId, Label labelDTO) {
public Integer addLabel(int schemaId, Label labelDTO) {
if (labelDTO == null) {
throw ServiceException.badRequest("No label?");
}

labelDTO.schemaId = schemaId;
return addOrUpdateLabel(labelDTO);
}

@WithRoles
@Transactional
@Override
public Integer updateLabel(int schemaId, Label labelDTO) {
if (labelDTO == null) {
throw ServiceException.badRequest("No label?");
}

labelDTO.schemaId = schemaId;
return addOrUpdateLabel(labelDTO);
}

private int addOrUpdateLabel(Label labelDTO) {
if (!identity.hasRole(labelDTO.owner)) {
throw ServiceException.forbidden("This user is not a member of team " + labelDTO.owner);
}
if (labelDTO.name == null || labelDTO.name.isBlank()) {
throw ServiceException.badRequest("Label must have a non-blank name");
}

// some validation logic
validateLabel(labelDTO);
validateExtractors(labelDTO.extractors);

LabelDAO label = LabelMapper.to(labelDTO);
if (label.id == null || label.id < 0) {
label.id = null;

label.schema = (SchemaDAO) SchemaDAO.findByIdOptional(schemaId)
.orElseThrow(() -> ServiceException.notFound("Schema " + schemaId + " not found"));

checkSameName(label);
label.persistAndFlush();
emitLabelChanged(label.id, schemaId);
} else {
if (label.id != null && label.id > 0) {
// update existing label
LabelDAO existing = (LabelDAO) LabelDAO.findByIdOptional(label.id)
.orElseThrow(() -> ServiceException.notFound("Label " + label.id + " not found"));

if (!Objects.equals(existing.schema.id, schemaId)) {
if (!Objects.equals(existing.schema.id, labelDTO.schemaId)) {
throw ServiceException.badRequest("Label id=" + label.id + ", name=" + existing.name +
" belongs to a different schema: " + existing.schema.id + "(" + existing.schema.uri + ")");
}
Expand All @@ -745,30 +756,44 @@ public Integer addOrUpdateLabel(int schemaId, Label labelDTO) {
.forbidden("Cannot transfer ownership: this user is not a member of team " + existing.owner);
}
if (!existing.name.equals(label.name)) {
// if we are changing the name checks if it conflicts with others
checkSameName(label);
}
existing.name = label.name;

// when we clear extractors we should also delete label_values
// when we clear extractors we should also delete label_values and dataset views
em.createNativeQuery(
"DELETE FROM dataset_view WHERE dataset_id IN (SELECT dataset_id FROM label_values WHERE label_id = ?1)")
.setParameter(1, existing.id).executeUpdate();
em.createNativeQuery("DELETE FROM label_values WHERE label_id = ?1").setParameter(1, existing.id).executeUpdate();
LabelValueDAO.delete("labelId = ?1", existing.id);

existing.name = label.name;
existing.extractors.clear();
existing.extractors.addAll(label.extractors);

existing.function = label.function;
existing.owner = label.owner;
existing.access = label.access;
existing.filtering = label.filtering;
existing.metrics = label.metrics;
existing.persistAndFlush();

emitLabelChanged(existing.id, existing.getSchemaId());
} else {
// create new label
// check whether the schema is existing, otherwise throw error
label.schema = (SchemaDAO) SchemaDAO.findByIdOptional(labelDTO.schemaId)
.orElseThrow(() -> ServiceException.notFound("Schema " + labelDTO.schemaId + " not found"));
// check the name is a valid one and does not collide with others
checkSameName(label);
label.persistAndFlush();
}

emitLabelChanged(label.id, label.getSchemaId());
return label.id;
}

private void validateLabel(Label labelDTO) {
if (labelDTO.name == null || labelDTO.name.isBlank()) {
throw ServiceException.badRequest("Label must have a non-blank name");
}
}

private void emitLabelChanged(int labelId, int schemaId) {
String datasetIdQuery = """
SELECT ds.id, ds.testId
Expand Down
2 changes: 1 addition & 1 deletion horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/BaseServiceTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -592,7 +592,7 @@ protected int postLabel(Schema schema, String name, String function, Consumer<La
mutate.accept(l);
}
Response response = jsonRequest().body(l).post("/api/schema/" + schema.id + "/labels");
response.then().statusCode(200);
response.then().statusCode(201);
return Integer.parseInt(response.body().asString());
}

Expand Down
30 changes: 15 additions & 15 deletions horreum-backend/src/test/java/io/hyperfoil/tools/horreum/svc/SchemaServiceNoRestTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -467,7 +467,7 @@ void testCreateSchemaLabel() {
Label l = createSampleLabel("Blabla", s, null, "({x, y}) => ({ z: 1 })",
new Extractor("x", "$.x", true), new Extractor("y", "$.y", false));

int id = schemaService.addOrUpdateLabel(s.id, l);
int id = schemaService.addLabel(s.id, l);
assertEquals(1, LabelDAO.count());
LabelDAO label = LabelDAO.findById(id);
assertNotNull(label);
Expand All @@ -483,13 +483,13 @@ void testCreateSchemaLabelWithBlankName() {
new Extractor("x", "$.x", true), new Extractor("y", "$.y", false));

// empty name
ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addOrUpdateLabel(s.id, l));
ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addLabel(s.id, l));
assertEquals("Label must have a non-blank name", thrown.getMessage());
assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), thrown.getResponse().getStatus());

// null name
l.name = null;
thrown = assertThrows(ServiceException.class, () -> schemaService.addOrUpdateLabel(s.id, l));
thrown = assertThrows(ServiceException.class, () -> schemaService.addLabel(s.id, l));
assertEquals("Label must have a non-blank name", thrown.getMessage());
assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), thrown.getResponse().getStatus());
}
Expand All @@ -502,7 +502,7 @@ void testCreateSchemaLabelWithNotExistingId() {
new Extractor("x", "$.x", true), new Extractor("y", "$.y", false));
l.id = 999;

ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addOrUpdateLabel(s.id, l));
ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addLabel(s.id, l));
assertEquals(String.format("Label %d not found", l.id), thrown.getMessage());
assertEquals(Response.Status.NOT_FOUND.getStatusCode(), thrown.getResponse().getStatus());
}
Expand All @@ -515,12 +515,12 @@ void testCreateSchemaLabelWithFailures() {
new Extractor("x", "$.x", true), new Extractor("y", "$.y", false));

// null label dto
ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addOrUpdateLabel(s.id, null));
ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addLabel(s.id, null));
assertEquals("No label?", thrown.getMessage());
assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), thrown.getResponse().getStatus());

// not existing schema
thrown = assertThrows(ServiceException.class, () -> schemaService.addOrUpdateLabel(999, l));
thrown = assertThrows(ServiceException.class, () -> schemaService.addLabel(999, l));
assertEquals("Schema 999 not found", thrown.getMessage());
assertEquals(Response.Status.NOT_FOUND.getStatusCode(), thrown.getResponse().getStatus());
}
Expand All @@ -532,14 +532,14 @@ void testUpdateSchemaLabel() {
Label l = createSampleLabel("Blabla", s, null, "({x, y}) => ({ z: 1 })",
new Extractor("x", "$.x", true), new Extractor("y", "$.y", false));

int id = schemaService.addOrUpdateLabel(s.id, l);
int id = schemaService.addLabel(s.id, l);
assertEquals(1, LabelDAO.count());

// update the label
l.id = id;
l.name = "AnotherName";
l.extractors.add(new Extractor("z", "$.z", false));
int afterUpdateId = schemaService.addOrUpdateLabel(s.id, l);
int afterUpdateId = schemaService.addLabel(s.id, l);
assertEquals(id, afterUpdateId);

assertEquals(1, LabelDAO.count());
Expand All @@ -556,12 +556,12 @@ void testUpdateSchemaLabelWrongSchemaId() {
Label l = createSampleLabel("Blabla", s, null, "({x, y}) => ({ z: 1 })",
new Extractor("x", "$.x", true), new Extractor("y", "$.y", false));

int id = schemaService.addOrUpdateLabel(s.id, l);
int id = schemaService.addLabel(s.id, l);
assertEquals(1, LabelDAO.count());

// update the label passing the wrong schema id
l.id = id;
ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addOrUpdateLabel(999, l));
ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addLabel(999, l));
assertEquals(String.format("Label id=%d, name=%s belongs to a different schema: %d(%s)",
l.id, l.name, s.id, s.uri), thrown.getMessage());
assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), thrown.getResponse().getStatus());
Expand All @@ -575,14 +575,14 @@ void testUpdateSchemaLabelNameWhenAlreadyExisting() {
new Extractor("x", "$.x", true), new Extractor("y", "$.y", false));
Label another = createSampleLabel("AnotherLabel", s, null, "({x, y}) => ({ z: 1 })");

int id = schemaService.addOrUpdateLabel(s.id, l);
schemaService.addOrUpdateLabel(s.id, another);
int id = schemaService.addLabel(s.id, l);
schemaService.addLabel(s.id, another);
assertEquals(2, LabelDAO.count());

// update the label
l.id = id;
l.name = "AnotherLabel";
ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addOrUpdateLabel(s.id, l));
ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.addLabel(s.id, l));
assertEquals(String.format("There is an existing label with the same name (%s) in this " +
"schema; please choose different name.", l.name), thrown.getMessage());
assertEquals(Response.Status.BAD_REQUEST.getStatusCode(), thrown.getResponse().getStatus());
Expand All @@ -595,7 +595,7 @@ void testDeleteSchemaLabel() {
Label l = createSampleLabel("Blabla", s, null, "({x, y}) => ({ z: 1 })",
new Extractor("x", "$.x", true), new Extractor("y", "$.y", false));

int id = schemaService.addOrUpdateLabel(s.id, l);
int id = schemaService.addLabel(s.id, l);
assertEquals(1, LabelDAO.count());

schemaService.deleteLabel(s.id, id);
Expand All @@ -609,7 +609,7 @@ void testDeleteSchemaLabelWithFailures() {
Label l = createSampleLabel("Blabla", s, null, "({x, y}) => ({ z: 1 })",
new Extractor("x", "$.x", true), new Extractor("y", "$.y", false));

int id = schemaService.addOrUpdateLabel(s.id, l);
int id = schemaService.addLabel(s.id, l);
assertEquals(1, LabelDAO.count());

ServiceException thrown = assertThrows(ServiceException.class, () -> schemaService.deleteLabel(s.id, 999));
Expand Down
8 changes: 4 additions & 4 deletions horreum-integration-tests/src/test/java/io/hyperfoil/tools/horreum/it/HorreumClientIT.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -278,7 +278,7 @@ public void runExperiment() {
lblCpu.owner = dummyTest.owner;
lblCpu.metrics = true;
lblCpu.filtering = false;
lblCpu.id = horreumClient.schemaService.addOrUpdateLabel(schema.id, lblCpu);
lblCpu.id = horreumClient.schemaService.addLabel(schema.id, lblCpu);

Label lblThroughput = new Label();
lblThroughput.name = "throughput";
Expand All @@ -288,7 +288,7 @@ public void runExperiment() {
lblThroughput.owner = dummyTest.owner;
lblThroughput.metrics = true;
lblThroughput.filtering = false;
lblThroughput.id = horreumClient.schemaService.addOrUpdateLabel(schema.id, lblThroughput);
lblThroughput.id = horreumClient.schemaService.addLabel(schema.id, lblThroughput);

Label lblJob = new Label();
lblJob.name = "job";
Expand All @@ -298,7 +298,7 @@ public void runExperiment() {
lblJob.owner = dummyTest.owner;
lblJob.metrics = false;
lblJob.filtering = true;
lblJob.id = horreumClient.schemaService.addOrUpdateLabel(schema.id, lblJob);
lblJob.id = horreumClient.schemaService.addLabel(schema.id, lblJob);

Label lblBuildID = new Label();
lblBuildID.name = "build-id";
Expand All @@ -308,7 +308,7 @@ public void runExperiment() {
lblBuildID.owner = dummyTest.owner;
lblBuildID.metrics = false;
lblBuildID.filtering = true;
lblBuildID.id = horreumClient.schemaService.addOrUpdateLabel(schema.id, lblBuildID);
lblBuildID.id = horreumClient.schemaService.addLabel(schema.id, lblBuildID);

//3. Config change detection variables
Variable variable = new Variable();
Expand Down
2 changes: 1 addition & 1 deletion horreum-web/src/domain/schemas/Labels.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -69,7 +69,7 @@ export default function Labels({ schemaId, schemaUri, funcsRef }: LabelsProps) {
...labels
.filter(l => l.modified)
.map(l =>
schemaApi.addOrUpdateLabel(l.schemaId, l).then(id => {
schemaApi.addLabel(l.schemaId, l).then(id => {
l.id = id
l.modified = false
})
Expand Down

0 comments on commit e5ff803

Please sign in to comment.