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

Internal representation #29

Merged
merged 8 commits into from
Dec 30, 2024
Merged

Conversation

kevinschweikert
Copy link
Collaborator

@kevinschweikert kevinschweikert commented Dec 24, 2024

Hey @derekkraan, Merry Christmas! 🎁

I've got a pretty big present for you! 😄

the past few weeks i was working on this refactoring. There are no changes to the public API..just some Bugfixes and ordering of the CLI flags of cURL have been changed. Our test suite ist still the same with the same public functions.

The most important bit is the new module CurlReq.Request where we define a generic struct and helper function to modify that struct to represent a generic HTTP request. It also defines a behaviour (encode/2,decode/2) which the Curl and Req modules implement. They are just a refactor from the Main and Macro module.

The biggest benefit of this change would be, to set us up nicely to generate cURL commands from other HTTP clients. We would've just have to implement the behaviours and parse the request struct into our generic request struct.
Further, we seperate the cURL representation from the req representation which makes parsing so much easier because we don't have to think about req internals when parsing the cURL string and the other way round.
This makes processing the information simple, like getting auth or encoding information from the parsed headers and storing them in different field in the CurlReq.Request struct instead of just the generic header.

Also this refactoring found some existing bugs and added some enhancements. It is best schon in the following test:

  • fix: compression and encoding steps were not set
  • enhancement: body gets encoded into the correct req option (from body to json field)

BEFORE:

test "multiple headers with body" do
      assert ~CURL(curl -H "accept-encoding: gzip" -H "authorization: Bearer 6e8f18e6-141b-4d12-8397-7e7791d92ed4:lon" -H "content-type: application/json" -H "user-agent: req/0.4.14" -d "{\"input\":[{\"leadFormFields\":{\"Company\":\"k\",\"Country\":\"DZ\",\"Email\":\"k\",\"FirstName\":\"k\",\"Industry\":\"CTO\",\"LastName\":\"k\",\"Phone\":\"k\",\"PostalCode\":\"1234ZZ\",\"jobspecialty\":\"engineer\",\"message\":\"I would like to know if Roche delivers to The Netherlands.\"}}],\"formId\":4318}" -X POST "https://example.com/rest/v1/leads/submitForm.json") ==
               %Req.Request{
                 method: :post,
                 url: URI.parse("https://example.com/rest/v1/leads/submitForm.json"),
                 headers: %{
                   "accept-encoding" => ["gzip"],
                   "content-type" => ["application/json"],
                   "user-agent" => ["req/0.4.14"]
                 },
                 registered_options: MapSet.new([:auth]),
                 options: %{auth: {:bearer, "6e8f18e6-141b-4d12-8397-7e7791d92ed4:lon"}},
                 current_request_steps: [:auth],
                 request_steps: [auth: &Req.Steps.auth/1],
                 body:
                   "{\"input\":[{\"leadFormFields\":{\"Company\":\"k\",\"Country\":\"DZ\",\"Email\":\"k\",\"FirstName\":\"k\",\"Industry\":\"CTO\",\"LastName\":\"k\",\"Phone\":\"k\",\"PostalCode\":\"1234ZZ\",\"jobspecialty\":\"engineer\",\"message\":\"I would like to know if Roche delivers to The Netherlands.\"}}],\"formId\":4318}"
               }
    end

AFTER:

test "multiple headers with body" do
      assert ~CURL(curl -H "accept-encoding: gzip" -H "authorization: Bearer 6e8f18e6-141b-4d12-8397-7e7791d92ed4:lon" -H "content-type: application/json" -H "user-agent: req/0.4.14" -d "{\"input\":[{\"leadFormFields\":{\"Company\":\"k\",\"Country\":\"DZ\",\"Email\":\"k\",\"FirstName\":\"k\",\"Industry\":\"CTO\",\"LastName\":\"k\",\"Phone\":\"k\",\"PostalCode\":\"1234ZZ\",\"jobspecialty\":\"engineer\",\"message\":\"I would like to know if Roche delivers to The Netherlands.\"}}],\"formId\":4318}" -X POST "https://example.com/rest/v1/leads/submitForm.json") ==
               %Req.Request{
                 method: :post,
                 url: URI.parse("https://example.com/rest/v1/leads/submitForm.json"),
                 headers: %{
                   "user-agent" => ["req/0.4.14"]
                 },
                 registered_options: MapSet.new([:compressed, :auth, :json]),
                 options: %{
                   compressed: true,
                   auth: {:bearer, "6e8f18e6-141b-4d12-8397-7e7791d92ed4:lon"},
                   json: %{
                     "formId" => 4318,
                     "input" => [
                       %{
                         "leadFormFields" => %{
                           "Company" => "k",
                           "Country" => "DZ",
                           "Email" => "k",
                           "FirstName" => "k",
                           "Industry" => "CTO",
                           "LastName" => "k",
                           "Phone" => "k",
                           "PostalCode" => "1234ZZ",
                           "jobspecialty" => "engineer",
                           "message" =>
                             "I would like to know if Roche delivers to The Netherlands."
                         }
                       }
                     ]
                   }
                 },
                 current_request_steps: [:compressed, :auth, :encode_body],
                 request_steps: [
                   compressed: &Req.Steps.compressed/1,
                   auth: &Req.Steps.auth/1,
                   encode_body: &Req.Steps.encode_body/1
                 ]
               }
    end

I have some minor TODOs but i wanted to show you this PR and get some early thought on it

  • implement flavor option when generating the cURL command string
  • clean up some logic
  • add docs to curl and req modules

Closes #25

@kevinschweikert kevinschweikert force-pushed the feature/curl-internal-representation branch from 5ed8f14 to 8f551d8 Compare December 24, 2024 15:47
@kevinschweikert kevinschweikert force-pushed the feature/curl-internal-representation branch from 8f551d8 to ece1b5f Compare December 24, 2024 15:49
@kevinschweikert kevinschweikert marked this pull request as ready for review December 24, 2024 16:14
@derekkraan
Copy link
Owner

Hey Kevin,

I didn't know we had ambitions to support other HTTP clients, but sounds good to me!

@derekkraan derekkraan merged commit 2fcfd99 into main Dec 30, 2024
7 checks passed
@derekkraan derekkraan deleted the feature/curl-internal-representation branch December 30, 2024 07:36
@derekkraan
Copy link
Owner

I was thinking that it would be good to have a matrix of supported options; currently we only support Req, but in the future if we end up supporting httpd, Tesla, HTTPoison etc, it would be useful to see that. And I think it would be useful today to see what we already support for Req. I'll add an issue.

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.

Build curl command as iolist
2 participants