Add support for callCached on GET methods. #12
Conversation
Simplify storing cached data.
| @@ -17,12 +20,29 @@ namespace Endpoints { | |||
| return call.then(response => response.data); | |||
| } | |||
|
|
|||
| static callCached<TView>(endpoint: IEndpoint, data) { | |||
| if (this.endpointCache[endpoint.toString()] == null) { | |||
| return this.call(endpoint, data).then(result => { | |||
There was a problem hiding this comment.
Do we need to do this.call<TView>(endpoint, data) here?
| @@ -17,12 +20,29 @@ namespace Endpoints { | |||
| return call.then(response => response.data); | |||
| } | |||
|
|
|||
| static callCached<TView>(endpoint: IEndpoint, data) { | |||
| if (this.endpointCache[endpoint.toString()] == null) { | |||
There was a problem hiding this comment.
I'd prefer to extract endpoint.toString() to a variable here. Perhaps cacheKey? And reuse below?
| @@ -12,6 +12,9 @@ public class Config | |||
| public string EndpointsOutputDirectory { get; set; } | |||
| = "Endpoints"; | |||
|
|
|||
| public bool EndpointsSupportCaching { get; set; } | |||
| = true; | |||
There was a problem hiding this comment.
Since we now have folks using it, can we default to false for all new features please?
| @@ -72,6 +93,13 @@ public void WriteServiceObjectToBlock(TypeScriptBlock serviceBlock, WebApiContro | |||
| var interfaceWithCallFullName = $"{Config.EndpointsNamespace}.{webApiController.Name}.I{actionName}WithCall"; | |||
| var endpointFullName = $"{Config.EndpointsNamespace}.{webApiController.Name}.{actionName}"; | |||
|
|
|||
| var cachedBlock = Config.EndpointsSupportCaching && | |||
| string.Equals(verb.Verb, "GET", StringComparison.InvariantCultureIgnoreCase) | |||
There was a problem hiding this comment.
Can we use WebApiHttpVerb.Get here and compare with the action verb?
| @@ -72,6 +93,13 @@ public void WriteServiceObjectToBlock(TypeScriptBlock serviceBlock, WebApiContro | |||
| var interfaceWithCallFullName = $"{Config.EndpointsNamespace}.{webApiController.Name}.I{actionName}WithCall"; | |||
| var endpointFullName = $"{Config.EndpointsNamespace}.{webApiController.Name}.{actionName}"; | |||
|
|
|||
| var cachedBlock = Config.EndpointsSupportCaching && | |||
There was a problem hiding this comment.
I'd prefer if we make this a boolean variable first, and then do a if (boolean) block.AddAndUseBlock below and get rid of the newly added AddBlock(block) methods just for consistency. We could then begin a discussion if we need to support the other format? I dislike null checks. Control flows without null is better I think?
There was a problem hiding this comment.
That won't work because Fluent way of writing this will end the block and the function will be outside of proper scope
| .AddAndUseBlock($"call<TView>({callArgumentDefinition})", isFunctionBlock: false, terminationString: ",") | ||
| .AddStatement($"return {Config.ServiceName}.call<TView>(this, {callArgumentValue});") | ||
| .Parent | ||
| .AddBlock(cachedBlock); |
There was a problem hiding this comment.
Save this to a variable and do a if (boolean) variable.AddAndUseBlock( /*create cached block here */)?
Resolves #8
Accidentally called branch issue-9 which is wrong.