Skip to content
This repository has been archived by the owner on Jul 9, 2021. It is now read-only.

sra-spec: Clients generated from SRA spec can't validate numeric inputs. #1727

Open
feuGeneA opened this issue Mar 24, 2019 · 1 comment
Open
Labels
pinned @0x/sra-spec Standard Relayer API Spec

Comments

@feuGeneA
Copy link
Contributor

Expected Behavior

Generating an SRA client from the SRA spec should produce a client that can properly validate its inputs.

Current Behavior

Generating an SRA client from the SRA spec produces a client that has too many backslashes in its regular expressions to properly validate its inputs. Specifically, wholeNumberSchema (^\\d+$) and numberSchema (^\\d+(\\.\\d+)?$) don't work because the regular expressions have double-backslashes where they should only be single-.

Possible Solution

Perhaps change @0x/sra-spec to convert double-backslashes to single ones, in JSON schema regular expressions, before injecting them into the API spec.

Alternatively, perhaps remove the extra backslashes from the JSON schemas themselves, though I'm not sure that makes sense, since apparently the implementations of JSON schema validation (npmjs' jsonschema and PyPI's `jsonschema) handle this just fine.

Steps to Reproduce (for bugs)

  1. Generate a client from our SRA spec. (To see how the Python client was generated, see https://github.com/0xProject/0x-monorepo/blob/development/python-packages/sra_client/generate.sh)
  2. Try to use the client in a way that will validate a whole number.

Context

This issue arose when @michaelhly was trying to write example usage code for our Python SRA client.

He said:

there is a bug in the SRA client schema checker
it is complaining about the fee being '0'
the setter's regular expression has an extra backslash where it should not

    @taker_fee.setter
    def taker_fee(self, taker_fee):
        """Sets the taker_fee of this OrderSchema.


        :param taker_fee: The taker_fee of this OrderSchema.  # noqa: E501
        :type: str
        """
        if taker_fee is None:
            raise ValueError(
                "Invalid value for `taker_fee`, must not be `None`"
            )  # noqa: E501
        if taker_fee is not None and not re.search(
            r"^\\d+$", taker_fee
        ):  # noqa: E501
            raise ValueError(
                r"Invalid value for `taker_fee`, must be a follow pattern or equal to `/^\\d+$/`"
            )  # noqa: E501

        self._taker_fee = taker_fee

r"^\d+$" should be r"^\d+$"
and this is true for all the validation checkers in the sra_client

But the Python SRA client is all generated code. That regular expression wasn't written by hand in there; the code generator just copied it from the SRA spec.

The OpenAPI spec doesn't explicitly declare whether backslashes in regular expressions should be escaped, but it does specifically show an example using \d, and it does not escape the backslash. See https://swagger.io/docs/specification/data-models/data-types/#pattern

@feuGeneA feuGeneA added the @0x/sra-spec Standard Relayer API Spec label Apr 5, 2019
@LogvinovLeon LogvinovLeon removed their assignment Apr 8, 2019
@stale
Copy link

stale bot commented May 8, 2019

This issue has been automatically marked as stale because it has not had recent activity. It will be closed if no further activity occurs. Thank you for your contributions.

@stale stale bot added the stale label May 8, 2019
@dekz dekz added the pinned label May 8, 2019
@stale stale bot removed the stale label May 8, 2019
@fabioberger fabioberger changed the title Clients generated from SRA spec can't validate numeric inputs. sra-spec: Clients generated from SRA spec can't validate numeric inputs. Jun 4, 2019
feuGeneA added a commit that referenced this issue Dec 17, 2019
Root problem is that there are too many backslashes in the SRA spec
itself.  See #1727

This was previously fixed for heavily-tested endpoints (get and post
order, etc), but was only recently discovered for the get-order-config
endpoint.
feuGeneA added a commit that referenced this issue Dec 20, 2019
…ts (#2399)

* Bug fix: unescape backslashes in regexes

Root problem is that there are too many backslashes in the SRA spec
itself.  See #1727

This was previously fixed for heavily-tested endpoints (get and post
order, etc), but was only recently discovered for the get-order-config
endpoint.

* Demonstrate get_order_config()

* Rename DefaultApi to RelayerApi

* Simplify RelayerApi instantiation

* Document paylod and response schemas

* Stop caring which contracts are wrapped

* Increase platform agnosticism

* Update CHANGELOG

* Remove unnecessary f-string
Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
pinned @0x/sra-spec Standard Relayer API Spec
Projects
None yet
Development

No branches or pull requests

3 participants