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

Parameter validation for aws-wps requests #215

Open
wants to merge 3 commits into
base: master
Choose a base branch
from

Conversation

anguss00
Copy link
Contributor

@anguss00 anguss00 commented Jan 8, 2018

Requests are validated as follows:

Subset, layer and callback email are required parameters.
Subset must have a latitude, longitude and temporal section.
If these conditions are not met, the request aborts before going to batch.

Add geonetwork URL as sandbox for development mode.

@jonescc
Copy link
Contributor

jonescc commented Jan 8, 2018

@anguss00 - time should not be a required parameter - we just made it optional not long ago to support bathymetry files which don't have a temporal dimension.

@jonescc
Copy link
Contributor

jonescc commented Jan 8, 2018

@anguss00
Copy link
Contributor Author

anguss00 commented Jan 8, 2018

Gotcha, thanks @jonescc. Will update

@anguss00
Copy link
Contributor Author

anguss00 commented Jan 8, 2018

Probably better to validate on the regex defined in GGD originally. Will update this

@anguss00
Copy link
Contributor Author

anguss00 commented Jan 9, 2018

Done @jonescc

throw new ValidationException("Request must have a subset");
} else {
int latLonCount = 0;
Pattern latLonPattern = Pattern.compile("([+-]?\\d+\\.?\\d+)\\s*,\\s*([+-]?\\d+\\.?\\d+)");
Copy link
Contributor

@jonescc jonescc Jan 10, 2018

Choose a reason for hiding this comment

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

Should probably align better with aggregator validation https://github.com/aodn/aws-wps/blob/master/aggregation-worker/src/main/java/au/org/emii/geoserver/client/SubsetParameters.java#L40 - we haven't used regexes for some time - they were problematic

Copy link
Contributor

Choose a reason for hiding this comment

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

Even better if they used the same code so they couldn't get out of synch.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Yeah that's a really good point. I'll amend that

@gsatimos
Copy link
Contributor

@anguss00 is this still a pending merge?

@anguss00
Copy link
Contributor Author

This was an extra reserve task that just didn't quite make it in. So yes, pending merge, but I think needs another change or two.

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.

3 participants