throw a better error when a duplicate contract is created

manager/api/core/src/main/java/io/apiman/manager/api/core/IStorageQuery.java

@@ -255,5 +255,5 @@ public int getMaxPolicyOrderIndex(String organizationId, String entityId, String
      * @throws StorageException 
      */
     public List<PolicyDefinitionSummaryBean> listPluginPolicyDefs(Long pluginId) throws StorageException;
-    
+
 }

manager/api/rest-impl/src/main/java/io/apiman/manager/api/rest/impl/OrganizationResourceImpl.java

@@ -572,7 +572,7 @@ public ContractBean createContract(String organizationId, String applicationId,
             NotAuthorizedException {
         if (!securityContext.hasPermission(PermissionType.appEdit, organizationId))
             throw ExceptionFactory.notAuthorizedException();
-        
+
         ContractBean contract;
         ApplicationVersionBean avb;
         try {
@@ -640,10 +640,45 @@ public ContractBean createContract(String organizationId, String applicationId,
             throw e;
         } catch (Exception e) {
             storage.rollbackTx();
-            throw new SystemErrorException(e);
+            // Up above we are optimistically creating the contract.  If it fails, check to see
+            // if it failed because it was a duplicate.  If so, throw something sensible.  We 
+            // only do this on failure (we would get a FK contraint failure, for example) to 
+            // reduce overhead on the typical happy path.
+            if (contractAlreadyExists(organizationId, applicationId, version, bean)) {
+                throw ExceptionFactory.contractAlreadyExistsException();
+            } else {
+                throw new SystemErrorException(e);
+            }
         }
     }
 
+    /**
+     * Check to see if the contract already exists, by getting a list of all the 
+     * application's contracts and comparing with the one being created.
+     * @param organizationId
+     * @param applicationId
+     * @param version
+     * @param bean
+     */
+    private boolean contractAlreadyExists(String organizationId, String applicationId, String version,
+            NewContractBean bean) {
+        try {
+            List<ContractSummaryBean> contracts = query.getApplicationContracts(organizationId, applicationId, version);
+            for (ContractSummaryBean contract : contracts) {
+                if (contract.getServiceOrganizationId().equals(bean.getServiceOrgId()) &&
+                    contract.getServiceId().equals(bean.getServiceId()) &&
+                    contract.getServiceVersion().equals(bean.getServiceVersion()) &&
+                    contract.getPlanId().equals(bean.getPlanId())) 
+                {
+                    return true;
+                }
+            }
+            return false;
+        } catch (StorageException e) {
+            return false;
+        }
+    }
+
     /**
      * @see io.apiman.manager.api.rest.contract.IOrganizationResource#getContract(java.lang.String, java.lang.String, java.lang.String, java.lang.String)
      */

manager/test/api/src/test/resources/test-plan-data/contracts/contracts/007.5_create-duplicate-contract.resttest

@@ -0,0 +1,18 @@
+POST /organizations/Organization1/applications/Application1/versions/1.0/contracts admin/admin
+Content-Type: application/json
+
+{
+  "serviceOrgId" : "Organization1",
+  "serviceId" : "Service1",
+  "serviceVersion" : "1.0",
+  "planId" : "Plan1"
+}
+----
+409
+Content-Type: application/json
+
+{
+  "type" : "ContractAlreadyExistsException",
+  "errorCode" : 4005,
+  "message" : "Contract already exists."
+}

manager/test/api/src/test/resources/test-plans/contracts-testPlan.xml

@@ -42,6 +42,7 @@
     <test name="List Contracts for App 1">test-plan-data/contracts/contracts/006_get-contracts.resttest</test>
     <test name="List ApiRegistry for App 1 (JSON)">test-plan-data/contracts/contracts/007_get-apiregistry_json.resttest</test>
     <test name="List ApiRegistry for App 1 (XML)">test-plan-data/contracts/contracts/007_get-apiregistry_xml.resttest</test>
+    <test name="Create Duplicate Contract (failure)">test-plan-data/contracts/contracts/007.5_create-duplicate-contract.resttest</test>
     <test name="Break Contract 1">test-plan-data/contracts/contracts/008_delete-contract1.resttest</test>
     <test name="List Contracts for App 1 (after delete)">test-plan-data/contracts/contracts/009_get-contracts.resttest</test>
   </testGroup>