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

Enable runtime substitutions using class level validation blocks #4600

Closed
wants to merge 4 commits into from
Closed

Enable runtime substitutions using class level validation blocks #4600

wants to merge 4 commits into from

Conversation

homme
Copy link

@homme homme commented Mar 5, 2013

This fixes /issues/4596. As well as now honouring
class level validation blocks a validation hierarchy is implemented
across web, layer and class level validation blocks. This
hierarchy only takes effect when identical validation keys appear in
web, layer or class such that keys in more specialised blocks
override those in more generalised blocks. i.e. class overrides
layer which overrides web.

In conjunction with the above, default class level substitutions are
also enabled and also implement the precedence rules above. i.e. a
default substitution in class overrides layer which overrides
web.

Homme Zwaagstra added 2 commits March 5, 2013 14:55
This fixes /issues/4596.  As well as now honouring
class level validation blocks a validation hierarchy is implemented
across `web`, `layer` and `class` level validation blocks.  This
hierarchy *only* takes effect when identical validation keys appear in
`web`, `layer` or `class` such that keys in more specialised blocks
override those in more generalised blocks. i.e. `class` overrides
`layer` which overrides `web`.

In conjunction with the above, default class level substitutions are
also enabled and also implement the precedence rules above. i.e. a
default substitution in `class` overrides `layer` which overrides
`web`.
Previously the logic let through tags which didn't appear in the
validation blocks.
@tbonfort
Copy link
Member

@homme the CI server might not have mailed you, but your changes break 3 existing substitution testcases which should be checked given that adding CLASS substitutions is supposed to be backwards compatible.
http://ci.mapserver.org/job/mapserverpullrequests/82/console and navigate to the end of the log to see the diff.

@homme
Copy link
Author

homme commented Mar 11, 2013

@tbonfort Thanks for the heads up. I've enabled msautotest on my local checkout and the relevant tests now pass except for misc/expected/runtime_sub_test003.txt: this is because there is a bug in the original code such that a validation failing for a name in a layer can drop through to the web validation for that same name and pass. I believe the output in misc/result/runtime_sub_test003.txt is correct which shows a msEvalRegex() and a msValidateParameter() error.

I also note there are no tests for class validation: I can look at adding some if required.

@tbonfort
Copy link
Member

@mapserver-bot retest this please

tbonfort added a commit to MapServer/msautotest_DEPRECATED that referenced this pull request Mar 12, 2013
@tbonfort tbonfort closed this in 4e79097 Mar 12, 2013
@tbonfort
Copy link
Member

rebased and applied in 4e79097 , thanks @homme

mkofahl pushed a commit to faegi/mapserver that referenced this pull request Apr 9, 2013
…apServer#4600)

 As well as now honouring
class level validation blocks a validation hierarchy is implemented
across `web`, `layer` and `class` level validation blocks.  This
hierarchy *only* takes effect when identical validation keys appear in
`web`, `layer` or `class` such that keys in more specialised blocks
override those in more generalised blocks. i.e. `class` overrides
`layer` which overrides `web`.

In conjunction with the above, default class level substitutions are
also enabled and also implement the precedence rules above. i.e. a
default substitution in `class` overrides `layer` which overrides
`web`.

Ensure validation *always* occurs when a substitution is requested
Fixes MapServer#4596
Fixes MapServer#4600
@ghost ghost assigned tbonfort Apr 13, 2013
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.

None yet

2 participants