Merge multipath PRs (#473 and #477) #487
Conversation
# Conflicts: # src/commands.rs
There was a problem hiding this comment.
Fixed per @MeirShpilraien 's review (except for splitting tests that will be done later)
Cargo.toml
Outdated
| @@ -14,7 +14,7 @@ bson = "0.14" | |||
| serde_json = "1.0" | |||
| serde = "1.0" | |||
| libc = "0.2" | |||
| jsonpath_lib = { git = "https://github.com/RedisJSON/jsonpath.git", branch = "generic_json_path" } | |||
| jsonpath_lib = { git = "https://github.com/RedisJSON/jsonpath.git", branch = "omer-multipath-api" } | |||
There was a problem hiding this comment.
Make sure to change it back once you merge: RedisJSON/jsonpath#18
src/commands.rs
Outdated
| _ => RedisValue::Null, | ||
| }); | ||
| } | ||
| redis_key.apply_changes(ctx, "json.arrtrim")?; |
There was a problem hiding this comment.
If no changes were made you will still fire the notification no? Also, let's make sure we have a test for it.
src/commands.rs
Outdated
| }); | ||
| } | ||
|
|
||
| redis_key.apply_changes(ctx, "json.arrinsert")?; |
There was a problem hiding this comment.
Same comment here, you will fire notification anyway no? And also, let's make sure we have a test for it.
src/commands.rs
Outdated
| _ => RedisValue::Null, | ||
| }); | ||
| } | ||
| redis_key.apply_changes(ctx, "json.arrappend")?; |
src/commands.rs
Outdated
| _ => RedisValue::Null, | ||
| }); | ||
| } | ||
| redis_key.apply_changes(ctx, "json.strappend")?; |
There was a problem hiding this comment.
Notification + test
| None => RedisValue::Null, | ||
| }); | ||
| } | ||
| if need_notify { |
src/commands.rs
Outdated
| _ => None, | ||
| }); | ||
| } | ||
| redis_key.apply_changes(ctx, cmd)?; |
There was a problem hiding this comment.
notification + test
| newline: Option<&str>, | ||
| space: Option<&str>, | ||
| ) -> Result<RedisValue, Error> { | ||
| if paths.len() > 1 { |
There was a problem hiding this comment.
If this is the new API I think its better to behave the same on single path and multi path, i.e always return an array (worst case it will be an array of size 1), @emmanuel-keller WDYT?
* Multi path: del - add test and doc * WIP [skip ci] Multi path: get multi value and multi path * Multi path: fix multi value/path (not yet resp array) * Multi path: mget multi value/path and test fixes * Multi path: numincrby nummultby numpowby * Multi path: get fails with legacy path * Multi path: Try using jsonpath lib api select_values_with_paths * Multi path: Workaround without jsonpath lib api select_values_with_paths * Multi path: strappend strlen * Multi path: arrappend * Multi path: arrinsert * Multi path: arrlen * Multi path: arrpop * Multi path: arrtrim * Multi path: avoid crash in arrtrim (fix ArrayIndex normalize) * Multi path: objkeys * Multi path: objlen and type commands * Multi path: toggle command * Multi path: debug memory commands * Multi path: debug memory fix test * Multi path: json.type without get_type_name api * Multi path: json.resp * Multi path: temporarily build macos *on-any-branch * Multi path: restore legacy backward compatibility for numXby strlen strappend * Multipath - merge from temp PR #487
|
Not needed (this PR was needed for early integration testing. Individual PRs were merged) |
#473
#477