Hotfix/upgrade deps - #123
Closed
manuelr-dev wants to merge 10 commits into
Closed
Conversation
`Wsdl.parse/3` hands the SweetXml-parsed document to its helpers, but their
specs all declared `String.t()`. Dialyzer reads that as a contract nothing can
satisfy — `wsdl.ex:212 invalid_contract` on get_protocol_namespace — and from
there `parse/3` has no local return and a dozen helpers below it are reported
as unreachable. Eighteen warnings, all from one mistaken description.
SweetXml's xpath/2 takes either the raw document or a parsed xmlElement, and
Soap passes both, so `Soap.xml()` names that union and the helpers now use it.
`Xsd.parse_xsd/1` keeps `String.t()`, which is what it actually receives; only
its return needed widening, since the function has a `catch` clause answering
`{:error, err}` that the spec omitted.
None of this changes compiled code — specs and one type alias.
Alongside, so the toolchain runs clean on current Elixir:
- credo 1.6 crashed outright on Elixir 1.17 while tokenising, so the
dev-and-test dependencies move to their latest (credo 1.7.19,
dialyxir 1.4.7, ex_doc 0.40.3, excoveralls 0.18.5, mock 0.3.9), and
sweet_xml and xml_builder follow. httpoison widens to allow 3.0 as well;
the suite passes against it.
- the one issue credo then reported, a pattern-match argument written the
opposite way round to the rest of the codebase, is aligned.
- two request fixtures pinned an element order that current Elixir no longer
produces. Upstream already corrected one of them after this release; the
SOAP 1.2 fixture gets the same treatment.
The dialyzer fix follows the approach taken in instinctscience/soap, which
introduced the Soap.xml() type for the same reason.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`Soap.Request.get_http_client/0` already let a caller swap the HTTP client, but it was not an abstraction: the call sites decided what came back by matching HTTPoison's own structs, in xsd.ex, wsdl.ex and Soap.handle_response/1. A replacement could not be an adapter, only an impersonation. Requests now go through `Soap.Request` — `call/5` as before, plus `get/3` and `get!/3` for the document fetches that previously reached for the client themselves — and each answers `Soap.Response`. The struct already existed and already had the right shape; the conversion has moved from Soap.call/5 down to the seam, so it applies to every request rather than only to that one. Existing configurations keep working. Only the HTTPoison shapes are converted; a client answering Soap.Response is passed through untouched, as is anything else, which is what the test double returning a bare body relies on. The public `@spec` for Soap.call/5 states the return type rather than `any()`, and `get_http_client/0` no longer names HTTPoison in its own spec. Options are still forwarded verbatim, so their names remain the client's own — `recv_timeout` against the default HTTPoison is a hackney option. That is now said out loud in the docs instead of implied, and naming the options Soap honours is left to the Req work, where it is testable. Closes #2. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The swappable HTTP client existed mainly so tests could inject a double. Req
ships that itself — `Req.Test` stubs the transport — so the seam stopped paying
for itself the moment the client changed. `get_http_client/0` and the
`:globals[:http_client]` config are gone, along with the `mock` dependency and
every `with_mock HTTPoison` in the suite.
What replaces it is narrower: `config :soap, req_options: [...]` sets Req
options for every request, which is also where a test stub goes. Per-call
options still win over it.
Retries are off by default. Req retries transport errors and some statuses out
of the box, HTTPoison did not, and a repeated SOAP call is a repeated command
rather than a repeated read. A caller who wants them passes `retry:`.
Option names are Req's now, and Req rejects the ones it does not know rather
than ignoring them — `recv_timeout` raises with a suggestion of
`receive_timeout`. Callers passing hackney names will hear about it at the
first request instead of silently running on defaults. The redirect options
these modules set themselves are translated: `follow_redirect`/`max_redirect`
become `redirect`/`max_redirects`.
Two defects surfaced while covering the paths the old suite left open:
- `Wsdl.parse_from_file/2` matched `{:ok, wsdl} = File.read(path)`, so a
missing file raised MatchError instead of reporting. `Xsd.parse_from_file/1`
has always reported it, and both read the same kind of path.
- `operations/1` answers maps carrying `:name`, not strings. Its spec said
`nonempty_list(String.t())` and its example showed strings.
Coverage is 91.7%, with `soap.ex`, `request.ex`, `response.ex`, `exceptions.ex`,
`headers.ex` and `type.ex` at 100%. The suite gained the sad paths that were
missing: a fault reaching the fault parser through `Response.parse/2` rather
than only through the parser directly, a missing file on both readers, a
transport failure on `get/3` and `get!/3`, and the retry default with its
override.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`elixir: "~> 1.9"` was not true: req, plug, finch, mint and ex_doc all require
1.15, so that is the floor and mix.exs now says so. Verified by running the
whole chain on 1.15.8, 1.17.3 and 1.18.4 — format, warnings-as-errors, credo,
dialyzer and the suite are clean on each.
The CI matrix said 1.13–1.15 on OTP 25–26, which is why none of the three
defects fixed in the last two changes were ever caught: credo 1.6 crashed
outright on 1.17, dialyzer reported eighteen warnings, and three tests failed.
It now lists only pairs both projects support, so it needs no exclusions.
Split in two, because dialyzer across five rows is slow and buys little:
- compatibility.yml (was elixir.yml) builds and runs the suite across the
supported range
- quality.yml enforces the chain once, on 1.17: format, warnings-as-errors,
`credo --strict --all`, dialyzer, and `mix test --cover`, which fails below
90% through minimum_coverage in coveralls.json. Coverage is 91.7% today
Both now trigger on pull_request as well as push — the workflows only ever ran
on push, so a PR from a fork was never checked. Cache keys gained mix.exs
alongside mix.lock, since dependencies and dialyzer configuration both live
there and a PLT built before a change to it is the wrong PLT; the restore-keys
were identical to the keys, which made them no-ops, and are now prefixes.
The README badges pointed at upstream, including a Travis one for a service
that no longer exists. They point here now, and the two that nothing backs are
gone: coveralls, since no workflow posts to it, and the Hex ones, since this
fork is not published. The header says what it is and how to depend on it.
Closes #4.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The usage section documented the published package: it told a reader to add
`{:soap, "~> 1.0"}`, which fetches upstream rather than this. It now says what
the fork is, how it differs, and how to depend on it — by ref, never by branch.
Every example was checked against the code rather than carried over:
- response headers are lowercase now, because that is how Req delivers them;
the sample output said `Cache-Control`. A test pins the pair shape and the
request_url alongside it
- `operations/1` really does answer maps with `:name` and `:soap_action`, so
that example was right and the moduledoc that contradicted it was wrong
- the application-list section was pre-1.4 advice and is gone; Mix starts the
dependency
Three things the old README never said and a caller needs. A SOAP fault is a
successful exchange carrying an error, so it arrives as `{:ok, response}` with
a 4xx or 5xx status while `{:error, reason}` means the request never completed
— there is now a worked example handling both. `Soap.Response.parse/1` reads
the status to pick its parser. And `req_options` configures every request,
which is also how a test stubs the transport, so the testing section shows
`Req.Test` rather than leaving a reader to mock a client module that no longer
exists.
LICENSE.md carried the MIT text with no copyright line at all. Both holders are
named there and in the README.
Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
`@source_url` was upstream's repository, so ex_doc would send a reader to code that is not this, and the Changelog link went to hexdocs for a package this fork does not publish. Both now point here, with upstream kept as its own link rather than mistaken for the source. `source_ref` follows master, since the fork carries no version tags of its own. Same reason the README no longer claims documentation on HexDocs: it is not ours to publish. The two hexdocs links left are Req's, which is a dependency and does have documentation there. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Markdown folds a single newline into a space, so the two holders ran together on one line in both files. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
The PLT cache has never done anything. The workflow caches a directory and `mkdir -p`s it, but the project has no `dialyzer:` configuration, so dialyxir writes its PLTs into `_build` — which CI does not keep. Every run has rebuilt them from scratch, once per matrix row. `plt_local_path` and `plt_core_path` now name a directory of their own, so the cache has something to hold. Not `priv/`, which belongs to the application and is read at runtime through `:code.priv_dir/1`; a build artifact does not go there even in a library that ships no priv assets. Locally the dialyzer step goes from 26.6s to 1.3s once the PLTs are present. On a two-core runner the gap is wider, and it was being paid on every push. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
Two findings on the workflows, both fair. CodeQL: neither declared `permissions`, so the jobs ran with whatever the default token grants. Nothing here writes to the repository — it builds, it checks, it reports through the exit status — so both are `contents: read`. Semgrep: every action was referenced by a mutable tag. `@v4` and `@v1` can be repointed by whoever owns the action, which is how the trivy-action and kics-github-action compromises worked. All three are pinned to full commit SHAs, with the version they correspond to alongside so a reader can still see what is in use: actions/checkout v7.0.1 3d3c42e5aac5ba805825da76410c181273ba90b1 actions/cache v6.1.0 55cc8345863c7cc4c66a329aec7e433d2d1c52a9 erlef/setup-beam v1.24.1 54075bcc5e249e4758d363f27d099f55d843f124 Worth noting the tags were also stale: checkout and cache were three and two major versions behind. Co-Authored-By: Claude Opus 5 <noreply@anthropic.com>
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Sign up for free
to join this conversation on GitHub.
Already have an account?
Sign in to comment
1 participant
Add this suggestion to a batch that can be applied as a single commit.This suggestion is invalid because no changes were made to the code.Suggestions cannot be applied while the pull request is closed.Suggestions cannot be applied while viewing a subset of changes.Only one suggestion per line can be applied in a batch.Add this suggestion to a batch that can be applied as a single commit.Applying suggestions on deleted lines is not supported.You must change the existing code in this line in order to create a valid suggestion.Outdated suggestions cannot be applied.This suggestion has been applied or marked resolved.Suggestions cannot be applied from pending reviews.Suggestions cannot be applied on multi-line comments.Suggestions cannot be applied while the pull request is queued to merge.Suggestion cannot be applied right now. Please check back later.
No description provided.