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.

Sign up for GitHub

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

docs(api): fix request and response types #4244

Merged
merged 1 commit into from
Jun 6, 2024
Merged

docs(api): fix request and response types #4244

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
1 change: 0 additions & 1 deletion ...ion-api/src/main/java/org/eclipse/edc/connector/api/management/version/v1/VersionApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@
public interface VersionApi {

@Operation(description = "Gets the exact SemVer string of the Management API",
operationId = "getVersion",
responses = {
@ApiResponse(responseCode = "200", description = "The secret",
content = @Content(schema = @Schema(implementation = SecretOutputSchema.class)))
Expand Down
15 changes: 5 additions & 10 deletions ...rc/main/java/org/eclipse/edc/connector/controlplane/api/management/asset/v3/AssetApi.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -40,7 +40,6 @@
@Tag(name = "Asset V3")
public interface AssetApi {
@Operation(description = "Creates a new asset together with a data address",
operationId = "createAssetV3",
requestBody = @RequestBody(content = @Content(schema = @Schema(implementation = AssetInputSchema.class))),
responses = {
@ApiResponse(responseCode = "200", description = "Asset was created successfully. Returns the asset Id and created timestamp",
Expand All @@ -50,10 +49,9 @@ public interface AssetApi {
@ApiResponse(responseCode = "409", description = "Could not create asset, because an asset with that ID already exists",
content = @Content(array = @ArraySchema(schema = @Schema(implementation = ApiCoreSchema.ApiErrorDetailSchema.class)))) }
)
JsonObject createAsset(JsonObject asset);
JsonObject createAssetV3(JsonObject asset);

@Operation(description = "Request all assets according to a particular query",
operationId = "requestAssetV3",
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

some tools like API gateways use the operationId to generate their internal routing model. removing it may pose a problem.
I guess the safest way is if we have both, distinct method names and operationIDs

Copy link
Member Author

@ndr-brt ndr-brt Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

by default the operationId reflects the method name, if the method is already called requestAssetV3, explicitly defining the operationId is not necessary

Copy link
Member

@paullatzelsperger paullatzelsperger Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

but doesn't that mean, that if an operationId is present, it would override whatever the method name is?
and if so, naming the methods ...V3... should not be necessary?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

it is necessary to avoid the generated openapi mess (I debugged it, not sure about the root cause, but the rename thing definitely fixes the issue).
I see two appreciated side effects:

  • it will avoid outdated operationId definitions (because they are sterile strings)
  • defining different method names for different versions makes total sense (think about a method that in 2 different versions has 2 different signatures/behaviors)

Copy link
Member

@paullatzelsperger paullatzelsperger Jun 6, 2024 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

so long as it fixes the issue, i'm all for it. we need to be extra careful with method renaming though, as it could cause obscure behaviour in API gateways etc.

think about a method that in 2 different versions has 2 different signatures/behaviors

That shouldn't be a problem, since they'd live in different classes anyway.

requestBody = @RequestBody(
content = @Content(schema = @Schema(implementation = ApiCoreSchema.QuerySpecSchema.class))
),
Expand All @@ -63,10 +61,9 @@ public interface AssetApi {
@ApiResponse(responseCode = "400", description = "Request body was malformed",
content = @Content(array = @ArraySchema(schema = @Schema(implementation = ApiCoreSchema.ApiErrorDetailSchema.class))))
})
JsonArray requestAssets(JsonObject querySpecJson);
JsonArray requestAssetsV3(JsonObject querySpecJson);

@Operation(description = "Gets an asset with the given ID",
operationId = "getAssetV3",
responses = {
@ApiResponse(responseCode = "200", description = "The asset",
content = @Content(schema = @Schema(implementation = AssetOutputSchema.class))),
Expand All @@ -76,12 +73,11 @@ public interface AssetApi {
content = @Content(array = @ArraySchema(schema = @Schema(implementation = ApiCoreSchema.ApiErrorDetailSchema.class))))
}
)
JsonObject getAsset(String id);
JsonObject getAssetV3(String id);

@Operation(description = "Removes an asset with the given ID if possible. Deleting an asset is only possible if that asset is not yet referenced " +
"by a contract agreement, in which case an error is returned. " +
"DANGER ZONE: Note that deleting assets can have unexpected results, especially for contract offers that have been sent out or ongoing or contract negotiations.",
operationId = "removeAssetV3",
responses = {
@ApiResponse(responseCode = "204", description = "Asset was deleted successfully"),
@ApiResponse(responseCode = "400", description = "Request was malformed, e.g. id was null",
Expand All @@ -91,19 +87,18 @@ public interface AssetApi {
@ApiResponse(responseCode = "409", description = "The asset cannot be deleted, because it is referenced by a contract agreement",
content = @Content(array = @ArraySchema(schema = @Schema(implementation = ApiCoreSchema.ApiErrorDetailSchema.class))))
})
void removeAsset(String id);
void removeAssetV3(String id);

@Operation(description = "Updates an asset with the given ID if it exists. If the asset is not found, no further action is taken. " +
"DANGER ZONE: Note that updating assets can have unexpected results, especially for contract offers that have been sent out or are ongoing in contract negotiations.",
operationId = "updateAssetV3",
requestBody = @RequestBody(content = @Content(schema = @Schema(implementation = AssetInputSchema.class))),
responses = {
@ApiResponse(responseCode = "204", description = "Asset was updated successfully"),
@ApiResponse(responseCode = "404", description = "Asset could not be updated, because it does not exist."),
@ApiResponse(responseCode = "400", description = "Request was malformed, e.g. id was null",
content = @Content(array = @ArraySchema(schema = @Schema(implementation = ApiCoreSchema.ApiErrorDetailSchema.class)))),
})
void updateAsset(JsonObject asset);
void updateAssetV3(JsonObject asset);

@Schema(name = "AssetInput", example = AssetInputSchema.ASSET_INPUT_EXAMPLE)
record AssetInputSchema(
Expand Down
10 changes: 5 additions & 5 deletions ...va/org/eclipse/edc/connector/controlplane/api/management/asset/v3/AssetApiController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -63,7 +63,7 @@ public AssetApiController(AssetService service, TypeTransformerRegistry transfor

@POST
@Override
public JsonObject createAsset(JsonObject assetJson) {
public JsonObject createAssetV3(JsonObject assetJson) {
validator.validate(EDC_ASSET_TYPE, assetJson).orElseThrow(ValidationFailureException::new);

var asset = transformerRegistry.transform(assetJson, Asset.class)
Expand All @@ -83,7 +83,7 @@ public JsonObject createAsset(JsonObject assetJson) {
@POST
@Path("/request")
@Override
public JsonArray requestAssets(JsonObject querySpecJson) {
public JsonArray requestAssetsV3(JsonObject querySpecJson) {
QuerySpec querySpec;
if (querySpecJson == null) {
querySpec = QuerySpec.Builder.newInstance().build();
Expand All @@ -105,7 +105,7 @@ public JsonArray requestAssets(JsonObject querySpecJson) {
@GET
@Path("{id}")
@Override
public JsonObject getAsset(@PathParam("id") String id) {
public JsonObject getAssetV3(@PathParam("id") String id) {
var asset = of(id)
.map(it -> service.findById(id))
.orElseThrow(() -> new ObjectNotFoundException(Asset.class, id));
Expand All @@ -118,13 +118,13 @@ public JsonObject getAsset(@PathParam("id") String id) {
@DELETE
@Path("{id}")
@Override
public void removeAsset(@PathParam("id") String id) {
public void removeAssetV3(@PathParam("id") String id) {
service.delete(id).orElseThrow(exceptionMapper(Asset.class, id));
}

@PUT
@Override
public void updateAsset(JsonObject assetJson) {
public void updateAssetV3(JsonObject assetJson) {
validator.validate(EDC_ASSET_TYPE, assetJson).orElseThrow(ValidationFailureException::new);

var assetResult = transformerRegistry.transform(assetJson, Asset.class)
Expand Down
17 changes: 3 additions & 14 deletions ...g/eclipse/edc/connector/controlplane/api/management/catalog/BaseCatalogApiController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,7 @@
package org.eclipse.edc.connector.controlplane.api.management.catalog;

import jakarta.json.JsonObject;
import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.container.AsyncResponse;
import jakarta.ws.rs.container.Suspended;
import org.eclipse.edc.connector.controlplane.catalog.spi.CatalogRequest;
import org.eclipse.edc.connector.controlplane.catalog.spi.DatasetRequest;
import org.eclipse.edc.connector.controlplane.services.spi.catalog.CatalogService;
Expand All @@ -32,12 +27,10 @@
import org.eclipse.edc.web.spi.exception.InvalidRequestException;
import org.eclipse.edc.web.spi.exception.ValidationFailureException;

import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON;
import static org.eclipse.edc.connector.controlplane.catalog.spi.CatalogRequest.CATALOG_REQUEST_TYPE;
import static org.eclipse.edc.connector.controlplane.catalog.spi.DatasetRequest.DATASET_REQUEST_TYPE;

@Consumes(APPLICATION_JSON)
@Produces(APPLICATION_JSON)

public abstract class BaseCatalogApiController {

private final CatalogService service;
Expand All @@ -51,9 +44,7 @@ public BaseCatalogApiController(CatalogService service, TypeTransformerRegistry
this.validatorRegistry = validatorRegistry;
}

@POST
@Path("/request")
public void requestCatalog(JsonObject requestBody, @Suspended AsyncResponse response) {
public void requestCatalog(JsonObject requestBody, AsyncResponse response) {
validatorRegistry.validate(CATALOG_REQUEST_TYPE, requestBody).orElseThrow(ValidationFailureException::new);

var request = transformerRegistry.transform(requestBody, CatalogRequest.class)
Expand All @@ -69,9 +60,7 @@ public void requestCatalog(JsonObject requestBody, @Suspended AsyncResponse resp
});
}

@POST
@Path("dataset/request")
public void getDataset(JsonObject requestBody, @Suspended AsyncResponse response) {
public void getDataset(JsonObject requestBody, AsyncResponse response) {
validatorRegistry.validate(DATASET_REQUEST_TYPE, requestBody).orElseThrow(ValidationFailureException::new);

var request = transformerRegistry.transform(requestBody, DatasetRequest.class)
Expand Down
6 changes: 2 additions & 4 deletions ...n/java/org/eclipse/edc/connector/controlplane/api/management/catalog/v2/CatalogApiV2.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,7 +39,6 @@ public interface CatalogApiV2 {

@Operation(
requestBody = @RequestBody(content = @Content(schema = @Schema(implementation = CatalogRequestSchema.class))),
operationId = "requestCatalogV2",
responses = { @ApiResponse(
content = @Content(
mediaType = "application/json",
Expand All @@ -49,11 +48,10 @@ public interface CatalogApiV2 {
deprecated = true
)
@Deprecated(since = "0.7.0")
void requestCatalog(JsonObject request, @Suspended AsyncResponse response);
void requestCatalogV2(JsonObject request, @Suspended AsyncResponse response);

@Operation(
requestBody = @RequestBody(content = @Content(schema = @Schema(implementation = DatasetRequestSchema.class))),
operationId = "getDatasetV2",
responses = { @ApiResponse(
content = @Content(
mediaType = "application/json",
Expand All @@ -63,7 +61,7 @@ public interface CatalogApiV2 {
deprecated = true
)
@Deprecated(since = "0.7.0")
void getDataset(JsonObject request, @Suspended AsyncResponse response);
void getDatasetV2(JsonObject request, @Suspended AsyncResponse response);

@Schema(name = "CatalogRequest", example = CatalogRequestSchema.CATALOG_REQUEST_EXAMPLE)
record CatalogRequestSchema(
Expand Down
26 changes: 19 additions & 7 deletions .../eclipse/edc/connector/controlplane/api/management/catalog/v2/CatalogApiV2Controller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,15 +15,23 @@
package org.eclipse.edc.connector.controlplane.api.management.catalog.v2;

import jakarta.json.JsonObject;
import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.container.AsyncResponse;
import org.eclipse.edc.api.ApiWarnings;
import jakarta.ws.rs.container.Suspended;
import org.eclipse.edc.connector.controlplane.api.management.catalog.BaseCatalogApiController;
import org.eclipse.edc.connector.controlplane.services.spi.catalog.CatalogService;
import org.eclipse.edc.spi.monitor.Monitor;
import org.eclipse.edc.transform.spi.TypeTransformerRegistry;
import org.eclipse.edc.validator.spi.JsonObjectValidatorRegistry;

import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON;
import static org.eclipse.edc.api.ApiWarnings.deprecationWarning;

@Consumes(APPLICATION_JSON)
@Produces(APPLICATION_JSON)
@Path("/v2/catalog")
public class CatalogApiV2Controller extends BaseCatalogApiController implements CatalogApiV2 {
private final Monitor monitor;
Expand All @@ -33,15 +41,19 @@ public CatalogApiV2Controller(CatalogService service, TypeTransformerRegistry tr
this.monitor = monitor;
}

@POST
@Path("/request")
@Override
public void requestCatalog(JsonObject requestBody, AsyncResponse response) {
monitor.warning(ApiWarnings.deprecationWarning("/v2", "/v3"));
super.requestCatalog(requestBody, response);
public void requestCatalogV2(JsonObject requestBody, @Suspended AsyncResponse response) {
monitor.warning(deprecationWarning("/v2", "/v3"));
requestCatalog(requestBody, response);
}

@POST
@Path("dataset/request")
@Override
public void getDataset(JsonObject requestBody, AsyncResponse response) {
monitor.warning(ApiWarnings.deprecationWarning("/v2", "/v3"));
super.getDataset(requestBody, response);
public void getDatasetV2(JsonObject requestBody, @Suspended AsyncResponse response) {
monitor.warning(deprecationWarning("/v2", "/v3"));
getDataset(requestBody, response);
}
}
6 changes: 2 additions & 4 deletions ...n/java/org/eclipse/edc/connector/controlplane/api/management/catalog/v3/CatalogApiV3.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -39,27 +39,25 @@ public interface CatalogApiV3 {

@Operation(
requestBody = @RequestBody(content = @Content(schema = @Schema(implementation = CatalogRequestSchema.class))),
operationId = "requestCatalogV3",
responses = { @ApiResponse(
content = @Content(
mediaType = "application/json",
schema = @Schema(implementation = CatalogSchema.class)
),
description = "Gets contract offers (=catalog) of a single connector") }
)
void requestCatalog(JsonObject request, @Suspended AsyncResponse response);
void requestCatalogV3(JsonObject request, @Suspended AsyncResponse response);

@Operation(
requestBody = @RequestBody(content = @Content(schema = @Schema(implementation = DatasetRequestSchema.class))),
operationId = "getDatasetV3",
responses = { @ApiResponse(
content = @Content(
mediaType = "application/json",
schema = @Schema(implementation = DatasetSchema.class)
),
description = "Gets single dataset from a connector") }
)
void getDataset(JsonObject request, @Suspended AsyncResponse response);
void getDatasetV3(JsonObject request, @Suspended AsyncResponse response);

@Schema(name = "CatalogRequest", example = CatalogRequestSchema.CATALOG_REQUEST_EXAMPLE)
record CatalogRequestSchema(
Expand Down
24 changes: 24 additions & 0 deletions .../eclipse/edc/connector/controlplane/api/management/catalog/v3/CatalogApiV3Controller.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -14,15 +14,39 @@

package org.eclipse.edc.connector.controlplane.api.management.catalog.v3;

import jakarta.json.JsonObject;
import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.container.AsyncResponse;
import jakarta.ws.rs.container.Suspended;
import org.eclipse.edc.connector.controlplane.api.management.catalog.BaseCatalogApiController;
import org.eclipse.edc.connector.controlplane.services.spi.catalog.CatalogService;
import org.eclipse.edc.transform.spi.TypeTransformerRegistry;
import org.eclipse.edc.validator.spi.JsonObjectValidatorRegistry;

import static jakarta.ws.rs.core.MediaType.APPLICATION_JSON;

@Consumes(APPLICATION_JSON)
@Produces(APPLICATION_JSON)
@Path("/v3/catalog")
public class CatalogApiV3Controller extends BaseCatalogApiController implements CatalogApiV3 {
public CatalogApiV3Controller(CatalogService service, TypeTransformerRegistry transformerRegistry, JsonObjectValidatorRegistry validatorRegistry) {
super(service, transformerRegistry, validatorRegistry);
}

@POST
@Path("/request")
@Override
public void requestCatalogV3(JsonObject request, @Suspended AsyncResponse response) {
requestCatalog(request, response);
}

@POST
@Path("dataset/request")
@Override
public void getDatasetV3(JsonObject request, @Suspended AsyncResponse response) {
getDataset(request, response);
}
}
19 changes: 2 additions & 17 deletions ...tor/controlplane/api/management/contractagreement/BaseContractAgreementApiController.java
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -16,13 +16,6 @@

import jakarta.json.JsonArray;
import jakarta.json.JsonObject;
import jakarta.ws.rs.Consumes;
import jakarta.ws.rs.GET;
import jakarta.ws.rs.POST;
import jakarta.ws.rs.Path;
import jakarta.ws.rs.PathParam;
import jakarta.ws.rs.Produces;
import jakarta.ws.rs.core.MediaType;
import org.eclipse.edc.connector.controlplane.contract.spi.types.agreement.ContractAgreement;
import org.eclipse.edc.connector.controlplane.contract.spi.types.offer.ContractDefinition;
import org.eclipse.edc.connector.controlplane.services.spi.contractagreement.ContractAgreementService;
Expand All @@ -42,8 +35,6 @@
import static org.eclipse.edc.spi.query.QuerySpec.EDC_QUERY_SPEC_TYPE;
import static org.eclipse.edc.web.spi.exception.ServiceResultHandler.exceptionMapper;

@Consumes({ MediaType.APPLICATION_JSON })
@Produces({ MediaType.APPLICATION_JSON })
public abstract class BaseContractAgreementApiController {
protected final Monitor monitor;
private final ContractAgreementService service;
Expand All @@ -58,8 +49,6 @@ public BaseContractAgreementApiController(ContractAgreementService service, Type
this.validatorRegistry = validatorRegistry;
}

@POST
@Path("/request")
public JsonArray queryAgreements(JsonObject querySpecJson) {
QuerySpec querySpec;
if (querySpecJson == null) {
Expand All @@ -79,19 +68,15 @@ public JsonArray queryAgreements(JsonObject querySpecJson) {
.collect(toJsonArray());
}

@GET
@Path("{id}")
public JsonObject getAgreementById(@PathParam("id") String id) {
public JsonObject getAgreementById(String id) {
return Optional.of(id)
.map(service::findById)
.map(it -> transformerRegistry.transform(it, JsonObject.class)
.orElseThrow(failure -> new EdcException(failure.getFailureDetail())))
.orElseThrow(() -> new ObjectNotFoundException(ContractAgreement.class, id));
}

@GET
@Path("{id}/negotiation")
public JsonObject getNegotiationByAgreementId(@PathParam("id") String id) {
public JsonObject getNegotiationByAgreementId(String id) {
return Optional.of(id)
.map(service::findNegotiation)
.map(it -> transformerRegistry.transform(it, JsonObject.class)
Expand Down
Loading
Loading