Refactor get_expected_skiplink()
#220
Merged
Conversation
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Matches current behavior of `skiplink_seq_num()` method
Codecov Report
@@ Coverage Diff @@
## implement-new-validation-spec #220 +/- ##
=================================================================
- Coverage 94.32% 94.31% -0.01%
=================================================================
Files 46 46
Lines 4086 4082 -4
=================================================================
- Hits 3854 3850 -4
Misses 232 232
Help us with your feedback. Take ten seconds to tell us how you rate us. Have a feature suggestion? Share it here. |
cafca
commented
Aug 3, 2022
| @@ -503,7 +503,7 @@ mod tests { | |||
| #[rstest] | |||
| #[case::ok(&[(0, 8)], (0, 8))] | |||
| #[should_panic( | |||
| expected = "Expected skiplink target for <Author 53fc96> at log id 0 and seq num 4 not found in database" | |||
| expected = "Expected skiplink target not found in store: <Author 53fc96>, log id 0, seq num 4" | |||
There was a problem hiding this comment.
I thought it might be handy to structure the log so that it names the issue first (not found) and then gives the details (ids, etc.), then it's easier to read.
cafca
commented
Aug 3, 2022
| .get_entry_at_seq_num(author, log_id, &skiplink_seq_num) | ||
| .await? | ||
| } | ||
| // Unwrap because method always returns `Some` for seq num > 1 |
There was a problem hiding this comment.
Depending on how we handle this this would have to be changed again later.
sandreae
added a commit
that referenced
this pull request
Aug 6, 2022
* Rename custom error type * Create validation module * Helper validation method used in nextArgs * GQL scalar type for DocumentViewId * Include new modules * Refactor next_entry_args * WIP: refactor publish entry * WIP: refactor publish entry * WIP: nearly there with publish_entry * Introduce domain module * Make more fine grained methods for validation * Remove duplicate insertion of operation from replication service * Fix skiplink and backlink orsering mistake * Comment out test which hangs * Introduce test methods to be used instead of publish() and next_args() * Use cbor macro for more readabe test operations * update expected test hashes * Split test_utils in store into seperate files * Don't validate operation against schema for now * Make existing tests pass * Comment out operation schema validation for now * Test for ensure_log_id * Update test const names * Commit Cargo.lock * Use cbor macro for more readabe test operations * Remove modules which accidentaly came across in cherry-pick * Update tests for LogId's which now start from 0 * Update CHANGELOG * Specify p2panda-rs dep by commit hash * Fixes after p2panda bump * Test for ensure_entry_contains_expected_log_id * Split up data and api validation tests for publish_entry * Refactor verify seq_num, log_id, skiplink and backlink methods * Propergate panic from test runner back to test runtime * Rework seq and log id validation * Test for seq num validation * Refactor verify_log_id and add tests * Update test values * get_expected_skiplink doesn't return option plus tests * Don't return option from get_expected_backlink plus tests * A few more e2e tests * Make clippy happy * Fix after merge * Tidy test publish and next_args methods * Don't depend on DocumentStore in ensure_document_not_deleted * Remove unverified test versions of publish and next_args (whoop!) * Tests for ensure_document_not_deleted * Publish new entry on service bus in replication service * Remove unused deps * Remove unused deps * Add comment about code which will be deprecated after this PR * Remove param validation from next_args * Fix comment * Implement get_latest_log_id on SqlStorage and then safely increment with error handling * Helper methods for safely incrementing seq numbers * Validate passed parameters in gql query method * Refactor verify_seq_num * Move validation of entry and operation out of * Add doc strings for next_args * Remove determine_document_id helper * Add doc strings for `publish` * Fix test error string * Better seperation for pre-publish validation logic * Some nice refactoring in next_args * Move location of determine next args in publish * Implement SqlStorage using new associated type pattern * Make store implementation generic * Correct test names * Test for publish with missing skiplink using MemoryStore * Commit Cargo.lock * Clippy & fmt * Clippy... * Refactor one test * Test for errors when calling publish * Refactor seq_num and backlink getter to avoid unnecessary db call * More cases for missing entrytests * WIP: tests for publishing operations * Helpers for domain tests * Update duplicate entry test * Tests for next_args * Remove some TODOs * Fix one test * Remove println * Remove deprecated request and response structs * More tests for next args * Change test order * Remove wrapper method around bemboo verification * Test for DocumentViewId GQL scalar * Revert to using DocumentId in next_args GQL query plus test * Doc strings in verify module * Tests for publishing to valid and invalid logs * Deleted document tests * Test for missing next skiplink * Increment seq and log tests * Fix expected error strings * Test for latest_log_id * Update CHANGELOG * Docstring and comment review * Clearer error messages * Remove commited test logging file * License header * Comment about updating nex_entry_args API * Typo * Remove println * Better SQL query for latest_log_id * Correct comment string * Don't use nested imports * Correct comment * Move error into match statement * Revert get_expected_skiplink ensure behaviour and improve error message * Another test for get_expected_skiplink * Target p2panda-rs main branch * Fix CHANGELOG.md formatting * Use new string methods and `Human` trait for display in errors * Error in publish when next args have MAX_SEQ_NUM with tests * Comments in test * Rename function with get_checked prefix * A little more clippy happy * Refactor `get_expected_skiplink()` (#220) * Refactor `get_expected_skiplink()` Matches current behavior of `skiplink_seq_num()` method * Remove import * Remove comment * Fix tests * Use rev for p2panda-rs version * Group imports * Refactor initialize_db method * Use only one thread for tarpaulin Co-authored-by: Vincent Ahrend <mail@vincentahrend.com> Co-authored-by: Andreas Dzialocha <x12@adz.garden>
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
Matches current behavior of
skiplink_seq_num()methodDescribe your changes here.
📋 Checklist
CHANGELOG.md