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

Saxy.XML.element/3 and a non-list third argument results in a Dialyzer error #111

Closed
thbar opened this issue Jul 13, 2022 · 1 comment
Closed

Comments

@thbar
Copy link
Contributor

thbar commented Jul 13, 2022

On a project which uses Saxy.XML, I get Dialyzer errors with this code:

element("siri:StopVisitTypes", [], "all")

The code itself works fine.

I did a bit of digging to understand why Dialyzer reports an error, here are my findings!

The Saxy.XML code does support the call I wrote above indeed, here:

saxy/lib/saxy/xml.ex

Lines 71 to 81 in 98e2c9e

def element(name, attributes, children) when not is_nil(name) and is_list(children) do
{
to_string(name),
attributes(attributes),
children(children)
}
end
def element(name, attributes, child) when not is_nil(name) do
element(name, attributes, [child])
end

The typespecs, on the other hand, appear to require a list:

saxy/lib/saxy/xml.ex

Lines 24 to 30 in 98e2c9e

@type element() :: {
name :: String.t(),
attributes :: [{key :: String.t(), value :: String.t()}],
children :: [content]
}
@type content() :: element() | characters() | cdata() | ref() | comment() | String.t()

For now I'm turning my "all" into ["all"] to fix the error, but I wonder if we could modify the typespecs in Saxy to match the allowed behaviour here?

Thanks!

thbar added a commit to etalab/transport-site that referenced this issue Jul 13, 2022
thbar added a commit to etalab/transport-site that referenced this issue Jul 21, 2022
* Prepare todos for next round

* Add TODO

* Add simple POST mock server

* Add POST route to support SIRI

* Parse body as XML

* Extract testing code before reuse

* Format code

* Save live replacement of requestor ref via DOM model

* Make line_refs optional

Without line refs, everything is going to be returned (as I learned).

* Rebuild body from unmodified request for now

This is useful to print it, and I will work on requestor_ref replacement right after.

* Add TODO

* Replace GET test by POST for SIRI

* Move code to lib

* Add reproduction & fix for crash seen during my real-life testing

* Remove left-over

* Remove left-over

* Replace requestor_ref in query

* Adapt code

* Fix test

* Remove TODO

* Add support for POST in Finch wrapper

* Make a first query

* Add moduledoc

* Add relevant bits

* Fix more credo warnings

* Add helpful logger

* Improve SIRI CLI usage for myself

* Add helper to retrieve specific HTTP header

* Implement gzip decompression

We'll need to remove sensitive information (e.g. requestor_ref in particular), so this step is useful.

* Fix typo

* Add tooling to run batch of queries on all resources

* Support HTTP headers for SIRI

This is helpful to avoid HTTP 415 return (etalab/transport-private#5 (comment)) on CARENE.

* Remove completed & irrelevant todos

* Replace TODOs by #2476

* Add a comment pointing to #2476 so that things do not get lost

* Remove TODO

Removing this call may be required later when handling #1817

* Bubble up Mox/ExUnit exceptions

Without this, it is hard to write tests.

* Implement first fully POST SIRI test

* Return HTTP 405 for inadequate calls

POST on a GTFS-RT item and GET on a SIRI item are now forbidden for clarity.

I have added tests for this.

* Add todo

* Remove now unused code

* Fix incorrect tests & add notes for context

* Refactor method to extract seen requestor refs

This is going to be useful to verify the input payload from the controller.

* Update doc

* Start adapting code for requestor ref verification

* Return 403 Forbidden on incorrect incoming requestor_ref

* Move public requestor ref to config

* Delete mock_server.exs

* Try to fix Dialyzer issue

* Apply fix for Dialyzer

See qcam/saxy#111

* Revert "Try to fix Dialyzer issue"

This reverts commit b27a1fe.

* Add test about namespace & improve doc

* Use shorter form

Thanks @fchabouis for the idea

* Improve naming

* Add typespec

* Extract function for clarity & add tests

* Fix credo error

* Add a bit of typing for clarity

* Add spec to ensure we cover HEAD automatically for GTFS-RT

* Move code to single-line

* Extract method to improve understanding of code

* Always include XML version (1.0)

While the prolog is optional and 1.0 is the default, given the differences https://www.w3.org/TR/2006/REC-xml11-20060816/#sec-xml11 and the legacy servers we may target, we have decided to be explicit.

Co-authored-by: Francis Chabouis <fchabouis@gmail.com>

* Mix format

* Make prolog management more explicit

* Make test a bit more clear with variable on headers

* Add SIRI headers tests & improve GTFS-RT headers tests

Co-authored-by: Antoine Augusti <antoine.augusti@beta.gouv.fr>
Co-authored-by: Francis Chabouis <fchabouis@gmail.com>
@qcam
Copy link
Owner

qcam commented Aug 10, 2022

@thbar I think it's a spec bug. It should be fixed via be744ff

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

No branches or pull requests

2 participants