Conversation
| } | ||
|
|
||
| // otherwise compare entire bodies | ||
| return bodyBefore != bodyAfter |
There was a problem hiding this comment.
i think we should return a false here, as otherwise too prone to false positives due to non-deterministic fields
| /** | ||
| * Compare only the fields that were sent in the PUT/PATCH request. | ||
| * Returns true if any of those fields changed between before and after GET responses. | ||
| */ |
There was a problem hiding this comment.
add comment stating this only works where the payload is a JSON object matching the resource structure. it will not work for cases like JSON Patch RFC6902
| internal fun hasChangedModifiedFields( | ||
| bodyBefore: String, | ||
| bodyAfter: String, | ||
| fieldNames: Set<String> |
There was a problem hiding this comment.
to avoid flakiness, i think we should pass as well the bodyModify, and do check on that
|
|
||
| if(!jsonBefore.isJsonObject || !jsonAfter.isJsonObject){ | ||
| // not JSON objects, fallback to full comparison | ||
| return bodyBefore != bodyAfter |
There was a problem hiding this comment.
return false. see previous explanations
| val valueAfter = objAfter.get(field) | ||
|
|
||
| if(valueBefore != valueAfter){ | ||
| return true |
There was a problem hiding this comment.
besides being different from valueBefore, we should make sure that valueAfter is equal to valueModify
| return false | ||
| } catch (e: Exception) { | ||
| // JSON parsing failed, fallback to full comparison | ||
| return bodyBefore != bodyAfter |
|
|
||
| /** | ||
| * Checking bugs like: | ||
| * POST|PUT /X 2xx (create resource) |
There was a problem hiding this comment.
this first call is not strictly necessary. resources could be already existing or created with database insertions
| * | ||
| * If a PUT/PATCH fails with 4xx, it should have no side-effects. | ||
| * A GET before and after should return the same resource state. | ||
| */ |
There was a problem hiding this comment.
add to comment that we need to take into account the non-determinism of the fields, eg like timestamps and UUIDs
| return@forEach | ||
| } | ||
|
|
||
| // among those, find one that also has a successful creation step |
There was a problem hiding this comment.
thanks. after reading this code i realized there are quite a few edge cases we need to handle. code here needs to be changed. i update the algorithm description in notes.txt
2 participants
No description provided.