Skip to content

Commit

Permalink
fix(core): labels as map failed on json endpoint (#2049)
Browse files Browse the repository at this point in the history
close #2040
close #2037
  • Loading branch information
loicmathieu authored Sep 8, 2023
1 parent 89f7827 commit b2fcf42
Show file tree
Hide file tree
Showing 7 changed files with 47 additions and 36 deletions.
30 changes: 7 additions & 23 deletions core/src/main/java/io/kestra/core/models/flows/Flow.java
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import io.kestra.core.exceptions.InternalException;
import io.kestra.core.models.DeletedInterface;
import io.kestra.core.models.Label;
import io.kestra.core.models.annotations.PluginProperty;
import io.kestra.core.models.executions.Execution;
import io.kestra.core.models.listeners.Listener;
import io.kestra.core.models.tasks.FlowableTask;
Expand All @@ -29,17 +28,16 @@
import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;
import javax.validation.ConstraintViolation;
import javax.validation.ConstraintViolationException;
import javax.validation.Valid;
import javax.validation.constraints.*;
import java.util.*;
import java.util.stream.Collectors;
import java.util.stream.Stream;

@SuperBuilder(toBuilder = true)
@Getter
@AllArgsConstructor
@NoArgsConstructor
@Introspected
@ToString
Expand All @@ -64,7 +62,6 @@ public boolean hasIgnoreMarker(final AnnotatedMember m) {
@Pattern(regexp = "[a-z0-9._-]+")
String namespace;

@With
@Min(value = 1)
Integer revision;

Expand All @@ -75,7 +72,6 @@ public boolean hasIgnoreMarker(final AnnotatedMember m) {
@Schema(implementation = Object.class, anyOf = {List.class, Map.class})
List<Label> labels;


@Valid
List<Input<?>> inputs;

Expand Down Expand Up @@ -322,21 +318,9 @@ public String generateSource() {
}

public Flow toDeleted() {
return new Flow(
this.id,
this.namespace,
this.revision + 1,
this.description,
this.labels,
this.inputs,
this.variables,
this.tasks,
this.errors,
this.listeners,
this.triggers,
this.taskDefaults,
this.disabled,
true
);
return this.toBuilder()
.revision(this.revision + 1)
.deleted(true)
.build();
}
}
Original file line number Diff line number Diff line change
@@ -1,12 +1,14 @@
package io.kestra.core.models.flows;

import io.micronaut.core.annotation.Introspected;
import lombok.*;
import lombok.EqualsAndHashCode;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.ToString;
import lombok.experimental.SuperBuilder;

@SuperBuilder(toBuilder = true)
@Getter
@AllArgsConstructor
@NoArgsConstructor
@Introspected
@ToString
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,20 +2,16 @@

import io.kestra.core.services.FlowService;
import io.micronaut.core.annotation.Introspected;
import lombok.AllArgsConstructor;
import lombok.Getter;
import lombok.NoArgsConstructor;
import lombok.ToString;
import lombok.experimental.SuperBuilder;
import lombok.extern.slf4j.Slf4j;

@SuperBuilder(toBuilder = true)
@Getter
@AllArgsConstructor
@NoArgsConstructor
@Introspected
@ToString
@Slf4j
public class FlowWithSource extends Flow {
String source;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -173,14 +173,14 @@ public void multipleCondition() {

@Test
public void self1() {
Flow flow = parse("flows/valids/trigger-multiplecondition-listener.yaml").withRevision(1);
Flow flow = parse("flows/valids/trigger-multiplecondition-listener.yaml").toBuilder().revision(1).build();

assertThat(flowTopologyService.isChild(flow, flow), nullValue());
}

@Test
public void self() {
Flow flow = parse("flows/valids/trigger-flow-listener.yaml").withRevision(1);
Flow flow = parse("flows/valids/trigger-flow-listener.yaml").toBuilder().revision(1).build();

assertThat(flowTopologyService.isChild(flow, flow), nullValue());
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -348,6 +348,10 @@ public FlowWithSource update(Flow flow, Flow previous, String flowSource, Flow f

@SneakyThrows
private FlowWithSource save(Flow flow, CrudEventType crudEventType, String flowSource) throws ConstraintViolationException {
if (flow instanceof FlowWithSource) {
flow = ((FlowWithSource) flow).toFlow();
}

// flow exists, return it
Optional<FlowWithSource> exists = this.findByIdWithSource(flow.getNamespace(), flow.getId());
if (exists.isPresent() && exists.get().isUpdatable(flow, flowSource)) {
Expand All @@ -357,9 +361,9 @@ private FlowWithSource save(Flow flow, CrudEventType crudEventType, String flowS
List<FlowWithSource> revisions = this.findRevisions(flow.getNamespace(), flow.getId());

if (revisions.size() > 0) {
flow = flow.withRevision(revisions.get(revisions.size() - 1).getRevision() + 1);
flow = flow.toBuilder().revision(revisions.get(revisions.size() - 1).getRevision() + 1).build();
} else {
flow = flow.withRevision(1);
flow = flow.toBuilder().revision(1).build();
}

Map<Field<Object>, Object> fields = this.jdbcRepository.persistFields(flow);
Expand All @@ -376,6 +380,10 @@ private FlowWithSource save(Flow flow, CrudEventType crudEventType, String flowS
@SneakyThrows
@Override
public Flow delete(Flow flow) {
if (flow instanceof FlowWithSource) {
flow = ((FlowWithSource) flow).toFlow();
}

Optional<Flow> revision = this.findById(flow.getNamespace(), flow.getId(), Optional.of(flow.getRevision()));
if (revision.isEmpty()) {
throw new IllegalStateException("Flow " + flow.getId() + " doesn't exists");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -195,6 +195,10 @@ public FlowWithSource update(Flow flow, Flow previous, String flowSource, Flow f
}

private FlowWithSource save(Flow flow, CrudEventType crudEventType, String flowSource) throws ConstraintViolationException {
if (flow instanceof FlowWithSource) {
flow = ((FlowWithSource) flow).toFlow();
}

// flow exists, return it
Optional<Flow> exists = this.findById(flow.getNamespace(), flow.getId());
Optional<String> existsSource = this.findSourceById(flow.getNamespace(), flow.getId());
Expand All @@ -205,9 +209,9 @@ private FlowWithSource save(Flow flow, CrudEventType crudEventType, String flowS
List<FlowWithSource> revisions = this.findRevisions(flow.getNamespace(), flow.getId());

if (revisions.size() > 0) {
flow = flow.withRevision(revisions.get(revisions.size() - 1).getRevision() + 1);
flow = flow.toBuilder().revision(revisions.get(revisions.size() - 1).getRevision() + 1).build();
} else {
flow = flow.withRevision(1);
flow = flow.toBuilder().revision(1).build();
}

this.flows.put(flowId(flow), flow);
Expand All @@ -222,6 +226,10 @@ private FlowWithSource save(Flow flow, CrudEventType crudEventType, String flowS

@Override
public Flow delete(Flow flow) {
if (flow instanceof FlowWithSource) {
flow = ((FlowWithSource) flow).toFlow();
}

if (this.findById(flow.getNamespace(), flow.getId(), Optional.of(flow.getRevision())).isEmpty()) {
throw new IllegalStateException("Flow " + flow.getId() + " doesn't exists");
}
Expand All @@ -232,8 +240,9 @@ public Flow delete(Flow flow) {
this.flows.remove(flowId(deleted));
this.revisions.put(deleted.uid(), deleted);

Flow finalFlow = flow;
ListUtils.emptyOnNull(flow.getTriggers())
.forEach(abstractTrigger -> triggerQueue.delete(Trigger.of(flow, abstractTrigger)));
.forEach(abstractTrigger -> triggerQueue.delete(Trigger.of(finalFlow, abstractTrigger)));

eventPublisher.publishEvent(new CrudEvent<>(flow, CrudEventType.DELETE));

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,6 +10,7 @@
import io.kestra.core.models.hierarchies.FlowGraph;
import io.kestra.core.models.tasks.Task;
import io.kestra.core.models.validations.ValidateConstraintViolation;
import io.kestra.core.serializers.JacksonMapper;
import io.kestra.core.serializers.YamlFlowParser;
import io.kestra.core.tasks.debugs.Return;
import io.kestra.core.tasks.flows.Sequential;
Expand Down Expand Up @@ -251,7 +252,18 @@ void createFlow() {
Flow get = parseFlow(client.toBlocking().retrieve(HttpRequest.GET("/api/v1/flows/" + flow.getNamespace() + "/" + flow.getId()), String.class));
assertThat(get.getId(), is(flow.getId()));
assertThat(get.getInputs().get(0).getName(), is("a"));
}

@Test
void createFlowWithJsonLabels() {
Map<String, Object> flow = JacksonMapper.toMap(generateFlow("io.kestra.unittest", "a"));
flow.put("labels", Map.of("a", "b"));

Flow result = parseFlow(client.toBlocking().retrieve(POST("/api/v1/flows", flow), String.class));

assertThat(result.getId(), is(flow.get("id")));
assertThat(result.getLabels().get(0).key(), is("a"));
assertThat(result.getLabels().get(0).value(), is("b"));
}

@Test
Expand Down

0 comments on commit b2fcf42

Please sign in to comment.