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

Pattern Validation - Max Length Issue Fix #1426

Open
wants to merge 25 commits into
base: main
Choose a base branch
from

Conversation

samyakOO7
Copy link
Collaborator

@samyakOO7 samyakOO7 commented Nov 14, 2024

Pattern validation was taking priority in response schema for stub and request schema for test over max length parameter due to random() method generating value more than default

Fixed the use case putting a constraint of max length on random() method so that it never generates value for than max length keeping the current logic intact

Test Spec File - validate-regex-spec.yaml.zip

Backward Compatibility Spec (v2.0.36 release check) -
validate-regex-spec.yaml.zip

Logic used -

Default length - 5 (remains intact)

If minimum and maximum is both supplied, pattern length would be in range of (min to max)

If minimum length is not supplied in spec -> and maximum is less than default (5), pattern would be of length (1 i.e. generex min value, max)
Say max = 4 is only supplied
pattern length would be in range (1-4)

If minimum length is not supplied in spec -> and maximum is greater than default (5), pattern would be of length (5 (default, max)
Say max = 15 is only supplied
pattern length would be in range (5-15)

if max is not supplied and min value exist we will assume to take pattern length from (min , generex max value)

Test for maximum length conditions :
Request : {minLength : 2, maxLength: 11}
Response : {minLength : 2, maxLength: 6}

Before Stub :

curl -X POST -H "Content-Type: application/json" -d '{ "number": "12000000" }' http://0.0.0.0:49272/validate
{
    "number": "873.13863"
}

Generated value : 9 digit

Before Test:
Details:

Scenario: POST /validate -> 200 has FAILED
In scenario "Validate the input number. Response: Input is valid"
API: POST /validate -> 200

  In scenario "Validate the input number. Response: Input is valid"
     API: POST /validate -> 200
   
       REQUEST.BODY.number Contract expected string with maxLength 2 but request contained "2.8837947"

After Issue Fixed Stub:

curl -X POST -H "Content-Type: application/json" -d '{ "number": "12000000" }' http://0.0.0.0:9000/validate
{
    "number": "8.9162"
}

After Issue Fixed Test:

Request:
POST /validate
Accept-Charset: UTF-8
Accept: */*
Content-Type: application/json

{
    "number": "3."
}

Response:
200 OK
Vary: Origin
X-Specmatic-Result: success
X-Specmatic-Type: random
Content-Length: 26
Content-Type: application/json
Connection: keep-alive

{
    "number": "3618.9"
}

Test for Minimum Length conditions:
Request : {minLength : 2, maxLength: 11}
Response : {minLength : 2, maxLength: 6}

Stub:
Before Fix (The output generated was defaulting to 5 as minimum for num<5 (breaching min validation):

curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:54811/validate
{
    "number": "2.866"
}
 curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:54811/validate
{
    "number": "235.4"
}
curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:54811/validate
{
    "number": "8.150"
}

After Fix (Min validation is respected):

curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:9000/validate
{
    "number": "2."
}
 curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:9000/validate
{
    "number": "5.1"
}
curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:54811/validate
{
    "number": "8.150"
}
 curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:9000/validate
{
    "number": "39"
}

Test :
Before Fix:

Request:
POST /validate
Accept-Charset: UTF-8
Accept: */*
Content-Type: application/json

{
    "number": "6.7293812038"
}


Response:
400 Bad Request
Vary: Origin
X-Specmatic-Result: failure
Content-Length: 411
Content-Type: text/plain
Connection: keep-alive

In scenario "Validate the input number. Response: Input is valid"
API: POST /validate -> 200

  >> REQUEST.BODY.number
  
     Contract expected string with maxLength 2 but request contained "6.7293812038"

After Fix :

Request:
POST /validate
Accept-Charset: UTF-8
Accept: */*
Content-Type: application/json

{
    "number": "9."
}
Response:
200 OK
Vary: Origin
X-Specmatic-Result: success
X-Specmatic-Type: random
Content-Length: 23
Content-Type: application/json
Connection: keep-alive

{
    "number": "407"
}

Some edge cases taken care of defaults -


Request { maxLength = 4 }
Response { maxLength = 4 }

min = null and max (4) < default (5) [pattern (min ‎ =  Generex(min val = 1), max = 4)]

Stub-

curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:9000/validate
{
    "number": "7"
}
curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:9000/validate
{
    "number": "3.47"
}

Test -

Request:
POST /validate
Accept-Charset: UTF-8
Accept: */*
Content-Type: application/json

{
    "number": "65.0"
}
Response:
200 OK
Vary: Origin
X-Specmatic-Result: success
X-Specmatic-Type: random
Content-Length: 23
Content-Type: application/json
Connection: keep-alive

{
    "number": "4.72”
}

Request { maxLength = 15 }
Response { maxLength = 15 }
min length = null and max length > default (5) [pattern (min ‎ =  default(5), max = 15)]

Stub-

curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:9000/validate
{
    "number": "8.915"
}
curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:9000/validate
{
    "number": "9.609"
}
curl -X POST -H "Content-Type: application/json" -d '{ "number": "1" }' http://0.0.0.0:9000/validate
{
    "number": "7.09024600"
}

Test -

Request:
POST /validate
Accept-Charset: UTF-8
Accept: */*
Content-Type: application/json

{
    "number": "4.22214"
}
Response:
200 OK
Vary: Origin
X-Specmatic-Result: success
X-Specmatic-Type: random
Content-Length: 25
Content-Type: application/json
Connection: keep-alive

{
    "number": "2.648"
}


Clear outputs respecting validation schema are found with multiple test of min and max length

Thanks

Samy and others added 8 commits November 16, 2024 11:00
- Removing comments and instead renaming variables for readability
- Removing redundant rows in parameterised test for min and max length based String generation
- Adding test for regex based generation
- Extracting regex generation code to separate method
Copy link
Member

@harikrishnan83 harikrishnan83 left a comment

Choose a reason for hiding this comment

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

Looks good to me

if(defaultExample == null)
return StringValue(Generex(regex.removePrefix("^").removeSuffix("$")).random(randomStringLength))
if(defaultExample == null) {
if (maxLength != null)
Copy link
Member

Choose a reason for hiding this comment

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

Is there a possibility that the regex pattern can generate a String that is less than minLength, should we handle that also?

@@ -240,4 +250,12 @@ internal class StringPatternTest {
fun `string pattern encompasses email`() {
assertThat(StringPattern().encompasses(EmailPattern(), Resolver(), Resolver())).isInstanceOf(Result.Success::class.java)
}

@Test
fun `should fail to generate string when maxLength is less than minLength`() {
Copy link
Member

Choose a reason for hiding this comment

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

good that you have covered this scenario with a test

if(defaultExample == null) {
if (maxLength != null)
return StringValue(
Generex(regex.removePrefix("^").removeSuffix("$")).random(
Copy link
Member

Choose a reason for hiding this comment

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

any reason we have to remove the ^ and $?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

As these characters are not addressed by generex we are omitting it

if (maxLength != null)
return StringValue(
Generex(regex.removePrefix("^").removeSuffix("$")).random(
randomStringLength,
Copy link
Member

Choose a reason for hiding this comment

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

if randomStringLength is already based on minLength and maxLength why are we looking at maxLength again here?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Before it was only based on minLength

if (matches(it, resolver).isSuccess()) {
return it
}
throw ContractException("Schema example ${it.toStringLiteral()} does not match pattern $regex")
Copy link
Member

Choose a reason for hiding this comment

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

In this form, this condition is no more about regex matching.

Ideally, the resolver should be given a new MismatchMessages indicating that we are checking the schema example.

The matches method returns a result, and we could call result.throwOnFailure() instead of throwing this exception

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Resolved

minLength != null && 5 < minLength -> minLength
maxLength != null && 5 > maxLength -> maxLength
minLength != null && minLength > 0 -> minLength
maxLength != null && maxLength < 5 -> 1
Copy link
Member

Choose a reason for hiding this comment

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

This worked a little differently earlier. If maxLength < 4, use it, else use upto 5 chars. What's the reason we changed this?

Copy link
Collaborator Author

@samyakOO7 samyakOO7 Nov 21, 2024

Choose a reason for hiding this comment

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

Logic used -

Default length - 5 (remains intact)
If minimum and maximum is both supplied, pattern length would be in range of (min to max)
If minimum length is not supplied in spec -> and maximum is less than default (5), pattern would be of length (1 i.e. generex min value, max)
Say max = 4 is only supplied
pattern length would be in range (1-4)
If minimum length is not supplied in spec -> and maximum is greater than default (5), pattern would be of length (5 (default, max)
Say max = 15 is only supplied
pattern length would be in range (5-15)
if max is not supplied and min value exist we will assume to take pattern length from (min , generex max value)

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

We revamped this logic a bit as per the one given above


private fun generateFromRegex(regexWithoutCaretAndDollar: String, minLength: Int, maxLength: Int?): String =
maxLength?.let {
Generex(regexWithoutCaretAndDollar).random(minLength, it)
Copy link
Member

Choose a reason for hiding this comment

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

Regexes supported by Generex may also have a length, e.g. a{10}. While we are at it, do we want to handle this in case it's greater than a maxLength or less than minLength?

Copy link
Collaborator Author

@samyakOO7 samyakOO7 Nov 21, 2024

Choose a reason for hiding this comment

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

We are putting the constraints to generate regex from min to max length, previously we only gave it the min length and we assumed it to be normal length, which caused it to overflow most of the time, out of max range

The logic written before was a bit tricky to understand but was failing lot of cases

@MethodSource("lengthTestValues")
fun `generate string value of appropriate length matching minLength and maxLength parameters`(min: Int?, max: Int?, length: Int) {
@MethodSource("minLengthMaxLengthAndExpectedLength")
fun `generate string value as per minLength and maxLength`(min: Int?, max: Int?, expectedLengthOfGeneratedValue: Int) {
Copy link
Member

Choose a reason for hiding this comment

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

Use CsvSource instead of MethodSource, to keep the test data local to the test.


@ParameterizedTest
@MethodSource("regexMinLengthAndMaxLengthAndExpectedLength")
fun `generate string value as per regex in conjunction with minLength and maxLength`(regex: String?, min: Int?, max: Int?, expectedLength: Int) {
Copy link
Member

Choose a reason for hiding this comment

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

Use CsvSource instead of MethodSource, to keep the test data local to the test.

Samy and others added 7 commits November 27, 2024 12:53
* main: (124 commits)
  Fix schema regex and generation
  2.0.43
  Generate 4xx responses as per the schema in virtual-service be it any pattern containing a json object pattern
  Ignore a seed data entry which involves attribute selection during the seed data load stage of the virtual service
  Ignore a seed data entry with an id which is pre-existing in the cache of the virtual service
  Fix failing tests due to stale config.
  Add test for ExamplePreProcessor
  Add tests for Asserts.
  Cleanup on amber validation.
  2.0.42
  Changes for GoBack
  Added Details Pre Div
  Removed highlighting of specmatic errors in editor
  Editor syntax highlighting
  Fix full pattern validation with ListPattern.
  Add tests for partially valid examples.
  Better mismatchMessages for missing optional keys.
  better variable namings and partial handling
  Interactive Server partially valid examples.
  Modify result logic when all keys are mandatory.
  ...
- adding tests for contradicting length constraints and regex
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