Fix multiple error types #948

cuminandpaprika · 2020-07-10T01:10:39Z

This PR appends the error code to the type name, without changing the error response definition (which was previously a breaking change for transforms) This change is now hidden behind a toggle flag, so that it will not break arrai transforms

Behaviour with flag: https://github.com/anz-bank/sysl/pull/948/files#diff-96ec2bd3bb0448da4b0b23b92469582a
Behaviour without flag: https://github.com/anz-bank/sysl/pull/948/files#diff-ac6fdb35ec698c09e006ff0cb5a83a64

Changes:

Fixes an importer issue with multiple error return types being duplicated as types with the same name.
Also escapes output regex string, which was causing invalid Sysl to be output

Checklist:

Added related tests
Made corresponding changes to the documentation

codecov · 2020-07-10T01:14:24Z

Codecov Report

Merging #948 into master will increase coverage by 0.80%.
The diff coverage is 84.21%.

@@            Coverage Diff             @@
##           master     #948      +/-   ##
==========================================
+ Coverage   83.35%   84.15%   +0.80%     
==========================================
  Files          73       74       +1     
  Lines       10609    10326     -283     
==========================================
- Hits         8843     8690     -153     
+ Misses       1432     1324     -108     
+ Partials      334      312      -22

Impacted Files	Coverage Δ
pkg/pbutil/output.go	`76.00% <ø> (-0.48%)`	⬇️
pkg/syslwrapper/test_helper.go	`57.89% <ø> (-1.42%)`	⬇️
pkg/parse/listener_impl.go	`89.50% <61.53%> (+<0.01%)`	⬆️
pkg/importer/importer.go	`63.15% <63.15%> (ø)`
pkg/parse/parse.go	`82.97% <65.51%> (-1.00%)`	⬇️
pkg/mod/gomod.go	`73.68% <66.66%> (+1.27%)`	⬆️
pkg/mod/module.go	`66.66% <72.72%> (+0.95%)`	⬆️
pkg/importer/formats.go	`77.27% <77.27%> (ø)`
pkg/importer/openapi.go	`82.77% <81.78%> (-6.67%)`	⬇️
pkg/importer/swagger.go	`82.14% <89.74%> (+2.55%)`	⬆️
... and 16 more

pkg/importer/tests-openapi/multiple-return-values.sysl

Fixes an issue where the regex expression populated from the pattern property of string types was breaking the parser. This means that any transforms using this field will need to unescape the regex expression

Sometimes breaking changes to the importers need to be made without a corresponding fix in the transforms having been made yet. Feature flags allow these dependencies to be decoupled, so that support can be added at a later date.

Escapes typename of special characters Fixes an issue where multiple error responses with headers produce duplicate type names, as they were named <endpointname>_error Modify log to only show in debug Restore default handling of multiple error responses

andrewemeryanz

Minor change. Otherwise good.

pkg/importer/importer_test.go

Adds a test file multiple-error-responses.yaml which demonstrates the current importer behaviour with multiple error response codes. Tidies up the test file

cuminandpaprika requested review from a user and nofun97 July 10, 2020 01:10

ghost suggested changes Jul 10, 2020

View reviewed changes

pkg/importer/tests-openapi/multiple-return-values.sysl Outdated Show resolved Hide resolved

cuminandpaprika added 5 commits July 13, 2020 18:02

Add update of importer test files to make golden

3f4f38b

Escape regex output

71d1b43

Fixes an issue where the regex expression populated from the pattern property of string types was breaking the parser. This means that any transforms using this field will need to unescape the regex expression

Adds importer flags to allow feature toggling

28079c2

Sometimes breaking changes to the importers need to be made without a corresponding fix in the transforms having been made yet. Feature flags allow these dependencies to be decoupled, so that support can be added at a later date.

Add test for MultipleErrorResponses flag

64b7a18

cuminandpaprika force-pushed the fix-multiple-error-types branch from ee0b406 to 64b7a18 Compare July 13, 2020 22:53

cuminandpaprika requested review from andrewemeryanz and a user July 13, 2020 22:53

andrewemeryanz requested changes Jul 14, 2020

View reviewed changes

pkg/importer/importer_test.go Show resolved Hide resolved

cuminandpaprika force-pushed the fix-multiple-error-types branch from 161adbe to 983b0e4 Compare July 15, 2020 02:13

Add test for existing behaviour

91d9e7c

Adds a test file multiple-error-responses.yaml which demonstrates the current importer behaviour with multiple error response codes. Tidies up the test file

cuminandpaprika force-pushed the fix-multiple-error-types branch from 983b0e4 to 91d9e7c Compare July 15, 2020 02:33

cuminandpaprika requested a review from andrewemeryanz July 15, 2020 02:37

cuminandpaprika closed this Jul 17, 2020

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Fix multiple error types #948

Fix multiple error types #948

cuminandpaprika commented Jul 10, 2020 •

edited

Loading

codecov bot commented Jul 10, 2020 •

edited

Loading

andrewemeryanz left a comment

Fix multiple error types #948

Fix multiple error types #948

Conversation

cuminandpaprika commented Jul 10, 2020 • edited Loading

codecov bot commented Jul 10, 2020 • edited Loading

Codecov Report

andrewemeryanz left a comment

Choose a reason for hiding this comment

cuminandpaprika commented Jul 10, 2020 •

edited

Loading

codecov bot commented Jul 10, 2020 •

edited

Loading