fix: query param ordering respected when creating v2 pacts

rust/pact_matching/tests/test_pact.json

@@ -14,7 +14,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       }

rust/pact_matching/tests/test_pact_camel_case_spec_version.json

@@ -14,7 +14,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       }

rust/pact_matching/tests/test_pact_matchers.json

@@ -14,7 +14,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       },

rust/pact_matching/tests/test_pact_matchers_old_format.json

@@ -14,7 +14,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       },

rust/pact_matching/tests/test_pact_no_bodies.json

@@ -14,7 +14,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s"
+      "query" : "q=p&r=s&q=p2"
     },
     "response" : {
       "status" : 200,

rust/pact_matching/tests/test_pact_no_metadata.json

@@ -14,7 +14,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       }

rust/pact_matching/tests/test_pact_no_spec_version.json

@@ -14,7 +14,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       }

rust/pact_matching/tests/test_pact_no_version.json

@@ -14,7 +14,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       }

rust/pact_matching/tests/test_pact_query_old_format.json

@@ -14,7 +14,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       }

rust/pact_matching/tests/v2_pact_query.json

@@ -11,7 +11,7 @@
     "request" : {
       "method" : "GET",
       "path" : "/",
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       }

rust/pact_models/src/pact.rs

@@ -630,7 +630,7 @@ mod tests {
           "headers" : {
             "testreqheader" : "testreqheadervalue"
           },
-          "query" : "q=p&q=p2&r=s",
+          "query" : "q=p&r=s&q=p2",
           "body" : {
             "test" : true
           }

rust/pact_models/src/query_strings.rs

@@ -120,17 +120,28 @@ pub fn parse_query_string(query: &str) -> Option<HashMap<String, Vec<Option<Stri
 
 /// Converts a query string map into a query string
 pub fn build_query_string(query: HashMap<String, Vec<Option<String>>>) -> String {
-  query.into_iter()
-    .filter(|(k, _)| !k.is_empty())
-    .sorted_by(|a, b| Ord::cmp(&a.0, &b.0))
-    .flat_map(|kv| {
-      kv.1.iter()
-        .map(|v| match v {
-          None => kv.0.clone(),
-          Some(s) => format!("{}={}", kv.0, encode_query(s))
-        })
-        .collect_vec()
+  // Collect all (key, index, value) triples to preserve the order of values
+  let mut entries = vec![];
+  for (key, values) in &query {
+    for (idx, value) in values.iter().enumerate() {
+      entries.push((idx, key, value));
+    }
+  }
+  // Sort by index to preserve the order of values across keys
+  entries.sort_by_key(|(idx, _, _)| *idx);
+
+  // Sort by index, then by key lexicographically for each index
+  entries.sort_by(|(idx_a, key_a, _), (idx_b, key_b, _)| {
+    idx_a.cmp(idx_b).then_with(|| key_a.cmp(key_b))
+  });
+
+  entries.into_iter()
+    .filter(|(_, k, _)| !k.is_empty())
+    .map(|(_, k, v)| match v {
+      None => k.clone(),
+      Some(s) => format!("{}={}", k, encode_query(s))
     })
+    .collect::<Vec<_>>()
     .join("&")
 }
 
@@ -202,6 +213,7 @@ mod tests {
   use rstest::rstest;
 
   use crate::query_strings::parse_query_string;
+use crate::query_strings::build_query_string;
 
   #[test]
   fn parse_query_string_test() {
@@ -288,4 +300,78 @@ mod tests {
     let result = super::build_query_string(map);
     assert_eq!(result, expected)
   }
+
+  #[test]
+  fn build_query_string_multiple_keys_and_values_ordered() {
+    let mut query = HashMap::new();
+    query.insert("pacticipant".to_string(), vec![Some("foo".to_string()), Some("bar".to_string())]);
+    query.insert("version".to_string(), vec![Some("1.2.3".to_string()), Some("4.5.6".to_string())]);
+    // The order must be: pacticipant=foo&version=1.2.3&pacticipant=bar&version=4.5.6
+    let result = build_query_string(query);
+    assert_eq!(result, "pacticipant=foo&version=1%2e2%2e3&pacticipant=bar&version=4%2e5%2e6");
+  }
+
+
+    // sends URL Querying broker at: http://127.0.0.1:61567/matrix?q[][pacticipant]=Foo&q[][version]=1.2.3&q[][pacticipant]=Bar&q[][latest]=true&q[][tag]=prod&latestby=cvpv
+    // pact saves url as latestby=cvpv&q[][latest]=true&q[][pacticipant]=Foo&q[][tag]=prod&q[][version]=1%2e2%2e3&q[][pacticipant]=Bar
+    // pact should save url as q[][pacticipant]=Foo&q[][version]=1%2e2%2e3&q[][pacticipant]=Bar&q[][latest]=true&q[][tag]=prod&latestby=cvpv
+    // pact-ruby saves q%5B%5D%5Bpacticipant%5D=Foo&q%5B%5D%5Bversion%5D=1.2.3&q%5B%5D%5Bpacticipant%5D=Bar&q%5B%5D%5Blatest%5D=true&q%5B%5D%5Btag%5D=prod&latestby=cvpv 
+
+
+      // .query_param("q[][pacticipant]", "Foo")
+      // .query_param("q[][version]", "1.2.3")
+      // .query_param("q[][pacticipant]", "Bar")
+      // .query_param("q[][latest]", "true")
+      // .query_param("q[][tag]", "prod")
+      // .query_param("latestby", "cvpv");
+
+
+    #[test]
+    fn build_query_string_matrix_query_with_multiple_q_params_and_latestby() {
+      // This test checks that the query string is built in the expected order for matrix queries
+      // with multiple q[][param] and a latestby param.
+      let mut query = HashMap::new();
+      query.insert("q[][pacticipant]".to_string(), vec![Some("Foo".to_string()), Some("Bar".to_string())]);
+      query.insert("q[][version]".to_string(), vec![Some("1.2.3".to_string())]);
+      query.insert("q[][latest]".to_string(), vec![Some("true".to_string())]);
+      query.insert("q[][tag]".to_string(), vec![Some("prod".to_string())]);
+      query.insert("latestby".to_string(), vec![Some("cvpv".to_string())]);
+      // The expected order is: q[][pacticipant]=Foo&q[][version]=1%2e2%2e3&q[][pacticipant]=Bar&q[][latest]=true&q[][tag]=prod&latestby=cvpv
+      let result = build_query_string(query);
+      assert_eq!(
+        result,
+        "q[][pacticipant]=Foo&q[][version]=1%2e2%2e3&q[][pacticipant]=Bar&q[][latest]=true&q[][tag]=prod&latestby=cvpv"
+      );
+    }
+
+
+    // sends URL Querying broker at: http://127.0.0.1:61517/matrix?q[][pacticipant]=Foo&q[][version]=1.2.4&q[][pacticipant]=Bar&q[][latest]=true&latestby=cvpv
+    // pact saves URL as latestby=cvpv&q[][latest]=true&q[][pacticipant]=Foo&q[][version]=1%2e2%2e4&q[][pacticipant]=Bar
+    // pact should save url as q[][pacticipant]=Foo&q[][version]=1%2e2%2e4&q[][pacticipant]=Bar&q[][latest]=true&latestby=cvpv
+    // pact-ruby saves q%5B%5D%5Bpacticipant%5D=Foo&q%5B%5D%5Bversion%5D=1.2.4&q%5B%5D%5Bpacticipant%5D=Bar&q%5B%5D%5Blatest%5D=true&latestby=cvpv
+
+    // .query_param("q[][pacticipant]", "Foo")
+    // .query_param("q[][version]", "1.2.4")
+    // .query_param("q[][pacticipant]", "Bar")
+    // .query_param("q[][latest]", "true")
+    // .query_param("latestby", "cvpv");
+
+
+  #[test]
+  fn build_query_string_matrix_query_with_multiple_q_params_and_latestby_different_version() {
+    // This test checks that the query string is built in the expected order for matrix queries
+    // with multiple q[][param] and a latestby param, with a different version value.
+    let mut query = HashMap::new();
+    query.insert("q[][pacticipant]".to_string(), vec![Some("Foo".to_string()), Some("Bar".to_string())]);
+    query.insert("q[][version]".to_string(), vec![Some("1.2.4".to_string())]);
+    query.insert("q[][pacticipant]".to_string(), vec![Some("Bar".to_string())]);
+    query.insert("q[][latest]".to_string(), vec![Some("true".to_string())]);
+    query.insert("latestby".to_string(), vec![Some("cvpv".to_string())]);
+    // The expected order is: q[][pacticipant]=Foo&q[][version]=1%2e2%2e4&q[][pacticipant]=Bar&q[][latest]=true&latestby=cvpv
+    let result = build_query_string(query);
+    assert_eq!(
+      result,
+      "q[][pacticipant]=Foo&q[][version]=1%2e2%2e4&q[][pacticipant]=Bar&q[][latest]=true&latestby=cvpv"
+    );
+  }
 }

rust/pact_models/src/request.rs

@@ -420,7 +420,7 @@ mod tests {
         "b".to_string() => vec![Some("3".to_string())]
     }), .. Request::default() };
     expect!(request.to_json(&PactSpecification::V2).to_string()).to(
-      be_equal_to(r#"{"method":"GET","path":"/","query":"a=1&a=2&b=3"}"#)
+      be_equal_to(r#"{"method":"GET","path":"/","query":"a=1&b=3&a=2"}"#)
     );
   }
 

rust/pact_models/src/sync_pact.rs

@@ -412,7 +412,7 @@ mod tests {
   use maplit::hashmap;
   use serde_json::json;
 
-  use crate::sync_pact::RequestResponsePact;
+  use crate::{PactSpecification, pact::Pact, sync_pact::RequestResponsePact};
 
   #[test_log::test]
   fn convert_from_v4_json() -> anyhow::Result<()> {
@@ -598,6 +598,63 @@ mod tests {
       "x-xss-protection".to_string() => vec!["1; mode=block".to_string()]
     }));
 
+    Ok(())
+  }
+  #[test_log::test]
+  fn convert_from_v4_to_v2_with_repeated_query_params_preserves_order() -> anyhow::Result<()> {
+    let pact_json = json!({
+      "consumer": {
+        "name": "convert_from_v4_to_v2_consumer"
+      },
+      "interactions": [
+        {
+          "description": "get participants",
+          "key": "abc123",
+          "pending": false,
+          "request": {
+            "method": "GET",
+            "path": "/participants",
+            "query": {
+              "pacticipant": [
+                "foo",
+                "bar"
+              ],
+              "version": [
+                "1.2.3",
+                "4.5.6"
+              ]
+            }
+          },
+          "response": {
+            "status": 200
+          },
+          "type": "Synchronous/HTTP"
+        }
+      ],
+      "metadata": {
+        "pactRust": {
+          "models": "1.0.4"
+        },
+        "pactSpecification": {
+          "version": "4.0"
+        }
+      },
+      "provider": {
+        "name": "convert_from_v4_to_v2_provider"
+      }
+    });
+
+    let pact = RequestResponsePact::from_json("test", &pact_json)?;
+    let v2_json = pact.to_json(PactSpecification::V2)?;
+
+    // Check that the query string is preserved in order in the output
+    let interactions = v2_json.get("interactions").unwrap().as_array().unwrap();
+    let req = interactions[0].get("request").unwrap();
+    let query_str = req.get("query").unwrap().as_str().unwrap();
+
+    // The order must be: pacticipant=foo&version=1.2.3&pacticipant=bar&version=4.5.6
+    expect!(query_str).to(be_equal_to("pacticipant=foo&version=1%2e2%2e3&pacticipant=bar&version=4%2e5%2e6"));
+
     Ok(())
   }
 }

rust/pact_verifier/tests/pact-two.json

@@ -15,7 +15,7 @@
       "headers" : {
         "testreqheader" : "testreqheadervalue"
       },
-      "query" : "q=p&q=p2&r=s",
+      "query" : "q=p&r=s&q=p2",
       "body" : {
         "test" : true
       }