Added openApi spec formats #22

seriousme · 2021-03-27T15:34:14Z

As discussed in #9 I added formats for:

byte
int32
int64
float
double
password
binary
As defined in the OpenAPI specification at https://spec.openapis.org/oas/v3.0.0#data-types

Kind regards,
Hans

coveralls · 2021-03-27T15:35:10Z

Pull Request Test Coverage Report for Build 711580351

0 of 0 changed or added relevant lines in 0 files are covered.
6 unchanged lines in 1 file lost coverage.
Overall coverage increased (+0.3%) to 92.641%

Files with Coverage Reduction	New Missed Lines	%
dist/formats.js	6	88.97%

Totals
Change from base Build 711572653:	0.3%
Covered Lines:	126
Relevant Lines:	133

💛 - Coveralls

seriousme · 2021-03-27T15:44:03Z

I think coveralls has a quirck: is says 6 lines lost coverage, but looking at my output it says:

------------|---------|----------|---------|---------|------------------------
All files   |   94.85 |       90 |     100 |   94.78 |                        
 formats.js |   91.89 |    86.57 |     100 |   91.67 | 87,109,113,130,135,157 
 index.js   |     100 |      100 |     100 |     100 |                        
 limit.js   |   97.06 |    93.33 |     100 |   97.06 | 49                     
------------|---------|----------|---------|---------|------------------------

87 is within compareDate
109 & 113 are within compareTime
130 & 135 are within compareDateTime
157 is within function regex()

Happy to have a look at that as well, but I'd rather not mix it with this PR ;-)

Kind regards,
Hans

epoberezkin · 2021-03-28T09:58:29Z

tests/extras/format.json

+        "valid": false
+      },
+      {
+        "description": "2**32 fails",


the test should be that 231 also fails, 232 - 1 succeeds

epoberezkin · 2021-03-28T09:59:46Z

src/formats.ts

+  // byte: https://github.com/miguelmota/is-base64
+  byte: /^(?:[A-Za-z0-9+/]{4})*(?:[A-Za-z0-9+/]{2}==|[A-Za-z0-9+/]{3}=)?$/gm,
+  // signed 32 bit integer
+  int32: validateInteger(32),


these should be defined as:

{ type: "number", validateInteger(32), }

otherwise that would be applied to strings (they should not be) and would not be applied to numbers (they should be)

I got it to work using:

{ type: "number", validate: validateInteger(32) }

The problem that I have now is that the test on a non-numerical value ("x") now returns true instead of false ?!?
Can I just safely remove that test ?

to fail on non-numeric value the schema should have type: "number", format on its own should not fail non-numbers.

this is the general approach of JSON Schema that type should be explicit in the schema, it is never implied by anything else. Even your old definition would pass {} for example on that test, unless there is type in the schema.

epoberezkin · 2021-03-28T10:05:34Z

src/formats.ts

@@ -62,6 +69,21 @@ export const fullFormats: DefinedFormats = {
  "json-pointer-uri-fragment": /^#(?:\/(?:[a-z0-9_\-.!$&'()*+,;:=@]|%[0-9a-f]{2}|~0|~1)*)*$/i,
  // relative JSON-pointer: http://tools.ietf.org/html/draft-luff-relative-json-pointer-00
  "relative-json-pointer": /^(?:0|[1-9][0-9]*)(?:#|(?:\/(?:[^~/]|~0|~1)*)*)$/,
+  // the following formats are used by the openapi specification: https://spec.openapis.org/oas/v3.0.0#data-types


Actually 3.0.0 lists more datatypes than 3.1.0

The OpenApi crew thinks semantic versioning is not relevant since each spec explictly names its version, however for tool builders its a bit annoying that a minor version increase is not backwards compatible :-(

src/formats.ts

epoberezkin · 2021-03-28T10:23:55Z

src/formats.ts

+  // C-type float
+  float: validateNumber(128),
+  // C-type double
+  double: validateNumber(1024),


I think this could be defined as {type: "number", validate: isFinite}, although type: "number" would do it already, so maybe just true

From a javascript perspective yes, but if you validate them before export then a receiver using C will see a difference between floats and doubles. This was the only test I found to check the difference between floats and doubles.
We can set it to true, but it might come back to haunt us ;-)

src/formats.ts

epoberezkin · 2021-03-28T10:25:48Z

src/formats.ts

+  const max = Number(BigInt(2) ** BigInt(bits - 1))
+  const min = max * -1
+
+  return (value) => max >= value && min <= value


strictly ">" I think (although the limit is slightly lower I think)...

This is where it becomes a bit of a challenge
Javascript integers go up to 2^53 hence the BigInt type
Ideally AJV would use BigInt for all its integer handling, however BigInt only plays nice with other BigInts :-(
(e.g. 2*BigInt(2) fails BigInt(2)*BigInt(2) succeeds.
So either we stick to basic javascript integers and skip the whole BigInt stuff of we need to come up with something clever ..

epoberezkin · 2021-03-28T10:26:59Z

src/formats.ts

+  // signed 64 bit integer
+  int64: validateInteger(64),
+  // C-type float
+  float: validateNumber(128),


Possibly, float can be constrained by hardcoded IEEE 754 limits (as 2**128 is not precise anyway and would cause run-time conversions)...

I will need to read up a bit on this ;-)

seriousme · 2021-03-28T13:24:51Z

We might be making our lives a bit too complex ;-)
Looking at https://json-schema.org/understanding-json-schema/reference/numeric.html

float and double should just be "number", no smart arithmic on when something is a double or not
integer is anything that passes: Number.isInteger(), again no smarts on int32 or int64

The float, double, int32 and int64 labels would then just be hints to developers, just like password and binary.

What do you think ?

Kind regards,
Hans

epoberezkin · 2021-03-30T18:40:08Z

Sorry for the delay.

float and double should just be "number", no smart arithmic on when something is a double or not

I am ok with that. Options are:

make them true that would work with default options, but would be wrong with strictNumbers: false option
make them isFinite

I suggest true as I don't believe anybody would use strictNumbers: false (it's only for backwards compatibility, should be deprecated tbh), so agreed.

Also ok to hardcode the limits for float as a single precision number.

integer is anything that passes: Number.isInteger(), again no smarts on int32 or int64

agreed re int64 (In JS it would limit it to 2^53, but anything above is not really integer
For int32 I suggest hardcode specific limits either as -2^31 ... 2^31 - 1 or as -2147483648 ... 2147483647 (although the first is probably cleaner, should be negligible difference to performance if it's a module level constant.

seriousme · 2021-03-31T19:09:17Z

I have reworked the implementation of float, double, int32 and int64

Since I can't specify:

float: { type:"number", validate: true}

and we still want to block strings I had to use a function that returns true.

On the testing I found a weird thing, if I add this test:

      {
        "description": "non-numeric fails",
        "data": "x",
        "valid": false
      }

To any of the numeric formats, the test returns true instead of false.
Should we be worried ?

Hans

epoberezkin · 2021-03-31T19:36:04Z

Should we be worried ?

No

Schema {"format": "double"} in the test allows any string, to only allow numbers it should be {"format": "double", "type": "number"}. I would suggest changing the schema (if you do, then "256.1" would also fail).

You might want to say that format: double should apply to both number and correctly formatted string, but I don't think it's correct - in JSON Schema data type is never implied by other keywords, it is not applied by any string format (for example any number is valid against {format: "email"}), so why would formats for numbers require that the data is a number (and why would they apply to strings, when string formats do not apply for numbers)?

Hope it makes some sense :)

seriousme · 2021-04-01T17:58:04Z

OK, I have fixed the tests to use {"format": "double", "type": "number"}.
(and replaced "256.1" by 256.1 ;-))
Can you please review again ?

Kind regards,
Hans

epoberezkin · 2021-04-02T11:56:33Z

thank you!

Added openApi spec formats

aa03409

Merge branch 'master' into master

ddbfb10

epoberezkin reviewed Mar 28, 2021

View reviewed changes

roggervalf mentioned this pull request Mar 30, 2021

Adding int32 format #8

Closed

seriousme added 2 commits March 31, 2021 20:28

Merge branch 'master' into master

c509499

Reworked float, double, int32 and int64

7f89297

updated tests to force "number" type

fc389cd

Merge branch 'master' into master

c2399d0

epoberezkin merged commit ec288c4 into ajv-validator:master Apr 2, 2021

kevinccbsg mentioned this pull request Apr 3, 2021

[question] Release v2.0.2 does not include new formats #25

Closed

amh4r mentioned this pull request Apr 5, 2021

Add peer deps and missing ajv formats Q42/openapi-typescript-validator#1

Closed

seriousme mentioned this pull request Apr 25, 2021

[question] can you use ajv-formats with ajv@6.12.6 ? #26

Open

seriousme mentioned this pull request May 11, 2021

Property names are missing in error text seriousme/fastify-openapi-glue#222

Closed

vwong mentioned this pull request Jun 27, 2023

Feat: Upgrade AJV from 6 to 8 pactflow/swagger-mock-validator#31

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Added openApi spec formats #22

Added openApi spec formats #22

seriousme commented Mar 27, 2021

coveralls commented Mar 27, 2021 •

edited

Loading

seriousme commented Mar 27, 2021

epoberezkin Mar 28, 2021

epoberezkin Mar 28, 2021

seriousme Mar 28, 2021

epoberezkin Mar 30, 2021

epoberezkin Mar 30, 2021

epoberezkin Mar 28, 2021

seriousme Mar 28, 2021

epoberezkin Mar 28, 2021

seriousme Mar 28, 2021

epoberezkin Mar 28, 2021

seriousme Mar 28, 2021

epoberezkin Mar 28, 2021

seriousme Mar 28, 2021

seriousme commented Mar 28, 2021

epoberezkin commented Mar 30, 2021

seriousme commented Mar 31, 2021

epoberezkin commented Mar 31, 2021 •

edited

Loading

seriousme commented Apr 1, 2021

epoberezkin commented Apr 2, 2021

Added openApi spec formats #22

Added openApi spec formats #22

Conversation

seriousme commented Mar 27, 2021

coveralls commented Mar 27, 2021 • edited Loading

Pull Request Test Coverage Report for Build 711580351

💛 - Coveralls

seriousme commented Mar 27, 2021

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

Choose a reason for hiding this comment

seriousme commented Mar 28, 2021

epoberezkin commented Mar 30, 2021

seriousme commented Mar 31, 2021

epoberezkin commented Mar 31, 2021 • edited Loading

seriousme commented Apr 1, 2021

epoberezkin commented Apr 2, 2021

coveralls commented Mar 27, 2021 •

edited

Loading

epoberezkin commented Mar 31, 2021 •

edited

Loading