0xProject / 0x-monorepo

0x protocol monorepo - includes our smart contracts and many developer tools
Other
1.41k stars 466 forks source link

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

Open feuGeneA opened 5 years ago

feuGeneA commented 5 years ago

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

stale[bot] commented 5 years ago

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.