-
Notifications
You must be signed in to change notification settings - Fork 6
Description
There are several structures within the code that are "re-used" across retrieve and deploy and MetadataService and CloudDataService that contain fields that are not applicable in all scenarios that they are used in.
For example, the NlxPlan structure defines the response expected from the server when executing Get Retrieve Plan and Get Deploy Plan for both the MetadataService and CloudDataService plans.
Unfortunately, the actual response from the server and the NlxPlan definition different between the MetadataService and the CloudDataService and also across Get Retrieve Plan and Get Deploy Plan:
MetadataService- Response never appears to containHost,Port,appSpecificorallPermissionSetsfor eitherGet deploy PlanorGet Retrieve PlanCloudDataService- Response never appears to containallPermissionSetsand only containsappSpecificonGet Retrieve Plan
Another example is NlxPlanFilter which includes since field which is only applicable to retrieve APIs.
The generic "catch-all" approach to structure definition and usage results in numerous issues, especially considering there is no documentation on the APIs themselves publicly available, for example:
- Increases code complexity and cost of maintenance as anyone (internal to Skuid or external) trying to properly utilize these fields needs to have intimate knowledge of the actual response structure per API, when fields will have a value, when they won't, etc. It should not take inspecting the physical response returned and/or reviewing all the areas currently in the code (which may themselves be inaccurate) to see how fields are used and when they may (or may not be reliable) just to utilize a field in a structure
- Conditional statements (e.g., Host != "") are required in "generic handlers" because nothing can be explicit even the service (meta or data) or api (retrieve or deploy) is known
- Fields that do not even apply to certain APIs are sent to (or attempted to be marshalled back from) when they should not be sent and/or will never exist. For example, when
--appflag is passed, the value ofappSpecificis NEVER correct forMetadataServicebut it isfalseeverywhere and sent to the server asfalsewhen executing theMetadataServiceretrieve plan.
In order to eliminate incorrect fields being (un)marshalled and in order to decrease code complexity and ensure that anyone that is maintaining the code can clearly understand in/out expectations, data structures should not be shared unless their send/receive's are identical. The absolute worst case, and far from ideal, would be that the code at least contains comments in the structures that indicates when fields are reliable (e.g., they are sent to/returned from server) and when they are not.