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

Bugfix: check to determine if asset can be deleted #1408

Conversation

paullatzelsperger
Copy link
Member

@paullatzelsperger paullatzelsperger commented Jun 2, 2022

What this PR changes/adds

This PR adds a method to the ContractNegotiationStore that returns all ContractNegotiations that have a ContractAgreement which references a particular asset in order to check, whether the asset can be deleted or not.

Why it does that

Deleting Assets should be forbidden once it is referenced by a ContractAgreement. Previously the check used a filter expression, which is not (yet) implemented for SQL.

Further notes

  • The implementations for CosmosDB and InMem did not change functionally - the same query as was there before can be used. Only SQL - due to its relational nature - needed some love.
  • the ReflectionUtil threw a NPE when recursively resolving a field, who's parent is null, e.g. ContractNegotiation -> contractAgreement -> assetId when contractAgreement is null.

Linked Issue(s)

Closes #1403

Checklist

  • added appropriate tests? (some)
  • performed checkstyle check locally?
  • added/updated copyright headers?
  • documented public classes/methods?
  • added/updated relevant documentation?
  • added relevant details to the changelog? (skip with label no-changelog)
  • formatted title correctly? (take a look at the CONTRIBUTING and styleguide for details)

@paullatzelsperger paullatzelsperger force-pushed the bugfix/1403_fix_asset_deletion_check branch from f307dff to 2bdfddc Compare June 3, 2022 06:36
@codecov-commenter
Copy link

codecov-commenter commented Jun 3, 2022

Codecov Report

Merging #1408 (dbf03c4) into main (4a1cca9) will increase coverage by 0.00%.
The diff coverage is 80.00%.

@@           Coverage Diff           @@
##             main    #1408   +/-   ##
=======================================
  Coverage   67.60%   67.61%           
=======================================
  Files         719      719           
  Lines       15911    15928   +17     
  Branches     1043     1044    +1     
=======================================
+ Hits        10757    10770   +13     
- Misses       4678     4683    +5     
+ Partials      476      475    -1     
Impacted Files Coverage Δ
...otiation/store/CosmosContractNegotiationStore.java 60.81% <0.00%> (-2.57%) ⬇️
...gotiation/store/ContractNegotiationStatements.java 96.42% <ø> (ø)
...negotiation/store/SqlContractNegotiationStore.java 81.72% <66.66%> (-0.51%) ⬇️
...aceconnector/common/reflection/ReflectionUtil.java 87.50% <100.00%> (+0.54%) ⬆️
...tiationstore/InMemoryContractNegotiationStore.java 90.47% <100.00%> (+2.14%) ⬆️
...datamanagement/asset/service/AssetServiceImpl.java 100.00% <100.00%> (ø)
.../contractnegotiation/store/PostgresStatements.java 89.18% <100.00%> (+1.68%) ⬆️

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 4a1cca9...dbf03c4. Read the comment docs.

@paullatzelsperger paullatzelsperger force-pushed the bugfix/1403_fix_asset_deletion_check branch 3 times, most recently from bac44b3 to d4105b9 Compare June 3, 2022 07:41
@paullatzelsperger paullatzelsperger marked this pull request as ready for review June 5, 2022 09:46
@paullatzelsperger paullatzelsperger requested review from ronjaquensel and removed request for ndr-brt June 5, 2022 09:47
@paullatzelsperger paullatzelsperger force-pushed the bugfix/1403_fix_asset_deletion_check branch from 5625a44 to f179074 Compare June 7, 2022 05:16
@paullatzelsperger paullatzelsperger merged commit df0ed36 into eclipse-edc:main Jun 7, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

SQL: Deletion of Assets impossible after first Contract Negotation
3 participants