fix: [OData] Name mapping bug in NamingUtils#1026
Conversation
| .replaceAll("^(?i)ODataServiceFor", "") | ||
| .replaceAll("^(?i)RemoteApiFor", "") | ||
| .replaceAll("^(?i)ApiFor", "") | ||
| .replaceAll("^(?i)Api", "") |
There was a problem hiding this comment.
(Major)
Wouldn't this match edge-cases like "SapInvoice"?
I would probably rather go with something like "A(pi|PI)" or straight up two lines.
Edit: I realize you changed the replacement from "anywhere in the string" to "only in the beginning". Therefore potentially bringing a breaking change for existing users that rely on "Api" being removed from their service name(?).
Which would be fine if we had a major release or sth.
There was a problem hiding this comment.
Yes, It is breaking change.
My rational was, to be consistent across regeneration, we have three options
Always remove "Api"breaking to users relying on Api
Always keep "Api"breaking to users who have been regenerating with.edmxfile updates.
Don't apply any transformations to mappings that are read fromserviceNameMappings.propertiesI didn't feel confident about removing this additional transformation as it might be a safety measure (?). But, it would be non-breaking.
What do you think about Option 3?
There was a problem hiding this comment.
I didnt get your comment the first time. I have updated the regex.
There was a problem hiding this comment.
It still is not edge-case free. Eg: "Apiary" would be matched. But this is unavoidable
NamingUtils
Context
NamingUtilscurrently makes unintentional changes to the mappings inserviceNameMappings.propertiesIf the file name is
API MATERIAL DOCUMENT SRV.edmxFirst run:
"API_MATERIAL_DOCUMENT_SRV"(from .edmx filename)"APIMaterialDocumentSrv".replace("Api", ""): No match (looking for "Api", found "API")"APIMATERIALDOCUMENTSRV""APIMATERIALDOCUMENTSRV".toLowerCase()="apimaterialdocumentsrv"Second run (re-processing stored value):
APIMATERIALDOCUMENTSRVfor className and"apimaterialdocumentsrv"for packageName (from mapping)"Apimaterialdocumentsrv".replace("Api", ""): MATCHES! Removes "Api""materialdocumentsrv"❌Even though the className stays consistent across runs, the result is semantically flawed since prefix "Api" is not removed.
Feature scope:
Definition of Done
Error handling created / updated & covered by the tests aboveDocumentation updated