Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Missing transformer in data-plane-selector-control-api #4245

Closed
timdah opened this issue Jun 6, 2024 · 3 comments · Fixed by #4291
Closed

Missing transformer in data-plane-selector-control-api #4245

timdah opened this issue Jun 6, 2024 · 3 comments · Fixed by #4291
Assignees
Labels
bug Something isn't working

Comments

@timdah
Copy link
Contributor

timdah commented Jun 6, 2024

Bug Report

Describe the Bug

The getAllDataPlaneInstances endpoint throws a 500 because the DataPlaneInstance --> JsonObject transformer is not registered.

@Override
@GET
public JsonArray getAllDataPlaneInstances() {
var instances = service.getAll().orElseThrow(exceptionMapper(DataPlaneInstance.class));
return instances.stream()
.map(i -> transformerRegistry.transform(i, JsonObject.class))
.filter(Result::succeeded)
.map(Result::getContent)
.collect(toJsonArray());
}

@Override
public void initialize(ServiceExtensionContext context) {
validatorRegistry.register(DATAPLANE_INSTANCE_TYPE, DataPlaneInstanceValidator.instance());
typeTransformerRegistry.register(new JsonObjectToDataPlaneInstanceTransformer());
var controller = new DataplaneSelectorControlApiController(validatorRegistry, typeTransformerRegistry, dataPlaneSelectorService, clock);
webService.registerResource(ApiContext.CONTROL, controller);
}

Context Information

  • EDC 0.7.0

Possible Implementation

I see two possible solutions:

  1. Register the transformer
  2. Delete the endpoint, as this is a duplicate of the only endpoint (at least in V3) in the data-plane-selector-api extension.
    @Consumes({ MediaType.APPLICATION_JSON })
    @Produces({ MediaType.APPLICATION_JSON })
    @Path("/v3/dataplanes")
    public class DataplaneSelectorApiV3Controller implements DataplaneSelectorApiV3 {
    private final DataPlaneSelectorService selectionService;
    private final TypeTransformerRegistry transformerRegistry;
    public DataplaneSelectorApiV3Controller(DataPlaneSelectorService selectionService, TypeTransformerRegistry transformerRegistry) {
    this.selectionService = selectionService;
    this.transformerRegistry = transformerRegistry;
    }
    @Override
    @GET
    public JsonArray getAllDataPlaneInstances() {
    var instances = selectionService.getAll().orElseThrow(exceptionMapper(DataPlaneInstance.class));
    return instances.stream()
    .map(i -> transformerRegistry.transform(i, JsonObject.class))
    .filter(Result::succeeded)
    .map(Result::getContent)
    .collect(toJsonArray());
    }
    }
@github-actions github-actions bot added the triage all new issues awaiting classification label Jun 6, 2024
Copy link

github-actions bot commented Jun 6, 2024

Thanks for your contribution 🔥 We will take a look asap 🚀

@ronjaquensel ronjaquensel self-assigned this Jun 6, 2024
@ndr-brt
Copy link
Member

ndr-brt commented Jun 6, 2024

I vote solution 1, deletion is not correct, because control and management contexts should be kept separated as they serve separate needs, so this duplication is accepted (and I'll say needed) tho.

@paullatzelsperger
Copy link
Member

also, generally we shouldn't just remove endpoints without deprecating them first.

@ronjaquensel ronjaquensel added bug Something isn't working and removed triage all new issues awaiting classification labels Jun 7, 2024
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working
Projects
None yet
Development

Successfully merging a pull request may close this issue.

4 participants