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

Auto-Rules JSON creation does not validate numeric values #530

Closed
andrewazores opened this issue Jun 22, 2021 · 2 comments · Fixed by #539
Closed

Auto-Rules JSON creation does not validate numeric values #530

andrewazores opened this issue Jun 22, 2021 · 2 comments · Fixed by #539
Assignees
Labels
bug Something isn't working good first issue Good for newcomers

Comments

@andrewazores
Copy link
Member

cryostatio/cryostat#416 (comment)

When POSTing a rule definition, all of the same validations should be performed on the input whether it is supplied as a form or a a JSON document. Currently, the JSON document pathway does not validate numeric inputs the same way that the form pathway does.

@andrewazores andrewazores added bug Something isn't working good first issue Good for newcomers labels Jun 22, 2021
@jan-law jan-law self-assigned this Jun 22, 2021
@jan-law
Copy link
Contributor

jan-law commented Jun 24, 2021

Is this Rule constructor logic desirable? Neither the form nor JSON document pathway will inform users of negative maxAgeSeconds or maxSizeBytes:

this.maxAgeSeconds =
                builder.maxAgeSeconds > 0 ? builder.maxAgeSeconds : this.archivalPeriodSeconds;
this.maxSizeBytes = builder.maxSizeBytes;

@andrewazores
Copy link
Member Author

There needs to be a way for users to specify unlimited size/age, so this could be 0 or a negative value. If the max age isn't specified but the archival period is specified then the max age is set to equal the archival period, so that the series of archived recordings contain as little overlapping data as possible.

Gson would normally deserialize a missing numeric entry as 0 here anyway, so a negative value would mean the client explicitly specified no limit - but they could just as easily use 0 to do this, if that's what the API spec defines. So I think requiring non-negative is okay here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
bug Something isn't working good first issue Good for newcomers
Projects
None yet
Development

Successfully merging a pull request may close this issue.

2 participants