Skip to content

Commit

Permalink
fix: API clients with response caching not passing parameters correctly
Browse files Browse the repository at this point in the history
  • Loading branch information
ascott18 committed Nov 18, 2024
1 parent 9d9e753 commit 39212a5
Show file tree
Hide file tree
Showing 8 changed files with 106 additions and 38 deletions.
6 changes: 6 additions & 0 deletions CHANGELOG.md
Original file line number Diff line number Diff line change
@@ -1,3 +1,9 @@
# 5.2.1

## Fixes

- API clients with response caching enabled were not passing parameters correctly

# 5.2.0

## Features
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ namespace IntelliTect.Coalesce.CodeGeneration.Tests
{
public class TargetClassesFullGenerationTest : CodeGenTestBase
{
// #IF directive so this doesn't needlessly run for multiple TFMs. It only needs to run for one.
#if NET8_0
/// <summary>
/// This test isn't asserting anything in .NET land.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ ExternalParent singleExternal

#nullable restore

[Coalesce, Execute(HttpMethod = HttpMethod.Get)]
public ExternalParent InstanceGetMethodWithObjParam(ExternalParent obj)
{
return obj;
}

[Coalesce, Execute]
public ExternalParentAsOutputOnly MethodWithExternalTypesWithSinglePurpose(
Expand Down
2 changes: 1 addition & 1 deletion src/coalesce-vue/src/api-client.ts
Original file line number Diff line number Diff line change
Expand Up @@ -1490,7 +1490,7 @@ export abstract class ApiState<

invoker = async () => {
const [promise, requests] = apiClient._observeRequests(
() => originalInvoker.apply(thisArg, [apiClient, ...arguments]),
() => originalInvoker.apply(thisArg, [apiClient, ...args]),
true
);
const { request, method } = requests[0];
Expand Down
78 changes: 41 additions & 37 deletions src/coalesce-vue/test/api-client.spec.ts
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,7 @@ import {
ComplexModelViewModel,
PersonListViewModel,
} from "@test-targets/viewmodels.g";
import { Person, Statuses } from "@test-targets/models.g";
import { ExternalParent, Person, Statuses } from "@test-targets/models.g";

function makeAdapterMock(result?: any) {
return makeEndpointMock<AxiosRequestConfig>(result);
Expand Down Expand Up @@ -898,23 +898,31 @@ describe("$makeCaller", () => {
});

describe("useResponseCaching", () => {
test("dehydrates and hydrates object results", async () => {
let studentId = 1;
AxiosClient.defaults.adapter = () =>
makeEndpointMock({
name: "steve",
studentWrapperObject: {
name: "bob",
student: {
studentId: studentId++,
name: "bob",
test.only("dehydrates and hydrates object results", async () => {
let requestNum = 1;

const mock = mockEndpoint(
new ComplexModelViewModel().$metadata.methods
.instanceGetMethodWithObjParam,
vitest.fn(async (req) => {
return {
wasSuccessful: true,
object: {
...req.params.obj,
valueArray: [requestNum++],
},
},
} as Advisor)();
};
})
);

const runTest = () => {
const caller = new StudentApiClient().$makeCaller("item", (c) =>
c.getWithObjParam(42, new Advisor({ name: "steve" }))
const caller = new ComplexModelApiClient().$makeCaller(
"item",
(c, param: number) =>
c.instanceGetMethodWithObjParam(
param,
new ExternalParent({ stringList: ["foo"] })
)
);
caller.useResponseCaching();
return caller;
Expand All @@ -923,12 +931,21 @@ describe("$makeCaller", () => {
// Make the first caller and invoke it, which will populate the cache.
const caller1 = runTest();
expect(caller1.result).toBeNull();
await caller1();
await caller1(42);
expect(caller1.result).not.toBeNull();
expect(caller1.result).toMatchObject(
new ExternalParent({ stringList: ["foo"], valueArray: [1] })
);
const cacheValue = Object.values(sessionStorage)[0];
expect(cacheValue).not.toBeFalsy();
expect(cacheValue).not.toContain("$metadata");

expect(mock).toBeCalledTimes(1);
expect(mock.mock.calls[0][0].params).toMatchObject({
id: 42,
obj: { stringList: ["foo"] },
});

// Make another caller. It will be dormant until invoked.
const caller2 = runTest();
expect(caller2.result).toBeNull();
Expand All @@ -937,18 +954,9 @@ describe("$makeCaller", () => {
expect(caller2.isLoading).toBe(false);

// Invoke the caller. At this point, the cached response will get loaded.
const caller2Promise = caller2();
const caller2Promise = caller2(42);
expect(caller2.result).toMatchObject(
new Advisor({
name: "steve",
studentWrapperObject: {
name: "bob",
student: {
studentId: 1,
name: "bob",
},
},
})
new ExternalParent({ stringList: ["foo"], valueArray: [1] })
);
expect(caller2.wasSuccessful).toBe(true);
expect(caller2.hasResult).toBe(true);
Expand All @@ -958,20 +966,16 @@ describe("$makeCaller", () => {
// Observe that the results are set with the new api response.
await caller2Promise;
expect(caller2.result).toMatchObject(
new Advisor({
name: "steve",
studentWrapperObject: {
name: "bob",
student: {
studentId: 2,
name: "bob",
},
},
})
new ExternalParent({ stringList: ["foo"], valueArray: [2] })
);
expect(caller2.wasSuccessful).toBe(true);
expect(caller2.hasResult).toBe(true);
expect(caller2.isLoading).toBe(false);
expect(mock).toBeCalledTimes(2);
expect(mock.mock.calls[1][0].params).toMatchObject({
id: 42,
obj: { stringList: ["foo"] },
});
});

test("respects stored max age", async () => {
Expand Down
9 changes: 9 additions & 0 deletions src/test-targets/api-clients.g.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

32 changes: 32 additions & 0 deletions src/test-targets/metadata.g.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

11 changes: 11 additions & 0 deletions src/test-targets/viewmodels.g.ts

Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.

0 comments on commit 39212a5

Please sign in to comment.