Skip to content

Commit

Permalink
1599 optimize requests (#1653)
Browse files Browse the repository at this point in the history 
* chore(app): dont set database multiple times so that initial fetch is only ran once

* chore(app, platforms): process returned passport for each crud request

* chore(app): revert databse update function

* fix(database-client): update tests to match new request format

* chore(app): update ceramiccontext unit tests to account for api updates
  • Loading branch information
@tim-schultz
tim-schultz authored Aug 29, 2023
1 parent 1c8b754 commit ccad4c4
Show file tree
Hide file tree
Showing 4 changed files with 164 additions and 147 deletions.
156 changes: 72 additions & 84 deletions app/__tests__/context/ceramicContext.test.tsx
View file
Original file line number Diff line number Diff line change
Expand Up @@ -228,27 +228,24 @@ describe("CeramicContextProvider syncs stamp state with ceramic", () => {
(PassportDatabase as jest.Mock).mockImplementationOnce(() => {
return {
...passportDbMocks,
addStamps: addStampsMock,
getPassport: jest
.fn()
.mockImplementationOnce(async () => {
return {
passport: {
stamps: [],
},
errorDetails: {},
status: "Success",
};
})
.mockImplementationOnce(async () => {
return {
passport: {
stamps,
},
errorDetails: {},
status: "Success",
};
}),
addStamps: addStampsMock.mockImplementationOnce(async () => {
return {
passport: {
stamps,
},
errorDetails: {},
status: "Success",
};
}),
getPassport: jest.fn().mockImplementationOnce(async () => {
return {
passport: {
stamps: [],
},
errorDetails: {},
status: "Success",
};
}),
setStamps: setStampMock,
};
});
Expand Down Expand Up @@ -288,27 +285,24 @@ describe("CeramicContextProvider syncs stamp state with ceramic", () => {
(PassportDatabase as jest.Mock).mockImplementationOnce(() => {
return {
...passportDbMocks,
deleteStamps: deleteStampsMock,
getPassport: jest
.fn()
.mockImplementationOnce(async () => {
return {
passport: {
stamps,
},
errorDetails: {},
status: "Success",
};
})
.mockImplementationOnce(async () => {
return {
passport: {
stamps: [],
},
errorDetails: {},
status: "Success",
};
}),
deleteStamps: deleteStampsMock.mockImplementationOnce(async () => {
return {
passport: {
stamps: [],
},
errorDetails: {},
status: "Success",
};
}),
getPassport: jest.fn().mockImplementationOnce(async () => {
return {
passport: {
stamps: [],
},
errorDetails: {},
status: "Success",
};
}),
};
});
(CeramicDatabase as jest.Mock).mockImplementationOnce(() => {
Expand Down Expand Up @@ -346,27 +340,24 @@ describe("CeramicContextProvider syncs stamp state with ceramic", () => {
(PassportDatabase as jest.Mock).mockImplementationOnce(() => {
return {
...passportDbMocks,
patchStamps: patchStampsMock,
getPassport: jest
.fn()
.mockImplementationOnce(async () => {
return {
passport: {
stamps: [],
},
errorDetails: {},
status: "Success",
};
})
.mockImplementationOnce(async () => {
return {
passport: {
stamps: added,
},
errorDetails: {},
status: "Success",
};
}),
patchStamps: patchStampsMock.mockImplementationOnce(async () => {
return {
passport: {
stamps: added,
},
errorDetails: {},
status: "Success",
};
}),
getPassport: jest.fn().mockImplementationOnce(async () => {
return {
passport: {
stamps: added,
},
errorDetails: {},
status: "Success",
};
}),
};
});

Expand Down Expand Up @@ -400,27 +391,24 @@ describe("CeramicContextProvider syncs stamp state with ceramic", () => {
(PassportDatabase as jest.Mock).mockImplementationOnce(() => {
return {
...passportDbMocks,
patchStamps: patchStampsMock,
getPassport: jest
.fn()
.mockImplementationOnce(async () => {
return {
passport: {
stamps: [],
},
errorDetails: {},
status: "Success",
};
})
.mockImplementationOnce(async () => {
return {
passport: {
stamps,
},
errorDetails: {},
status: "Success",
};
}),
patchStamps: patchStampsMock.mockImplementationOnce(async () => {
return {
passport: {
stamps,
},
errorDetails: {},
status: "Success",
};
}),
getPassport: jest.fn().mockImplementationOnce(async () => {
return {
passport: {
stamps,
},
errorDetails: {},
status: "Success",
};
}),
};
});

Expand Down
66 changes: 42 additions & 24 deletions app/context/ceramicContext.tsx
View file
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { createContext, useContext, useEffect, useMemo, useState, useRef } from "react";
import { createContext, useContext, useEffect, useState, useRef, useCallback } from "react";
import {
Passport,
PassportLoadResponse,
Expand Down Expand Up @@ -383,7 +383,8 @@ export const CeramicContextProvider = ({ children }: { children: any }) => {
case "connected": {
if (dbAccessTokenStatus === "failed") {
setIsLoadingPassport(IsLoadingPassportState.FailedToConnect);
} else if (dbAccessToken && address) {
break;
} else if (dbAccessToken && address && !database) {
// Ceramic Network Connection
const ceramicClientInstance = new CeramicDatabase(
viewerConnection.selfID.did,
Expand Down Expand Up @@ -415,13 +416,13 @@ export const CeramicContextProvider = ({ children }: { children: any }) => {
default:
break;
}
}, [viewerConnection.status, address, dbAccessToken, dbAccessTokenStatus]);
}, [viewerConnection, address, dbAccessToken, dbAccessTokenStatus]);

useEffect(() => {
if (database && ceramicClient) {
if (database) {
fetchPassport(database, false, true);
}
}, [database, ceramicClient]);
}, [database]);

const passportLoadSuccess = (
database: CeramicDatabase | PassportDatabase,
Expand Down Expand Up @@ -457,17 +458,16 @@ export const CeramicContextProvider = ({ children }: { children: any }) => {
}
};

const fetchPassport = async (
const handlePassportUpdate = async (
passportResponse: PassportLoadResponse,
database: CeramicDatabase | PassportDatabase,
skipLoadingState?: boolean,
isInitialLoad?: boolean
): Promise<Passport | undefined> => {
if (!skipLoadingState) setIsLoadingPassport(IsLoadingPassportState.Loading);

// fetch, clean and set the new Passport state
const { status, errorDetails, passport } = await database.getPassport();
) => {
let passportToReturn: Passport | undefined = undefined;

const { status, errorDetails, passport } = passportResponse;

switch (status) {
case "Success":
passportToReturn = passportLoadSuccess(database, passport, skipLoadingState);
Expand All @@ -493,6 +493,19 @@ export const CeramicContextProvider = ({ children }: { children: any }) => {
return passportToReturn;
};

const fetchPassport = async (
database: CeramicDatabase | PassportDatabase,
skipLoadingState?: boolean,
isInitialLoad?: boolean
): Promise<Passport | undefined> => {
if (!skipLoadingState) setIsLoadingPassport(IsLoadingPassportState.Loading);

// fetch, clean and set the new Passport state
const getResponse = await database.getPassport();

return await handlePassportUpdate(getResponse, database, skipLoadingState, isInitialLoad);
};

const handleCheckRefreshPassport = async (): Promise<boolean> => {
let success = true;
if (ceramicClient && passportLoadResponse) {
Expand Down Expand Up @@ -600,10 +613,14 @@ export const CeramicContextProvider = ({ children }: { children: any }) => {
const handleAddStamps = async (stamps: Stamp[]): Promise<void> => {
try {
if (database) {
await database.addStamps(stamps);
const newPassport = await fetchPassport(database, true);
if (ceramicClient && newPassport) {
ceramicClient.setStamps(newPassport.stamps).catch((e) => console.log("error setting ceramic stamps", e));
const addResponse = await database.addStamps(stamps);

handlePassportUpdate(addResponse, database);

if (ceramicClient && addResponse.passport) {
ceramicClient
.setStamps(addResponse.passport.stamps)
.catch((e) => console.log("error setting ceramic stamps", e));
}
if (dbAccessToken) {
refreshScore(address, dbAccessToken);
Expand All @@ -618,10 +635,10 @@ export const CeramicContextProvider = ({ children }: { children: any }) => {
const handlePatchStamps = async (stampPatches: StampPatch[]): Promise<void> => {
try {
if (database) {
await database.patchStamps(stampPatches);
const newPassport = await fetchPassport(database, true);
const patchResponse = await database.patchStamps(stampPatches);
handlePassportUpdate(patchResponse, database);

if (ceramicClient && newPassport) {
if (ceramicClient && patchResponse.passport) {
(async () => {
try {
const deleteProviderIds = stampPatches
Expand All @@ -630,7 +647,7 @@ export const CeramicContextProvider = ({ children }: { children: any }) => {

if (deleteProviderIds.length) await ceramicClient.deleteStampIDs(deleteProviderIds);

await ceramicClient.setStamps(newPassport.stamps);
await ceramicClient.setStamps(patchResponse.passport?.stamps || []);
} catch (e) {
console.log("error patching ceramic stamps", e);
}
Expand All @@ -650,11 +667,12 @@ export const CeramicContextProvider = ({ children }: { children: any }) => {
const handleDeleteStamps = async (providerIds: PROVIDER_ID[]): Promise<void> => {
try {
if (database) {
await database.deleteStamps(providerIds);

const newPassport = await fetchPassport(database, true);
if (ceramicClient && newPassport) {
ceramicClient.setStamps(newPassport.stamps).catch((e) => console.log("error setting ceramic stamps", e));
const deleteResponse = await database.deleteStamps(providerIds);
handlePassportUpdate(deleteResponse, database);
if (ceramicClient && deleteResponse.status === "Success" && deleteResponse.passport?.stamps) {
ceramicClient
.setStamps(deleteResponse.passport.stamps)
.catch((e) => console.log("error setting ceramic stamps", e));
}
if (dbAccessToken) {
refreshScore(address, dbAccessToken);
Expand Down
18 changes: 12 additions & 6 deletions database-client/__tests__/passportScorerClient.test.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -65,7 +65,8 @@ describe("scorerClient", () => {

it("should log an error when axios.post fails", async () => {
const error = new Error("Request failed");
jest.spyOn(axios, "post").mockImplementation((url: string): Promise<{}> => {
const request = "post";
jest.spyOn(axios, request).mockImplementation((url: string): Promise<{}> => {
return new Promise((_, reject) => {
reject(error);
});
Expand All @@ -75,13 +76,15 @@ describe("scorerClient", () => {

expect(logger.info).toHaveBeenCalledWith(`adding stamp to passportScorer address: ${address}`);
expect(logger.error).toHaveBeenCalledWith(
`Error saving stamp to passportScorer address: ${address}:${error.toString()}`
`[Scorer] Error thrown when making ${request} for passport with did ${address}: Error: Request failed`,
{ error }
);
});

it("should log an error when axios.delete fails", async () => {
const error = new Error("Request failed");
jest.spyOn(axios, "delete").mockImplementation((url: string): Promise<{}> => {
const request = "delete";
jest.spyOn(axios, request).mockImplementation((url: string): Promise<{}> => {
return new Promise((_, reject) => {
reject(error);
});
Expand All @@ -94,7 +97,8 @@ describe("scorerClient", () => {
`deleting stamp from passportScorer for ${providerIds.join(", ")} on ${address}`
);
expect(logger.error).toHaveBeenCalledWith(
`Error deleting stamp from passportScorer for ${providerIds.join(", ")} on ${address}: Error: Request failed`
`[Scorer] Error thrown when making ${request} for passport with did ${address}: Error: Request failed`,
{ error }
);
});

Expand Down Expand Up @@ -132,7 +136,8 @@ describe("scorerClient", () => {

it("should log an error when axios.patch fails", async () => {
const error = new Error("Request failed");
jest.spyOn(axios, "patch").mockImplementation((url: string): Promise<{}> => {
const request = "patch";
jest.spyOn(axios, request).mockImplementation((url: string): Promise<{}> => {
return new Promise((_, reject) => {
reject(error);
});
Expand All @@ -142,7 +147,8 @@ describe("scorerClient", () => {

expect(logger.info).toHaveBeenCalledWith(`patching stamps in passportScorer for address: ${address}`);
expect(logger.error).toHaveBeenCalledWith(
`Error patching stamps in passportScorer for address: ${address}:${error.toString()}`
`[Scorer] Error thrown when making ${request} for passport with did ${address}: ${error.toString()}`,
{ error }
);
});
});
Loading

0 comments on commit ccad4c4

Please sign in to comment.