Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

pactffi_pact_handle_write_file when used with plugins does not write 'transport' key to pact #458

Open
YOU54F opened this issue Jul 23, 2024 · 4 comments
Labels
bug Indicates an unexpected problem or unintended behavior

Comments

@YOU54F
Copy link
Member

YOU54F commented Jul 23, 2024

Expected - Calling pactffi_pact_handle_write_file will result in the same pact file as pactffi_write_file

Actual - Calling pactffi_pact_handle_write_file will result in a pact file, with a missing transport key in the plugin interaction

Reproducer - master...YOU54F:pact-reference:bug/pactffi_pact_handle_write_file_no_transport

The test skips sending a consumer request, and checking mismatches, and simply aims to serialise the file after starting the mock server. Sending a valid consumer request does not change the outcome

Uncomment pactffi_pact_handle_write_file and comment out pactffi_write_pact_file linw to see an error

#[test_log::test]
fn protobuf_consumer_test() {
  let consumer_name = CString::new("protobuf-consumer").unwrap();
  let provider_name = CString::new("protobuf-provider").unwrap();
  let pact_handle = pactffi_new_pact(consumer_name.as_ptr(), provider_name.as_ptr());
  pactffi_with_specification(pact_handle, PactSpecification::V4);
  let description = CString::new("a request to a plugin").unwrap();
  let interaction = pactffi_new_sync_message_interaction(pact_handle, description.as_ptr());
  let plugin_name = CString::new("protobuf").unwrap();
  pactffi_using_plugin(pact_handle, plugin_name.as_ptr(),null());

  let content_type = CString::new("application/grpc").unwrap();
  let proto_path: PathBuf = std::env::current_dir().unwrap().join("../../proto/area_calculator.proto");
  let json = json!({
    "pact:proto":  proto_path,
    "pact:proto-service": "Calculator/calculateOne",
     "pact:content-type": "application/protobuf",
     "request": {
       "rectangle": {
         "length": "matching(number, 3)",
         "width": "matching(number, 4)"
       }
     },
     "response": {
       "value": ["matching(number, 12)"]
     }
   });
  let body = CString::new(json.to_string()).unwrap();
  let address = CString::new("0.0.0.0").unwrap();
  let transport = CString::new("grpc").unwrap();

  pactffi_interaction_contents(interaction.clone(), InteractionPart::Request, content_type.as_ptr(), body.as_ptr());


  let port = pactffi_create_mock_server_for_transport(pact_handle.clone(), address.as_ptr(), 0, transport.as_ptr(), null());

  expect!(port).to(be_greater_than(0));

  let tmp = TempDir::new().unwrap();
  let tmp_path = tmp.path().to_string_lossy().to_string();
  let file_path = CString::new(tmp_path.as_str()).unwrap();
  pactffi_write_pact_file(port, file_path.as_ptr(), true);
  // Uncomment pactffi_pact_handle_write_file and comment out pactffi_write_pact_file to see an error
  // pactffi_pact_handle_write_file(pact_handle, file_path.as_ptr(), true);
  pactffi_cleanup_mock_server(port);
  pactffi_cleanup_plugins(pact_handle);

  let contents = fs::read_to_string(tmp_path + "/protobuf-consumer-protobuf-provider.json").unwrap();
  let json: Value = serde_json::from_str(&contents).unwrap();

  assert_eq!(json.get("interactions").unwrap()[0]["transport"],"grpc");
}

relevant code

@JP-Ellis
Copy link
Contributor

From the perspective of the FFI, I would first question whether we need to both function at all? Or should we deprecate one in favour of the other?

Having said that, if we do decide to keep both, I agree they should probably produce the same result.

Do you know which one is the more correct output? I'm guessing that pactffi_pact_handle_write_file is incorrect since, as you put it, it is missing the transport key?

@YOU54F
Copy link
Member Author

YOU54F commented Jul 24, 2024

I hadn't seen pactffi_pact_handle_write_file before, but it was used in pact-net and I have been looking at the work to incorporate plugins into pact-net.

I think it might be because its a message pact handle I get back from pactffi_new_sync_message_interaction and pactffi_pact_handle_write_file requires a pact handle.

`pact_ffi_write_pact looks correct to me (in the fact the transport key is written which allows plugins to be used on the verification side)

I don't think pactffi_with_specification is suitable for the message pact handle and aligns with some of the message handle confusion you and me have had recently

@JP-Ellis
Copy link
Contributor

Yeah, this definitely seems to link with #440.

From the recent work in Pact Python, I can confirm that it is possible to use message interactions without using MessagePactHandle and MessageHandle (using, instead PactHandle and InteractionHandle respectively).

@rholshausen rholshausen added the bug Indicates an unexpected problem or unintended behavior label Jul 31, 2024
@rholshausen
Copy link
Contributor

Found the root cause of the problem.

pactffi_write_pact_file calls pact_mock_server::write_pact_file which retrieves the plugin mock server Pact and then writes it out. The plugin mock server knows all about the transport being used and can set that field.

pactffi_pact_handle_write_file writes out the Pact stored with the handle. It is generic, and knows nothing about plugins and transports.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Indicates an unexpected problem or unintended behavior
Projects
None yet
Development

No branches or pull requests

3 participants