fix: common fields rule (#747) #750
Conversation
maxim-tschumak
changed the title
Codecov Report
@@ Coverage Diff @@
## master #750 +/- ##
============================================
- Coverage 86.5% 86.27% -0.24%
- Complexity 959 964 +5
============================================
Files 168 168
Lines 2587 2601 +14
Branches 354 360 +6
============================================
+ Hits 2238 2244 +6
Misses 186 186
- Partials 163 171 +8
Continue to review full report at Codecov.
|
maxim-tschumak
changed the title
maxim-tschumak
changed the title
maxim-tschumak
changed the title
| assertThat(rule.checkField("modified", StringProperty("date-time"))).isNull() | ||
| assertThat(rule.checkField("type", StringProperty())).isNull() | ||
| fun `checkField matching should be case insensitive`() { | ||
| assertThat(rule.checkField("iD", createSchema(INTEGER_TYPE, null))).isNotNull() |
There was a problem hiding this comment.
I'm not sure whether it is wise to switch to a name that is not an exact match. I would suggest to remove the field conversion to lower case anyhow, as this is addressed by the snake_case rule in the first place.
| else null | ||
| } | ||
| fun checkField(name: String, property: Schema<Any>): String? = | ||
| commonFields[name.toLowerCase()]?.let { (type, format) -> |
There was a problem hiding this comment.
Let's remove the lower case conversion, as this failure is covered by the snake_case check, and we expect exact name matches.
| assertThat(rule.checkField("iD", createSchema(INTEGER_TYPE, null))).isNotNull() | ||
| assertThat(rule.checkField("CREATED", createSchema(INTEGER_TYPE, null))).isNotNull() | ||
| assertThat(rule.checkField("tYpE", createSchema(null, null))).isNotNull() | ||
| assertThat(rule.checkField("CREated", createSchema(STRING_TYPE, "time"))).isNotNull() |
There was a problem hiding this comment.
By the way, we should introduce some synonyms rule: created -> created_at, modified -> modified_at.
| schema: | ||
| properties: | ||
| id: | ||
| "${'$'}ref": "#/components/schemas/CustomId" |
There was a problem hiding this comment.
Unfortunately, this is not the only allowed place for a reference.
There was a problem hiding this comment.
It's a representative test - we don't want to test the resolve feature of the swagger-parser.
maxim-tschumak
force-pushed
the
747-fix-common-fields-rule
branch
from
e810c51
to
753d6e9
Compare
maxim-tschumak
force-pushed
the
747-fix-common-fields-rule
branch
2 times, most recently
from
828c378
to
792faee
Compare
maxim-tschumak
force-pushed
the
747-fix-common-fields-rule
branch
from
792faee
to
1e650b4
Compare
|
Now, the rule also checks all the nested properties recursively. @tkrop please have another look. |
|
👍 |
1 similar comment
|
👍 |
Related to #714
Fixes #747