Skip to content

chore!: remove deprecated server function and overhaul c examples - #502

Merged
JP-Ellis merged 5 commits into
masterfrom
chore/upgrade-mock-server-in-ffi
Nov 28, 2025
Merged

chore!: remove deprecated server function and overhaul c examples#502
JP-Ellis merged 5 commits into
masterfrom
chore/upgrade-mock-server-in-ffi

Conversation

@JP-Ellis

@JP-Ellis JP-Ellis commented Sep 5, 2025

Copy link
Copy Markdown
Member

This PR started with the intention of upgrading the version of the server in the FFI. I took the opportunity to remove deprecated functions at the same time, which then required an update of the C examples... and here we are now, with the server not upgraded (yet), but with completely overhauled C examples.

So in the end, this PR:

  • Removes the deprecated server functions from the FFI
  • Overhaul both consumer and provider C examples
  • Integrates these examples in CI to make sure they are tested

@JP-Ellis JP-Ellis self-assigned this Sep 5, 2025
@JP-Ellis
JP-Ellis force-pushed the chore/upgrade-mock-server-in-ffi branch 3 times, most recently from aa6e96a to c50ddc1 Compare September 9, 2025 06:59
This removes the two deprecated functions, and adapts the tests to make
use of the remaining non-deprecated function
`pactffi_create_mock_server_for_transport`; all in preparation to
upgrade the mock server dependency to version 2.

BREAKING CHANGE: The functions `pactffi_create_mock_server` and
    `pactffi_create_mock_server_for_pact` have been removed following
    their deprecation. Please use the newer
    `pactffi_create_mock_server_for_transport` function.

Signed-off-by: JP-Ellis <josh@jpellis.me>
@JP-Ellis
JP-Ellis force-pushed the chore/upgrade-mock-server-in-ffi branch 25 times, most recently from f2d082c to 7171f12 Compare November 13, 2025 10:21
@JP-Ellis
JP-Ellis force-pushed the chore/upgrade-mock-server-in-ffi branch 6 times, most recently from 7fe4407 to 40e343d Compare November 17, 2025 04:24
@JP-Ellis JP-Ellis changed the title chore!: upgrade mock server in FFI chore!: remove deprecated server function and overhaul c examples Nov 17, 2025
@JP-Ellis
JP-Ellis force-pushed the chore/upgrade-mock-server-in-ffi branch from 40e343d to db85c79 Compare November 17, 2025 04:30
@JP-Ellis
JP-Ellis requested a review from YOU54F November 17, 2025 04:31
@JP-Ellis
JP-Ellis marked this pull request as ready for review November 17, 2025 04:31
@JP-Ellis
JP-Ellis force-pushed the chore/upgrade-mock-server-in-ffi branch 4 times, most recently from 0953fb6 to 9493804 Compare November 17, 2025 09:24

@YOU54F YOU54F left a comment

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks great dude

Should we have error handling for any ffi function calls which may return an error?

For future, it would be great to add different protocol types on the consumer side, multiple transports on the provider side, both main types of provider verification (via url for webhooks from a consumer pact that has been published but requires verification, or via broker consumer version selectors for regular provider ci builds).

how to setup passing in env vars for broker selectors, and filter descriptions so users can consistently setup each library ( I think that should be in the core - relates to #250 ) - It might be a bit of a mixed bag, the verifier cli supports some env vars https://github.com/pact-foundation/pact-reference/tree/master/rust/pact_verifier_cli

I have support for some in pact ruby v2, pact ruby v1 used some.

All that is an aside from this PR though, great work. ship it!

Will have to just check we do a 0.5.0 release of the FFI and make a decent release note re: breaking changes just to let people know how to switch from the deprecated function to the new one.

Comment thread .github/workflows/build-ffi.yml
Comment thread c/provider/src/main.c Outdated
@JP-Ellis
JP-Ellis force-pushed the chore/upgrade-mock-server-in-ffi branch from 9493804 to 585b1c1 Compare November 17, 2025 21:48
@JP-Ellis

Copy link
Copy Markdown
Member Author

Should we have error handling for any ffi function calls which may return an error?

It's one of these things where a proper implementation should absolutely do this, but having error handling every where is also cumbersome and might detract from the main point of these. I'll make a mention of that though in the code/docs.

For future, it would be great to add different protocol types on the consumer side, multiple transports on the provider side, both main types of provider verification (via url for webhooks from a consumer pact that has been published but requires verification, or via broker consumer version selectors for regular provider ci builds).

Absolutely, but this PR already started as a detour 😅 This can be future work.

Will have to just check we do a 0.5.0 release of the FFI and make a decent release note re: breaking changes just to let people know how to switch from the deprecated function to the new one.

Yes, absolutely! I did put a not about that in the commit:

BREAKING CHANGE: The functions `pactffi_create_mock_server` and
    `pactffi_create_mock_server_for_pact` have been removed following
    their deprecation. Please use the newer
    `pactffi_create_mock_server_for_transport` function.

This can be fleshed out to explain how it should be used.

* Example usage:
* struct ResponseBuffer resp;
* response_buffer_init(&resp);
* // ... perform cURL operations that fill resp ...

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Is it 'fill resp' ? I think it's something like 'frees resp'

Copy link
Copy Markdown
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure I understand what you mean.

The example is showing that if you create a buffer with response_buffer_init, you then need to free it later with response_buffer_free. The commented out portion is just a placeholder where the user would (typically) fill the response.

Copy link
Copy Markdown
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Sorry, I misunderstood. I thought this comment is for the line below response_buffer_free(&resp);, but it isn't.

Comment thread c/consumer/include/curl_utils.h Outdated
I have overhauled the consumer example to:

- Use `CMake` and the Corrosion package to automatically find and build
  the FFI (including the header) and link to the necessary dependencies.
- Separate out the tests into individual files to help with the
  end-users' understanding of the tests.

The initial motivation was to remove the reliance on deprecated FFI
functions.

Signed-off-by: JP-Ellis <josh@jpellis.me>
@JP-Ellis
JP-Ellis force-pushed the chore/upgrade-mock-server-in-ffi branch from 585b1c1 to a06b954 Compare November 20, 2025 00:58
@JP-Ellis

Copy link
Copy Markdown
Member Author

Thanks Tien for the PR review! Hopefully it all made sense (though I know you're quite familiar with the FFI already 😅)

@JP-Ellis
JP-Ellis force-pushed the chore/upgrade-mock-server-in-ffi branch from a06b954 to 3da99cc Compare November 28, 2025 00:16
This is a complete redesign of the provider example, intended to build
on top of the consumer example. It (hopefully) provides a reference as
to how the FFI is to be used, on both consumer and provider sides.

Signed-off-by: JP-Ellis <josh@jpellis.me>
Signed-off-by: JP-Ellis <josh@jpellis.me>
Signed-off-by: JP-Ellis <josh@jpellis.me>
@JP-Ellis
JP-Ellis force-pushed the chore/upgrade-mock-server-in-ffi branch from 3da99cc to c6469a7 Compare November 28, 2025 00:37
@JP-Ellis
JP-Ellis merged commit 9f74b80 into master Nov 28, 2025
34 of 36 checks passed
@JP-Ellis
JP-Ellis deleted the chore/upgrade-mock-server-in-ffi branch November 28, 2025 01:49
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

None yet

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants