Skip to content

Commit

Permalink
replace null separator in apq cache key with : to match convention (#…
Browse files Browse the repository at this point in the history
…4886)

This PR conforms the apq cache key to follow redis convention. This
helps when using redis clients to properly display keys in nested form.

query plan cache key was fixed in a similar pr:
#4583

Co-authored-by: Geoffroy Couprie <geo.couprie@gmail.com>
  • Loading branch information
tapaderster and Geal committed Apr 2, 2024
1 parent 349bab8 commit 2a4142e
Show file tree
Hide file tree
Showing 3 changed files with 11 additions and 5 deletions.
7 changes: 7 additions & 0 deletions .changesets/fix_apq_cache_key_to_match_redis_convention.md
@@ -0,0 +1,7 @@
### Replaces null separator in apq cache key with : to match redis convention

This PR conforms the apq cache key to follow redis convention. This helps when using redis clients to properly display keys in nested form.

query plan cache key was fixed in a similar pr: #4583

By [@tapaderster](https://github.com/tapaderster) in https://github.com/apollographql/router/pull/4886
2 changes: 1 addition & 1 deletion apollo-router/src/services/layers/apq.rs
Expand Up @@ -158,7 +158,7 @@ fn query_matches_hash(query: &str, hash: &[u8]) -> bool {
}

fn redis_key(query_hash: &str) -> String {
format!("apq\0{query_hash}")
format!("apq:{query_hash}")
}

pub(crate) fn calculate_hash_for_query(query: &str) -> String {
Expand Down
7 changes: 3 additions & 4 deletions apollo-router/tests/integration/redis.rs
Expand Up @@ -165,7 +165,7 @@ mod test {
let query_hash = "4c45433039407593557f8a982dafd316a66ec03f0e1ed5fa1b7ef8060d76e8ec";

client
.del::<String, _>(&format!("apq\x00{query_hash}"))
.del::<String, _>(&format!("apq:{query_hash}"))
.await
.unwrap();

Expand Down Expand Up @@ -196,8 +196,7 @@ mod test {
res.errors.first().unwrap().message,
"PersistedQueryNotFound"
);

let r: Option<String> = client.get(&format!("apq\x00{query_hash}")).await.unwrap();
let r: Option<String> = client.get(&format!("apq:{query_hash}")).await.unwrap();
assert!(r.is_none());

// Now we register the query
Expand All @@ -222,7 +221,7 @@ mod test {
assert!(res.data.is_some());
assert!(res.errors.is_empty());

let s: Option<String> = client.get(&format!("apq\x00{query_hash}")).await.unwrap();
let s: Option<String> = client.get(&format!("apq:{query_hash}")).await.unwrap();
insta::assert_display_snapshot!(s.unwrap());

// we start a new router with the same config
Expand Down

0 comments on commit 2a4142e

Please sign in to comment.