fix: Support deleted object in ledger_entry #1483
Conversation
* created query to fetch last two entries * fix duplicate query
* created query to fetch last two entries * fix duplicate query * add db fetch func * adding db fetching func * changed Blob to pair<uint, Blob> to catch sequence as well
* added deleted_ledger_index logic to extract last two ledger objects * fixed a bug on index, should be non-negative * optimize the logic * fixed a previous bug in doFetchLastTwoLedgerObjects and develped the new API for entry ledger * fix tag_invoke default value for deleted_ledger_index & add some log info * add RPC field for include deleted and improve a style issue * remove an impossible case for LedgerEntry * fix an assignment issue on deleted_ledger_index * add a comment
* init unit test cases
* add a new case and change a test name
* add a new test case
* added comments to each test and remove redudant code
* change {} to Blob{} to improve readability
There was a problem hiding this comment.
Since the include_deleted is optional, I would suggest to try to utilize our cache as much as possible. Maybe we don't need to land on DB every time.
1 Keep using 'fetchLedgerObject' to get the last object by seq (This method will use cache effectively)
2 If include_deleted is true and the object from 1 is empty , use fetchLedgerObject + the seq_from_object_1 -1 getting the second one.
I think it might be easier and no need extra CQL statement. I am happy to discuss it offline if needed.
BTW, please assign the issue to yourself when you start doing that. In case other people may pick it as no one works on it.
cindyyan317
changed the title
* remove previous fetchTwo functions and change the unit tests
Kassaking7
changed the title
Kassaking7
changed the title
…eader. Make changes on those name
Codecov ReportAttention: Patch coverage is
Additional details and impacted files@@ Coverage Diff @@
## develop #1483 +/- ##
===========================================
+ Coverage 68.13% 68.18% +0.04%
===========================================
Files 236 236
Lines 9558 9576 +18
Branches 5308 5320 +12
===========================================
+ Hits 6512 6529 +17
- Misses 1660 1661 +1
Partials 1386 1386 ☔ View full report in Codecov by Sentry. |
There was a problem hiding this comment.
I think the seq you are using to query the second latest object is not right. You need the seq which the object is modified at.
You can find more info about 'objects' table from here
|
I give a change on the logic:
Hope this time it works good :) I created a unit test for this case and looks good to me. |
| if (!ledgerObject || ledgerObject->empty()) { | ||
| if (not input.includeDeleted) | ||
| return Error{Status{"entryNotFound"}}; | ||
| auto const deletedSeq = sharedPtrBackend_->fetchLedgerObjectSeq(key, lgrInfo.seq, ctx.yield); |
There was a problem hiding this comment.
Do we need to check deletedSeq, it can be nullopt?
…g7/clio into project-DEFI-129-branch
Fixes #1306