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

This fixes Issue #72 #76

Merged
merged 2 commits into from
Sep 25, 2019
Merged

This fixes Issue #72 #76

merged 2 commits into from
Sep 25, 2019

Conversation

pvince
Copy link
Contributor

@pvince pvince commented Feb 5, 2019

Fixed issue #72 where int32 and int64 values were only being validated as positive, unsigned values. Added validation that ensures the values are signed integers as per the OAS 3.0 specification.master

Also updated the comments to indicate these changes.

Also also added tests for various edge case values.

… positive, unsigned values. Added validation that ensures the values are signed integers as per the OAS 3.0 specification.master

Also updated the comments to indicate these changes.

Also also added tests for various edge case values.
@coveralls
Copy link

coveralls commented Feb 5, 2019

Pull Request Test Coverage Report for Build 336

  • 5 of 6 (83.33%) changed or added relevant lines in 1 file are covered.
  • 11 unchanged lines in 4 files lost coverage.
  • Overall coverage increased (+0.3%) to 81.098%

Changes Missing Coverage Covered Lines Changed/Added Lines %
src/options.ts 5 6 83.33%
Files with Coverage Reduction New Missed Lines %
src/oas3/parameterParsers/pathStyleParser.ts 1 88.89%
src/oas3/parameterParsers/simpleStringParser.ts 1 90.57%
src/oas3/parameterParsers/structuredParser.ts 4 84.75%
src/oas3/urlEncodedBodyParser.ts 5 68.42%
Totals Coverage Status
Change from base Build 333: 0.3%
Covered Lines: 1061
Relevant Lines: 1249

💛 - Coveralls

// Javascript can only safely support a range of -(2^53 - 1) to (2^53 - 1)
// https://developer.mozilla.org/en-US/docs/Web/JavaScript/Reference/Global_Objects/Number/MAX_SAFE_INTEGER
const INT_64_MIN = Number.MIN_SAFE_INTEGER;
const INT_64_MAX = Number.MAX_SAFE_INTEGER;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I'm not sure if it's better to make this Number.MAX_SAFE_INTEGER or Math.pow(2, 63) - 1. If someone passes us a JSON object with 4611686018427388001, this is allowed according to the OpenAPI spec, but we are going to round it to 4611686018427388000 which is... odd.

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think we should probably allow big numbers, and let it be up to the person using the library to be aware of the fact that JavaScript is weird. :P

@jwalton
Copy link
Contributor

jwalton commented Feb 6, 2019

One other minor comment; exegesis uses semantic-release, so I can either squash-merge your commit to a 'fix: Fixes validation of int32 and int64 values.', or you can change one of your commit messages to follow this format (this gets used to generate the release notes, and to bump the version number).

@jwalton
Copy link
Contributor

jwalton commented Sep 25, 2019

🎉 This PR is included in version 2.1.0 🎉

The release is available on:

Your semantic-release bot 📦🚀

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

3 participants