Skip to content

Conversation

@SimY4
Copy link

@SimY4 SimY4 commented Jul 3, 2018

No description provided.

@codecov-io
Copy link

Codecov Report

Merging #175 into master will decrease coverage by 0.03%.
The diff coverage is 0%.

Impacted file tree graph

@@             Coverage Diff              @@
##             master     #175      +/-   ##
============================================
- Coverage     63.65%   63.61%   -0.04%     
  Complexity      562      562              
============================================
  Files            72       72              
  Lines          1978     1979       +1     
  Branches        374      375       +1     
============================================
  Hits           1259     1259              
- Misses          574      575       +1     
  Partials        145      145
Impacted Files Coverage Δ Complexity Δ
...namodb/repository/query/AbstractDynamoDBQuery.java 19.49% <0%> (-0.17%) 7 <0> (ø)

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 48d3b58...7e73403. Read the comment docs.

@derjust
Copy link
Owner

derjust commented Jul 3, 2018

Thanks for your PR!

I just tested with the provided test case (big thanks!) but got the expected EmptyResultDataAccessException do you encounter a NullPointerException?
Which version of spring-data-dynamodb do you use?

@SimY4
Copy link
Author

SimY4 commented Jul 3, 2018

Hi, @derjust

So, provided by default in CrudRepository#deleteById(ID) will throw EmptyResultDataAccessException.

But if you create a custom delete method like MyRepo#deleteBySomethingEquals(String) if no entity satisfies given condition, Null pointer exception is raised.

@SimY4
Copy link
Author

SimY4 commented Jul 3, 2018

Another observation: the delete executor in current form will always delete single record even if query params will return more than one. Should I change query.getSingleResult() to query,getResultList() and then run bulk delete if result list is not empty?

This was referenced Jul 4, 2018
@derjust derjust merged commit 7e73403 into derjust:master Jul 5, 2018
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants