fix(manager-api): perform unregister only if client is in correct state
If a contracts is deleted we try to reregister or unregister the client automatically, however there
was no check if the current state was correct and an `ActionException` was thrown in some cases.
Closes:
MODIFY
manager/api/rest-impl/src/main/java/io/apiman/manager/api/service/ContractService.java
manager/api/rest-impl/src/main/java/io/apiman/manager/api/service/ContractService.java
@@ -237,7 +237,13 @@ public class ContractService implements DataAccessUtilMixin {
ArrayList<ContractBean> contractsToDelete = Lists.newArrayList(tryAction(() -> storage.getAllContracts(organizationId, clientId, version)));
try {
+ if (!contractsToDelete.isEmpty()) {
+ // Need to unregister client if in Registered or AwaitingApproval state
+ ClientStatus clientStatus = contractsToDelete.get(0).getClient().getStatus();
+ if (clientStatus == ClientStatus.Registered || clientStatus == ClientStatus.AwaitingApproval) {
actionService.unregisterClient(organizationId, clientId, version);
+ }
+ }
deleteContractsInternal(organizationId, clientId, version, contractsToDelete, contractsToDelete);
} catch (Exception e) {
throw new SystemErrorException(e);
@@ -254,7 +260,10 @@ public class ContractService implements DataAccessUtilMixin {
ContractBean contractToDelete = storage.getContract(contractId);
if (allContracts.size() <= 1) {
// If we are deleting the only/last contract, then we can unregister.
+ ClientStatus clientStatus = contractToDelete.getClient().getStatus();
+ if (clientStatus == ClientStatus.Registered || clientStatus == ClientStatus.AwaitingApproval) {
actionService.unregisterClient(organizationId, clientId, version);
+ }
deleteContractsInternal(organizationId, clientId, version, allContracts, List.of(contractToDelete));
} else {
// If we still have contracts left, we should re-register the existing client.

MODIFY
manager/test/api/src/test/java/io/apiman/manager/test/ReregisterClientTest.java
manager/test/api/src/test/java/io/apiman/manager/test/ReregisterClientTest.java
@@ -40,6 +40,8 @@ import org.junit.runner.RunWith;
"DELETE:/mock-gateway/clients/Organization/Client/1.0\n" +
"GET:/mock-gateway/system/status\n" +
"PUT:/mock-gateway/clients\n" +
+ "GET:/mock-gateway/system/status\n" +
+ "DELETE:/mock-gateway/clients/Organization/Client/1.0\n" +
""
)
@ManagerRestTestPublishPayload({

MODIFY
manager/test/api/src/test/resources/test-plan-data/reregister-client/109_create-contract.resttest
manager/test/api/src/test/resources/test-plan-data/reregister-client/109_create-contract.resttest
@@ -10,6 +10,7 @@ Content-Type: application/json
----
200
Content-Type: application/json
+X-RestTest-BindTo-contractId: id
{
"client" : {

ADD
manager/test/api/src/test/resources/test-plan-data/reregister-client/115_delete-contract.resttest
+DELETE /organizations/Organization/clients/Client/versions/1.0/contracts/${contractId} admin/admin
+----
+204

ADD
manager/test/api/src/test/resources/test-plan-data/reregister-client/116_get-version-1.0.resttest
+GET /organizations/Organization/clients/Client/versions/1.0 admin/admin
+----
+200
+Content-Type: application/json
+
+{
+ "client" : {
+ "organization" : {
+ "id" : "Organization"
+ },
+ "id" : "Client",
+ "name" : "Client",
+ "description" : "This is the description of the Client.",
+ "createdBy":"admin"
+ },
+ "status" : "Retired",
+ "version" : "1.0",
+ "createdBy" : "admin"
+}

MODIFY
manager/test/api/src/test/resources/test-plans/reregister-client-testPlan.xml
manager/test/api/src/test/resources/test-plans/reregister-client-testPlan.xml
<test name="Re-Register Client">test-plan-data/reregister-client/112_reregister-client.resttest</test>
<test name="Retire Client">test-plan-data/reregister-client/113_unregister-client.resttest</test>
<test name="Re-Register Client [2]">test-plan-data/reregister-client/114_reregister-client.resttest</test>
+ <!--Should not unregister an retired client-->
+ <test name="Retire Client">test-plan-data/reregister-client/113_unregister-client.resttest</test>
+ <test name="Break Contract">test-plan-data/reregister-client/115_delete-contract.resttest</test>
+ <test name="Get Version - retired">test-plan-data/reregister-client/116_get-version-1.0.resttest</test>
</testGroup>
</testPlan>

fix(manager-api): perform unregister only if client is in correct state

If a contracts is deleted we try to reregister or unregister the client automatically, however there 
was no check if the current state was correct and an `ActionException` was thrown in some cases.

Closes: #2404

Co-authored-by: Marc Savy <marc@rhymewithgravy.com>