Merge pull request #1761 from DimuthuMadushan/cache-update

Fix Invalid Cache Key Generation in Server Side Caching

ballerina-tests/tests/44_server_caches.bal

@@ -126,7 +126,7 @@ function dataProviderServerCacheOperationalLevel() returns map<[string, string[]
         "9": ["server_cache_with_unions_operational_level", ["server_cache_with_unions_1", "server_cache_with_unions_2", "server_cache_with_unions_1"], (), ["A", "B", "A"]],
         "10": ["server_cache_with_unions_operational_level", ["server_cache_with_unions_1", "server_cache_with_unions_2", "server_cache_with_unions_3"], (), ["A", "C", "A"]],
         "11": ["server_cache_eviction", ["server_cache_2", "server_cache_4", "server_cache_5", "server_cache_4"], (), ["B", "A", "C", "A"]],
-        "12": ["server_cache_with_inputs", ["server_cache_with_nullable_inputs_7", "server_cache_with_nullable_inputs_7"], (), ["B", "C"]],
+        "12": ["server_cache_with_inputs", ["server_cache_with_nullable_inputs_7", "server_cache_with_nullable_inputs_8"], (), ["B", "C"]],
         "13": ["server_cache_with_inputs", ["server_cache_with_empty_input_1", "server_cache_with_empty_input_2"], (), ["B", "D"]],
         "14": ["server_cache_with_partial_responses", ["server_cache_with_partial_reponses_1", "server_cache_with_partial_reponses_2", "server_cache_with_partial_reponses_1"], (), ["A", "B", "A"]],
         "15": ["server_cache_with_partial_responses", ["server_cache_with_partial_reponses_1", "server_cache_with_partial_reponses_2", "server_cache_with_partial_reponses_3"], (), ["A", "C", "A"]],
@@ -248,3 +248,27 @@ function dataProviderServerSideCacheWithDynamicResponse() returns map<[string, s
     };
     return dataSet;
 }
+
+@test:Config {
+    groups: ["server_cache"],
+    dataProvider: dataProviderServerCacheWithListInput
+}
+isolated function testServerCacheWithListInput(string documentFile, string[] resourceFileNames, json variables = (), string[] operationNames = []) returns error? {
+    string url = "http://localhost:9091/cache_with_list_input";
+    string document = check getGraphqlDocumentFromFile("server_cache_with_list_input");
+
+    foreach int i in 0..< resourceFileNames.length() {
+        json actualPayload = check getJsonPayloadFromService(url, document, variables, operationNames[i]);
+        json expectedPayload = check getJsonContentFromFile(resourceFileNames[i]);
+        assertJsonValuesWithOrder(actualPayload, expectedPayload);
+    }
+}
+
+function dataProviderServerCacheWithListInput() returns map<[string, string[], json, string[]]> {
+    map<[string, string[], json, string[]]> dataSet = {
+        "1": ["server_cache_with_list_input", ["server_cache_with_list_input_1", "server_cache_with_list_input_2", "server_cache_with_list_input_3", "server_cache_with_list_input_1"], (), ["A", "B", "G", "A"]],
+        "2": ["server_cache_with_list_input", ["server_cache_with_list_input_4", "server_cache_with_list_input_5", "server_cache_with_list_input_6", "server_cache_with_list_input_4"], (), ["D", "H", "E", "D"]],
+        "3": ["server_cache_with_list_input", ["server_cache_with_list_input_7", "server_cache_with_list_input_8", "server_cache_with_list_input_7"], (), ["F", "I", "F"]]
+    };
+    return dataSet;
+}

ballerina-tests/tests/resources/documents/server_cache_with_list_input.graphql

@@ -0,0 +1,63 @@
+query A {
+    users1(ids: [1]) {
+        id
+        name
+        age
+    }
+}
+
+query B {
+    users1(ids: [2]) {
+        id
+        name
+        age
+    }
+}
+
+query C {
+    users1(ids: [3]) {
+        id
+        name
+        age
+    }
+}
+
+query D {
+    users2(ids: [[1], [3], [2]]) {
+        id
+        name
+        age
+    }
+}
+
+query E {
+    users2(ids: [[1], [2], [3]]) {
+        id
+        name
+        age
+    }
+}
+
+query F {
+    cities(addresses: [{number: "1", street: "Main St", city: "London"}])
+}
+
+mutation G {
+    updateUser(id: 1, name: "Shane", age: 30) {
+        id
+        name
+        age
+    }
+}
+
+mutation H {
+    updateUser(id: 1, name: "Rock", age: 22) {
+        id
+        name
+        age
+    }
+}
+
+query I {
+    cities(addresses: [{number: "2", street: "Main St", city: "london"}])
+}

ballerina-tests/tests/resources/expected_results/server_cache_with_list_input_1.json

@@ -0,0 +1,11 @@
+{
+    "data": {
+        "users1": [
+            {
+                "id": 1,
+                "name": "John",
+                "age": 25
+            }
+        ]
+    }
+}

ballerina-tests/tests/resources/expected_results/server_cache_with_list_input_2.json

@@ -0,0 +1,11 @@
+{
+    "data": {
+        "users1": [
+            {
+                "id": 2,
+                "name": "Jessie",
+                "age": 17
+            }
+        ]
+    }
+}

ballerina-tests/tests/resources/expected_results/server_cache_with_list_input_3.json

@@ -0,0 +1,9 @@
+{
+    "data": {
+        "updateUser": {
+            "id": 1,
+            "name": "Shane",
+            "age": 30
+        }
+    }
+}

ballerina-tests/tests/resources/expected_results/server_cache_with_list_input_4.json

@@ -0,0 +1,11 @@
+{
+    "data": {
+        "users2": [
+            {
+                "id": 1,
+                "name": "Shane",
+                "age": 30
+            }
+        ]
+    }
+}

ballerina-tests/tests/resources/expected_results/server_cache_with_list_input_5.json

@@ -0,0 +1,9 @@
+{
+    "data": {
+        "updateUser": {
+            "id": 1,
+            "name": "Rock",
+            "age": 22
+        }
+    }
+}

ballerina-tests/tests/resources/expected_results/server_cache_with_list_input_6.json

@@ -0,0 +1,11 @@
+{
+    "data": {
+        "users2": [
+            {
+                "id": 1,
+                "name": "Rock",
+                "age": 22
+            }
+        ]
+    }
+}

ballerina-tests/tests/resources/expected_results/server_cache_with_list_input_7.json

@@ -0,0 +1,5 @@
+{
+    "data": {
+        "cities": ["London"]
+    }
+}

ballerina-tests/tests/resources/expected_results/server_cache_with_list_input_8.json

@@ -0,0 +1,5 @@
+{
+    "data": {
+        "cities": ["London", "london"]
+    }
+}

ballerina-tests/tests/resources/expected_results/server_cache_with_nullable_inputs_7.json

@@ -1,5 +1,5 @@
 {
-    "data": { 
+    "data": {
         "status": ["dead", "alive"]
     }
 }

ballerina-tests/tests/resources/expected_results/server_cache_with_nullable_inputs_8.json

@@ -0,0 +1,5 @@
+{
+    "data": {
+        "status": ["alive", "alive"]
+    }
+}

ballerina-tests/tests/test_services.bal

@@ -3000,3 +3000,47 @@ service /dynamic_response on basicListener {
         return self.user;
     }
 }
+
+@graphql:ServiceConfig {
+    cacheConfig: {
+        maxAge: 600
+    }
+}
+service /cache_with_list_input on basicListener {
+    private User user1 = {id: 1, name: "John", age: 25};
+    private User user2 = {id: 2, name: "Jessie", age: 17};
+    private Address[] addresses = [];
+
+    resource function get users1(int[] ids) returns User[] {
+        if ids.length() == 1 && ids[0] == 1 {
+            return [self.user1];
+        } else if ids.length() == 1 && ids[0] == 2 {
+            return [self.user2];
+        }
+        return [self.user1, self.user2];
+    }
+
+    resource function get users2(int[][] ids) returns User[] {
+        if ids.length() == 3 && ids[0][0] == 1 {
+            return [self.user1];
+        } else if ids.length() == 3 && ids[0][0] == 2 || ids[0][0] == 3 {
+            return [self.user2];
+        }
+        return [self.user1, self.user2];
+    }
+
+    resource function get cities(Address[] addresses) returns string[] {
+        self.addresses.push(...addresses);
+        return self.addresses.map(address => address.city);
+    }
+
+    remote function updateUser(int id, string name, int age) returns User|error {
+        self.user1 = {id:id, name:name, age:age};
+        return self.user1;
+    }
+
+    remote function updateAddress(Address address) returns Address {
+        self.addresses.push(address);
+        return address;
+    }
+}

ballerina/common_utils.bal

@@ -519,11 +519,38 @@ public isolated function __addError(Context context, ErrorDetail errorDetail) {
 isolated function generateArgHash(parser:ArgumentNode[] arguments, string[] parentArgHashes = [],
                                   string[] optionalFields = []) returns string {
     any[] argValues = [...parentArgHashes, ...optionalFields];
-    argValues.push(...arguments.'map((arg) => arg.getValue()));
+    argValues.push(...arguments.'map((arg) => getValueArrayFromArgumentNode(arg)));
     byte[] hash = crypto:hashMd5(argValues.toString().toBytes());
     return hash.toBase64();
 }
 
+isolated function getValueArrayFromArgumentNode(parser:ArgumentNode argumentNode) returns anydata[] {
+    anydata[] valueArray = [];
+    if argumentNode.isVariableDefinition() {
+        valueArray.push(argumentNode.getVariableValue());
+    } else {
+        parser:ArgumentValue|parser:ArgumentValue[] argValue = argumentNode.getValue();
+        if argValue is parser:ArgumentValue[] {
+            foreach parser:ArgumentValue argField in argValue {
+                parser:ArgumentNode|Scalar? argFieldNode = argField;
+                if argFieldNode is parser:ArgumentNode {
+                    valueArray.push(getValueArrayFromArgumentNode(argFieldNode));
+                } else {
+                    valueArray.push(argFieldNode);
+                }
+            }
+        } else {
+            parser:ArgumentNode|Scalar? argFieldNode = argValue;
+            if argFieldNode is parser:ArgumentNode {
+                valueArray.push(getValueArrayFromArgumentNode(argFieldNode));
+            } else {
+                valueArray.push(argFieldNode);
+            }
+        }
+    }
+    return valueArray;
+}
+
 isolated function getNullableFieldsFromType(__Type fieldType) returns string[] {
     string[] nullableFields = [];
     __Field[]? fields = unwrapNonNullype(fieldType).fields;

ballerina/response_generator.bal

@@ -153,7 +153,8 @@ class ResponseGenerator {
         __Type fieldType = getFieldTypeFromParentType(self.fieldType, self.engine.getSchema().types, fieldNode);
         (string|int)[] clonedPath = self.path.clone();
         clonedPath.push(fieldNode.getAlias());
-        Field 'field = new (fieldNode, fieldType, path = clonedPath, fieldValue = fieldValue);
+        Field 'field = new (fieldNode, fieldType, path = clonedPath, fieldValue = fieldValue,
+                            cacheConfig = self.cacheConfig, parentArgHashes = self.parentArgHashes);
         self.context.resetInterceptorCount();
         return self.engine.resolve(self.context, 'field);
     }

ballerina/tests/09_cache_utils.bal

@@ -26,7 +26,7 @@ function testCacheUtils() returns error? {
     Field 'field = getField(fields[0], Person, ["person"], {maxAge: 10});
     test:assertTrue('field.isCacheEnabled());
     test:assertEquals('field.getCacheMaxAge(), 10d);
-    test:assertEquals('field.getCacheKey(), "person.TYoV48w1dQ8BbOFaQ5N2IA==");
+    test:assertEquals('field.getCacheKey(), "person.Jq9sXPlesvC6Q7cU5RPkZA==");
 }
 
 @test:Config {
@@ -38,7 +38,7 @@ function testCacheConfigInferring() returns error? {
     Field 'field = getField(fields[0], Person, ["person"], {maxAge: 10});
     test:assertTrue('field.getSubfields() is Field[]);
     Field[] subfields = <Field[]>'field.getSubfields();
-    string[] expectedCacheKey = ["person.name.11FxOYiYfpMxmANj4kGJzg==", "person.address.t1JP49rdVkuozxi8sSh+bg=="];
+    string[] expectedCacheKey = ["person.name.11FxOYiYfpMxmANj4kGJzg==", "person.address.nj4v+q6cUjv3W/MbZdNQXg=="];
     foreach int i in 0..<subfields.length() {
         test:assertTrue(subfields[i].isCacheEnabled());
         test:assertEquals(subfields[i].getCacheMaxAge(), 10d);