') + ')', 'gi'); if (regex.test(text)) { found = true; var frag = document.createDocumentFragment(); var parts = text.split(regex); parts.forEach(function(part, i) { if (i % 2 === 0) { frag.appendChild(document.createTextNode(part)); } else { var span = document.createElement('span'); span.className = 'userscript-highlight'; span.textContent = part; frag.appendChild(span); } }); node.parentNode.replaceChild(frag, node); } }); } else if (node.nodeType === 1 && node.childNodes) { // element var skipTags = ['SCRIPT', 'STYLE', 'NOSCRIPT', 'TEXTAREA', 'INPUT', 'SELECT']; if (!skipTags.includes(node.tagName)) { Array.from(node.childNodes).forEach(highlight); } } } highlight(document.body); // Re-highlight on dynamic content var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1 || node.nodeType === 3) highlight(node); }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Highlight Search Terms]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Strip utm_, fbclid, gclid, etc. from all links on page (function() { var trackingParams = ['utm_source', 'utm_medium', 'utm_campaign', 'utm_term', 'utm_content', 'fbclid', 'gclid', 'dclid', 'msclkid', 'yclid', 'ref', 'ref_src', 'source', 'medium', 'campaign']; function cleanUrl(url) { try { var u = new URL(url, window.location.origin); var changed = false; trackingParams.forEach(function(p) { if (u.searchParams.has(p)) { u.searchParams.delete(p); changed = true; } }); return changed ? u.toString() : url; } catch (e) { return url; } } function cleanLinks() { document.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } cleanLinks(); var observer = new MutationObserver(function(mutations) { mutations.forEach(function(m) { m.addedNodes.forEach(function(node) { if (node.nodeType === 1) { if (node.tagName === 'A') cleanLinks(); node.querySelectorAll('a[href]').forEach(function(a) { var clean = cleanUrl(a.href); if (clean !== a.href) a.href = clean; }); } }); }); }); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:Remove Tracking Parameters from Links]', __e); } })(); (function(){ try { var __m = "youtube.com"; var __re = new RegExp('^' + "youtube\\.com" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Auto-enable theater mode on YouTube (function() { function tryTheater() { var btn = document.querySelector('button[aria-label="Theater mode"], ytd-player #player button[title="Theater mode"]'); if (btn && !btn.classList.contains('activated')) { btn.click(); } } // Try immediately tryTheater(); // Try after navigation (SPA) var lastUrl = location.href; setInterval(function() { if (location.href !== lastUrl) { lastUrl = location.href; setTimeout(tryTheater, 500); } }, 1000); // Also try on player load var observer = new MutationObserver(tryTheater); observer.observe(document.body, { childList: true, subtree: true }); })(); } } catch(__e) { console.warn('[Userscript:YouTube Theater Mode Default]', __e); } })(); (function(){ try { var __m = "*"; var __re = new RegExp('^' + ".*" + ', 'i'); if (__m === '*' || __re.test(location.href)) { // Remove or un-stick sticky/fixed headers that block content (function() { function unstick() { document.querySelectorAll('header, nav, [role="banner"], .header, .navbar, .sticky, .fixed-top, [style*="position: fixed"], [style*="position:sticky"]').forEach(function(el) { if (el.style.position === 'fixed' || el.style.position === 'sticky' || getComputedStyle(el).position === 'fixed' || getComputedStyle(el).position === 'sticky') { el.style.position = 'static'; el.style.top = 'auto'; el.style.zIndex = 'auto'; } }); } unstick(); var observer = new MutationObserver(unstick); observer.observe(document.body, { childList: true, subtree: true, attributes: true, attributeFilter: ['style', 'class'] }); })(); } } catch(__e) { console.warn('[Userscript:Kill Sticky Headers]', __e); } })(); })(); [haskell-servant] Fix parsing of special characters by f-f · Pull Request #2676 · OpenAPITools/openapi-generator · GitHub
Skip to content

[haskell-servant] Fix parsing of special characters - #2676

Merged
wing328 merged 1 commit into
OpenAPITools:masterfrom
f-f:haskell-fix-special-chars-parsing
May 9, 2019
Merged

[haskell-servant] Fix parsing of special characters#2676
wing328 merged 1 commit into
OpenAPITools:masterfrom
f-f:haskell-fix-special-chars-parsing

Conversation

@f-f

@f-ff-f commented Apr 16, 2019

Copy link
Copy Markdown
Contributor

PR checklist

  • Read the contribution guidelines.
  • Ran the shell script under ./bin/ to update Petstore sample so that CIs can verify the change. (For instance, only need to run ./bin/{LANG}-petstore.sh, ./bin/openapi3/{LANG}-petstore.sh if updating the {LANG} (e.g. php, ruby, python, etc) code generator or {LANG} client's mustache templates). Windows batch files can be found in .\bin\windows\.
  • Filed the PR against the correct branch: master, 3.4.x, 4.0.x. Default: master.
  • Copied the technical committee to review the pull request if your PR is targeting a particular programming language.

Description of the PR

This reverts the change from #1850, as that patch broke the generator for our APIs.

In fact, after updating to upstream our service wanted to parse the following JSON:

[
{
"subscriptionStartDate": "2016-07-22",
"subscriptionEndDate": "2016-07-22",
}
]

..as this, which doesn't look correct:

[
{
"subscription*tDate": "2016-07-22",
"subscriptionEndDate": "2016-07-22",
}
]

The change was motivated as "a dependency upstream changed", but the only dependency involved here is text, and the signature of Data.Text.replace didn't change since the last time I updated this piece of code (version was 1.2.3.0, now we're at 1.2.3.1)

Also it looks like the change started to ignore the quote (') before the symbols to replace, which is purposefully there, so that the special characters are substituted only for name mangling, and not in every case as it happens here.
So this makes me guess that this was just a misunderstanding of the meaning of that code, so I added a comment on top of the function to explain better what's going on in there.

/cc @wing328@jonschoning@algas@Drezil

@Drezil

Copy link
Copy Markdown
Contributor

This is very interesting. We use this in our company to generate stubs and had exactly the same problem, which motivated the change - and pushed the patch that resolved our problems.

We work against an api that has "Percentage" as a field and that generated json as "%age" - similar to your "Start" <-> "*t"..

I will try your version later today if i got some time to better see where the problem is actually located.

@f-f

f-f commented Apr 17, 2019

Copy link
Copy Markdown
ContributorAuthor

We work against an api that has "Percentage" as a field and that generated json as "%age" - similar to your "Start" <-> "*t"..

@Drezil this sounds like a bug, because only a field called 'Percentage (note the quote there) should be translated to "%age"
However from the code here in this PR I don't see how this replacement without the quote can happen, so I'm definitely interested in a replication

@wing328

Copy link
Copy Markdown
Member

Given that there's no further question, I've merged this fix into master.

Thanks for the fix by @f-f

@wing328
wing328 merged commit 0ef5793 into OpenAPITools:masterMay 9, 2019
@f-f

f-f commented May 9, 2019

Copy link
Copy Markdown
ContributorAuthor

Thank you! 😊

@Drezil

Copy link
Copy Markdown
Contributor

@f-f After updating to the latest official version i still have issues with this. Maybe we can figure it out together.
At least i now have to make time to fix this for real.
I made a minimal example: https://github.com/Drezil/openapi_haskell_minimal_example

swagger definition is: https://github.com/Drezil/openapi_haskell_minimal_example/blob/master/swagger.yaml

$ > curl -X POST http://localhost:8080/foo -d '{"fooStartDate":"foo","bar_StartDate":"bar"}' -H "accept: application/json" -H "Content-Type: application/json"
Error in $: key "barUnderscoreStartDate" not present⏎
$ > curl -X POST http://localhost:8080/foo -d '{"fooStartDate":"foo","barUnderscoreStartDate":"bar"}' -H "accept: application/json" -H "Content-Type: application/json"
{"result":["bar"],"classUnderscorenames":["foo"]}⏎

Connecting with the Haskell-client to the Haskell-server works fine, as both use "Underscore" instead of the "_" defined in the swagger-api. But with that swagger-ui or other language server/clients still break.

Your problem with Start & *t is still fixed in this version, but the generated/consumed code for the dual problem i tried to solve is still present.
I have to program against an external api using underscores in field-names.

I also tried to dig into Aeson to look for answers. I don't understand how https://github.com/f-f/openapi-generator/blob/064e299caa6279ec5230ee897e4fadb25f23009c/modules/openapi-generator/src/main/resources/haskell-servant/Types.mustache#L84-L87 could have any effect, because it is basically a String -> String-Conversion, converting things like "class_name" into "classUnderscoreName" (the haskell-type generated) and vice-versa.
The change i did let me parse "class_name" correctly, but generated "*t" from a field "Start" - which caused your problem.

I don't see a solution how we could generate fields like "my*" while also being able to generate "myStar" - at least not with what we have now.

I think the solution is in some incantation of the Aeson-generic-derivation.
Maybe we need http://hackage.haskell.org/package/aeson-1.4.3.0/docs/Data-Aeson-Types.html#v:fieldLabelModifier for parsing & http://hackage.haskell.org/package/aeson-1.4.3.0/docs/Data-Aeson-Types.html#v:constructorTagModifier for generating? Or vice-versa?

Or we need to generate much more complicated code with explicit replacement for each case defined?

@f-f

f-f commented Jul 3, 2019

Copy link
Copy Markdown
ContributorAuthor

@Drezil sorry for delay, I just came back from vacation so now I have some time to look at your repro. I'll get back to you as soon as possible

@f-f

f-f commented Oct 31, 2019

Copy link
Copy Markdown
ContributorAuthor

@Drezil I realized I never got back to you on this, sorry for that. Unfortunately I'm not involved anymore with projects using this so I won't have time to continue working on it for now

@wing328

Copy link
Copy Markdown
Member

@f-f that's ok. We truly appreciate your contributions to this project

@Drezil please open a new issue to track #2676 (comment) if you've not done so.

Sign up for freeto join this conversation on GitHub. Already have an account? Sign in to comment

Projects

None yet

Development

Successfully merging this pull request may close these issues.

3 participants

@f-f@Drezil@wing328