SQL: Deletion of Assets impossible after first Contract Negotation

[DominikPinsel]

Bug Report

Describe the Bug

Affects all connectors that are using the PostgreSQL Asset Extension.

After the connector has done one or more contract negotiations, it is impossible to delete assets.

Expected Behavior

• deletion of assets possible as long as its legal (not part of any agreement etc.)

Observed Behavior

• deletion of all assets impossible after contract negotiation

Steps to Reproduce

Steps to reproduce the behavior:

1. Use Connector with PostgreSQL Asset Extension
2. Create Asset, etc. and do a contract negotiation with it
3. Create a new Asset
4. Delete the new Asset -> It's not possible to delete this new asset

Context Information

Before deleting the Asset the AssetService will check, whether a negotation/agreement exists for this asset.

 @Override
    public ServiceResult<Asset> delete(String assetId) {
        return transactionContext.execute(() -> {
            var filter = format("contractAgreement.assetId = %s", assetId);
            var query = QuerySpec.Builder.newInstance().filter(filter).build();

            // the store now returns always a negotation 
            var negotiationsOnAsset = contractNegotiationStore.queryNegotiations(query);
            if (negotiationsOnAsset.findAny().isPresent()) {
                return ServiceResult.conflict(format("Asset %s cannot be deleted as it is referenced by at least one contract agreement", assetId));
            }

            var deleted = loader.deleteById(assetId);
            if (deleted == null) {
                return ServiceResult.notFound(format("Asset %s does not exist", assetId));
            }

            return ServiceResult.success(deleted);
        });
    }

Because of an unfinished SQL Query the store will always return a negotiation if there exists at least one.

    @Override
    public String getQueryNegotiationsTemplate() {
        // todo: add WHERE ... AND ... ORDER BY... statements here
        return format("SELECT * FROM %s LEFT OUTER JOIN %s ON %s.%s = %s.%s LIMIT ? OFFSET ?;", getContractNegotiationTable(), getContractAgreementTable(),
                getContractNegotiationTable(), getContractAgreementIdFkColumn(), getContractAgreementTable(), getContractAgreementIdColumn());
    }

Detailed Description

Possible Implementation

• resolve TODO

_{Dominik Pinsel [email protected], Mercedes-Benz Tech Innovation GmbH, legal info/Impressum}

[paullatzelsperger]

I'm not sure the todo has much to do with it. I think the easiest fix is to create a separate method (getNegotiationsWithAgreementOnAsset() or similar) in the ContractNegotiationStore interface and a dedicated SQL query for this. If that method can simply return a boolean ("hasAgreements") then this statement could look similar to this:

SELECT *
FROM edc_contract_negotiation
WHERE edc_contract_negotiation.contract_agreement_id in (SELECT agreement_id
                                                         FROM edc_contract_agreement
                                                         WHERE asset_id = 'asset1');

if we want that new method to return the ContractNegotiation, we probably have to enhance the statement with a JOIN to also get the ContractAgreement

SELECT *
FROM edc_contract_negotiation
INNER JOIN edc_contract_agreement eca on edc_contract_negotiation.contract_agreement_id = eca.agreement_id
WHERE edc_contract_negotiation.contract_agreement_id in (SELECT agreement_id
                                                         FROM edc_contract_agreement
                                                         WHERE asset_id = 'asset1')

Of course, equivalent implementations for the other stores would also have to be done too.
@ndr-brt what are your thoughts on this?

[ndr-brt]

@paullatzelsperger I'm late at the party, but I think as easy-fix having a dedicated method would work, however in the long run would be better to have the postgresql implementations being able to handle the QuerySpec in its entirety to permit having a single implementation for this method across all the database implementations.

[paullatzelsperger]

@paullatzelsperger I'm late at the party, but I think as easy-fix having a dedicated method would work, however in the long run would be better to have the postgresql implementations being able to handle the QuerySpec in its entirety to permit having a single implementation for this method across all the database implementations.

I agree, having an implemention that can interpret QuerySpec filter expressions would be best, but it needs careful thinking with regards to security (SQL injection), and flexibility - after all, the database schema is (theoretically) flexible. Also, this would leak the database schema through the API, which is something to be aware of.

On the other hand I am a big fan of having explicit methods for a particular use case, but they should rather be on the service than on the store.

I'll open a discussion about this.