Skip to content

Update oas_models.py to support multipleOf#139

Open
kevinmcody wants to merge 1 commit intoMarketSquare:mainfrom
kevinmcody:main
Open

Update oas_models.py to support multipleOf#139
kevinmcody wants to merge 1 commit intoMarketSquare:mainfrom
kevinmcody:main

Conversation

@kevinmcody
Copy link
Copy Markdown

Fixes #76

Support multipleOf constraint on an Integer or Number.

Strategy: Use the ceiling of Min divided by multipleOf, floor of Max divided by multipleOf, for the random min/max, then multiply by multipleOf

Support multipleOf constraint on an Integer or Number.

Strategy:  Use the ceiling of Min divided by multipleOf, floor of Max divided by multipleOf, for the random min/max, then multiply by multipleOf
@robinmackaij
Copy link
Copy Markdown
Collaborator

Thanks for looking into this @kevinmcody !

Looking into multipleOf I must say I misinterpreted this property somewhat. What I didn´t consider, for example, is that multipleOf: 0.7 on an integer type is valid; -7, 7, 14, etc. would be valid; equivalent to multipleOf: 7.

Considering the phrasing of the JSON specification, the annotation for multipleOf should be changed to multipleOf: float | None = Field(default=None, gt=0).

Using Decimal internally is a good idea (I should probably refactor the NumberSchema further) and considering multipleOf is a float for both IntegerSchmea and NumberSchema, the calculation logic can be extraced to a function that takes the minimum value, the maximum value and the multipleOf value and returns a tuple of k_min and k_max.

Note that annotation of the multipleOf as a gt=0 means checking that it's a positive number is no longer needed.

Also I don´t check for min > max within the models. This is a deliberate choice; if the schema does not make sense, the request data will simply fail schema validation without the need of extra validations / checks within the models / value generation.

The ToDo's for the get_values_out_of_bounds are also not quite straightforward; e.g. multipleOf = 1 for an IntegerSchema shouldn´t break get_values_out_of_bounds and the NumberSchema should include samples where precision is the issue. The test set for the JSON schema has some good examples:
https://github.com/json-schema-org/JSON-Schema-Test-Suite/blob/main/tests/draft2020-12/multipleOf.json#L67

Before this can be released, the tests in test_get_valid_value and test_get_values_out_of_bounds.py should also be updated to cover the new code in the oas_models.

I've created a branch to continue the work on this feature: https://github.com/MarketSquare/robotframework-openapitools/tree/76_add_support_for_multipleof
Can you target that branch with this PR / further changes? I'll see if I can get some of the tests added on that branch this Wednesday.

@kevinmcody
Copy link
Copy Markdown
Author

kevinmcody commented Apr 28, 2026 via email

@robinmackaij
Copy link
Copy Markdown
Collaborator

Thanks for looking it over. I was also a little intimidated by the invalid values, that’s why I sheepishly left them as TODOs. 😊 I will certainly get on your suggested refactor, and look at the tests though, probably sometime this week.

That's great. I might have some time this weekend to help out / work on this if you want to. I feel a TDD approach would be helpful here; get the tests in first (incl. the edge cases) so we get a clear idea on how the implementation should work, then get the implementation done.

A contributions guide is still missing, but if you're working from the devcontainer setup, the inv utests is a useful command to quickly run the unit tests.

@kevinmcody
Copy link
Copy Markdown
Author

kevinmcody commented Apr 29, 2026 via email

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

Successfully merging this pull request may close these issues.

Add support for multipleOf

2 participants