Entity caching: sort entity keys before hashing the key

Cargo.lock

@@ -4040,7 +4040,7 @@ source = "registry+https://github.com/rust-lang/crates.io-index"
 checksum = "fc2f4eb4bc735547cfed7c0a4922cbd04a4655978c09b54f1f7b228750664c34"
 dependencies = [
  "cfg-if",
- "windows-targets 0.48.5",
+ "windows-targets 0.52.6",
 ]
 
 [[package]]
@@ -6250,9 +6250,9 @@ dependencies = [
 
 [[package]]
 name = "serde_json_bytes"
-version = "0.2.4"
+version = "0.2.5"
 source = "registry+https://github.com/rust-lang/crates.io-index"
-checksum = "0ecd92a088fb2500b2f146c9ddc5da9950bb7264d3f00932cd2a6fb369c26c46"
+checksum = "a6a27c10711f94d1042b4c96d483556ec84371864e25d0e1cf3dc1024b0880b1"
 dependencies = [
  "ahash",
  "bytes",

Cargo.toml

@@ -53,7 +53,12 @@ apollo-smith = "0.15.0"
 async-trait = "0.1.77"
 hex = { version = "0.4.3", features = ["serde"] }
 http = "1.1.0"
-insta = { version = "1.38.0", features = ["json", "redactions", "yaml", "glob"] }
+insta = { version = "1.38.0", features = [
+    "json",
+    "redactions",
+    "yaml",
+    "glob",
+] }
 once_cell = "1.19.0"
 reqwest = { version = "0.11.0", default-features = false, features = [
     "rustls-tls",
@@ -69,7 +74,7 @@ serde_json = { version = "1.0.114", features = [
     "preserve_order",
     "float_roundtrip",
 ] }
-serde_json_bytes = { version = "0.2.4", features = ["preserve_order"] }
+serde_json_bytes = { version = "0.2.5", features = ["preserve_order"] }
 sha1 = "0.10.6"
 tempfile = "3.10.1"
 tokio = { version = "1.36.0", features = ["full"] }

apollo-router/src/plugins/cache/entity.rs

@@ -1240,6 +1240,7 @@ pub(crate) fn hash_additional_data(
     let repr_key = ByteString::from(REPRESENTATIONS);
     // Removing the representations variable because it's already part of the cache key
     let representations = body.variables.remove(&repr_key);
+    body.variables.sort_keys();
     digest.update(serde_json::to_vec(&body.variables).unwrap());
     if let Some(representations) = representations {
         body.variables.insert(repr_key, representations);
@@ -1359,9 +1360,11 @@ fn extract_cache_keys(
     Ok(res)
 }
 
-pub(crate) fn hash_entity_key(representation: &Value) -> String {
+/// Returns a hash for the given representation, independent of field order. Destructively sorts any objects in the input.
+pub(crate) fn hash_entity_key(representation: &mut Value) -> String {
     // We have to hash the representation because it can contains PII
     let mut digest = Sha256::new();
+    representation.sort_all_objects();
     digest.update(serde_json::to_string(&representation).unwrap().as_bytes());
     hex::encode(digest.finalize().as_slice())
 }
@@ -1612,3 +1615,27 @@ impl Ord for CacheKeyStatus {
         }
     }
 }
+
+#[cfg(test)]
+mod tests {
+    use super::*;
+
+    #[test]
+    fn test_hash_entity_key_ordering() {
+        let mut representations = serde_json_bytes::json!([{
+            "id1": "test",
+            "id2": "test2"
+        }]);
+        let first_hash_key = hash_entity_key(&mut representations);
+        let mut representations = serde_json_bytes::json!([{
+            "id2": "test2",
+            "id1": "test"
+        }]);
+        let second_hash_key = hash_entity_key(&mut representations);
+
+        assert_eq!(
+            first_hash_key, second_hash_key,
+            "these 2 hashes should be equals because ordering doesn't matter"
+        );
+    }
+}

apollo-router/src/plugins/cache/invalidation.rs

@@ -98,7 +98,7 @@ impl Invalidation {
         &self,
         redis_storage: &RedisCacheStorage,
         origin: &'static str,
-        request: &InvalidationRequest,
+        request: &mut InvalidationRequest,
     ) -> Result<u64, InvalidationError> {
         let key_prefix = request.key_prefix();
         let subgraph = request.subgraph_name();
@@ -167,7 +167,7 @@ impl Invalidation {
         let mut count = 0;
         let mut errors = Vec::new();
         let mut futures = Vec::new();
-        for request in requests {
+        for mut request in requests {
             let redis_storage = match self.storage.get(request.subgraph_name()) {
                 Some(s) => s,
                 None => continue,
@@ -181,7 +181,7 @@ impl Invalidation {
                 let start = Instant::now();
 
                 let res = self
-                    .handle_request(redis_storage, origin, &request)
+                    .handle_request(redis_storage, origin, &mut request)
                     .instrument(tracing::info_span!("cache.invalidation.request"))
                     .await;
 
@@ -230,7 +230,8 @@ pub(crate) enum InvalidationRequest {
 }
 
 impl InvalidationRequest {
-    fn key_prefix(&self) -> String {
+    /// Compute a cache key prefix. For entity keys, this destructively sorts all objects.
+    fn key_prefix(&mut self) -> String {
         match self {
             InvalidationRequest::Subgraph { subgraph } => {
                 format!("version:{ENTITY_CACHE_VERSION}:subgraph:{subgraph}:*",)