Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): delete connection from KERIA before DB (better reliability) #571

Merged
Merged
Show file tree
Hide file tree
Changes from 3 commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
21 changes: 20 additions & 1 deletion src/core/agent/services/connectionService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -75,6 +75,7 @@ const signifyClient = jest.mocked({
notifications: () => ({
list: jest.fn(),
mark: jest.fn(),
delete: jest.fn(),
}),
ipex: () => ({
admit: jest.fn(),
Expand Down Expand Up @@ -279,13 +280,29 @@ describe("Connection service of agent", () => {
});
});

test("cannot delete connection by id if KERIA fails", async () => {
Agent.agent.getKeriaOnlineStatus = jest.fn().mockReturnValueOnce(true);
connectionNoteStorage.findAllByQuery = jest.fn().mockReturnValue([]);
const connectionId = "connectionId";

deleteContactMock.mockRejectedValueOnce(
new Error(ConnectionService.FAILED_TO_DELETE_CONNECTION)
);

await expect(
connectionService.deleteConnectionById(connectionId)
).rejects.toThrowError(ConnectionService.FAILED_TO_DELETE_CONNECTION);
});

test("can delete conenction by id", async () => {
Agent.agent.getKeriaOnlineStatus = jest.fn().mockReturnValueOnce(true);
connectionNoteStorage.findAllByQuery = jest.fn().mockReturnValue([]);
const connectionId = "connectionId";

deleteContactMock.mockResolvedValue(null);
await connectionService.deleteConnectionById(connectionId);
expect(deleteContactMock).toBeCalledWith(connectionId);
expect(connectionStorage.deleteById).toBeCalledWith(connectionId);
// expect(deleteContactMock).toBeCalledWith(connectionId); // it should be uncommented later when deleting on KERIA is re-enabled
});

test("Should delete connection's notes when deleting that connection", async () => {
Expand All @@ -297,7 +314,9 @@ describe("Connection service of agent", () => {
},
]);
const connectionId = "connectionId";
deleteContactMock.mockResolvedValue(null);
await connectionService.deleteConnectionById(connectionId);
expect(deleteContactMock).toBeCalledWith(connectionId);
expect(connectionNoteStorage.deleteById).toBeCalledTimes(1);
});

Expand Down
9 changes: 8 additions & 1 deletion src/core/agent/services/connectionService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,6 +48,7 @@ class ConnectionService extends AgentService {
static readonly DEFAULT_ROLE = "agent";
static readonly FAILED_TO_RESOLVE_OOBI =
"Failed to resolve OOBI, operation not completing...";
static readonly FAILED_TO_DELETE_CONNECTION = "Failed to delete connection";

static resolvedOobi: { [key: string]: any } = {};

Expand Down Expand Up @@ -167,8 +168,14 @@ class ConnectionService extends AgentService {

@OnlineOnly
async deleteConnectionById(id: string): Promise<void> {
await this.props.signifyClient
.contacts()
.delete(id)
.catch((message) => {
throw message;
});

await this.connectionStorage.deleteById(id);
// await this.signifyApi.deleteContactById(id); @TODO - foconnor: Uncomment when KERIA endpoint fixed
iFergal marked this conversation as resolved.
Show resolved Hide resolved
const notes = await this.getConnectNotesByConnectionId(id);
for (const note of notes) {
this.connectionNoteStorage.deleteById(note.id);
Expand Down
37 changes: 37 additions & 0 deletions src/core/agent/services/ipexCommunicationService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import { CredentialStatus } from "./credentialService.types";
import { Agent } from "../agent";
import { IdentifierStorage } from "../records";
import { ConfigurationService } from "../../configuration";
import { SignifyNotificationService } from "./signifyNotificationService";

const notificationStorage = jest.mocked({
open: jest.fn(),
Expand Down Expand Up @@ -74,6 +75,7 @@ let getExchangeMock = jest.fn().mockImplementation((id: string) => {
const ipexOfferMock = jest.fn();
const ipexGrantMock = jest.fn();
const schemaGetMock = jest.fn();
const deleteNotificationMock = jest.fn();
const signifyClient = jest.mocked({
connect: jest.fn(),
boot: jest.fn(),
Expand Down Expand Up @@ -121,6 +123,7 @@ const signifyClient = jest.mocked({
notifications: () => ({
list: jest.fn(),
mark: jest.fn(),
delete: deleteNotificationMock,
}),
ipex: () => ({
admit: jest.fn().mockResolvedValue(["admit", "sigs", "aend"]),
Expand Down Expand Up @@ -184,6 +187,32 @@ describe("Ipex communication service of agent", () => {
beforeAll(async () => {
await new ConfigurationService().start();
});

test("can accept ACDC if deleting KERIA fails", async () => {
Agent.agent.getKeriaOnlineStatus = jest.fn().mockReturnValueOnce(true);
const id = "uuid";
identifierStorage.getIdentifierMetadata = jest.fn().mockResolvedValue({
signifyName: "holder",
});
credentialListMock.mockResolvedValue([
{
sad: {
d: "id",
},
},
]);
credentialStorage.getCredentialMetadata = jest.fn().mockResolvedValue({
id: "id",
});

deleteNotificationMock.mockRejectedValueOnce(
new Error(SignifyNotificationService.FAILED_TO_DELETE_NOTIFICATION)
);
await expect(ipexCommunicationService.acceptAcdc(id)).rejects.toThrowError(
SignifyNotificationService.FAILED_TO_DELETE_NOTIFICATION
);
});

test("can accept ACDC", async () => {
Agent.agent.getKeriaOnlineStatus = jest.fn().mockReturnValueOnce(true);
const id = "uuid";
Expand All @@ -200,6 +229,7 @@ describe("Ipex communication service of agent", () => {
credentialStorage.getCredentialMetadata = jest.fn().mockResolvedValue({
id: "id",
});
deleteNotificationMock.mockResolvedValue(null);
await ipexCommunicationService.acceptAcdc(id);
expect(credentialStorage.saveCredentialMetadataRecord).toBeCalledWith(
expect.objectContaining({
Expand All @@ -210,6 +240,7 @@ describe("Ipex communication service of agent", () => {
id: "id",
status: CredentialStatus.CONFIRMED,
});
expect(deleteNotificationMock).toBeCalledWith(id);
expect(notificationStorage.deleteById).toBeCalledWith(id);
});

Expand Down Expand Up @@ -317,13 +348,16 @@ describe("Ipex communication service of agent", () => {
signifyName: "abc123",
});
ipexOfferMock.mockResolvedValue(["offer", "sigs", "gend"]);
deleteNotificationMock.mockResolvedValue(null);

await ipexCommunicationService.offerAcdcFromApply(noti, {});
expect(ipexOfferMock).toBeCalledWith({
senderName: "abc123",
recipient: "i",
acdc: expect.anything(),
apply: "d",
});
expect(deleteNotificationMock).toBeCalledWith(id);
expect(notificationStorage.deleteById).toBeCalledWith(id);
});

Expand Down Expand Up @@ -399,6 +433,8 @@ describe("Ipex communication service of agent", () => {
signifyName: "abc123",
});
ipexGrantMock.mockResolvedValue(["offer", "sigs", "gend"]);
deleteNotificationMock.mockResolvedValue(null);

await ipexCommunicationService.grantAcdcFromAgree(noti);
expect(ipexGrantMock).toBeCalledWith({
acdc: {},
Expand All @@ -410,6 +446,7 @@ describe("Ipex communication service of agent", () => {
recipient: "i",
senderName: "abc123",
});
expect(deleteNotificationMock).toBeCalledWith(id);
expect(notificationStorage.deleteById).toBeCalledWith(id);
});

Expand Down
20 changes: 20 additions & 0 deletions src/core/agent/services/ipexCommunicationService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -112,6 +112,13 @@ class IpexCommunicationService extends AgentService {
credentialId,
cred
);

await this.props.signifyClient
.notifications()
.delete(id)
.catch((message) => {
throw message;
});
await this.notificationStorage.deleteById(id);
this.props.eventService.emit<AcdcStateChangedEvent>({
type: AcdcEventTypes.AcdcStateChanged,
Expand Down Expand Up @@ -140,6 +147,12 @@ class IpexCommunicationService extends AgentService {
await this.props.signifyClient
.ipex()
.submitOffer(holderSignifyName, offer, sigs, end, [msg.exn.i]);
await this.props.signifyClient
.notifications()
.delete(notification.id)
.catch((message) => {
throw message;
});
await this.notificationStorage.deleteById(notification.id);
}

Expand Down Expand Up @@ -174,6 +187,13 @@ class IpexCommunicationService extends AgentService {
await this.props.signifyClient
.ipex()
.submitGrant(holderSignifyName, grant, sigs, end, [msgAgree.exn.i]);

await this.props.signifyClient
iFergal marked this conversation as resolved.
Show resolved Hide resolved
.notifications()
.delete(notification.id)
.catch((message) => {
throw message;
});
await this.notificationStorage.deleteById(notification.id);
}

Expand Down
26 changes: 26 additions & 0 deletions src/core/agent/services/multiSigService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,6 +7,7 @@ import { Agent } from "../agent";
import { EventService } from "./eventService";
import { MultiSigService } from "./multiSigService";
import { IdentifierStorage } from "../records";
import { SignifyNotificationService } from "./signifyNotificationService";

const notificationStorage = jest.mocked({
open: jest.fn(),
Expand All @@ -31,6 +32,8 @@ let groupGetRequestMock = jest.fn();
let queryKeyStateMock = jest.fn();
let queryKeyStateGetMock = jest.fn();

const deleteNotificationMock = jest.fn();

const signifyClient = jest.mocked({
connect: jest.fn(),
boot: jest.fn(),
Expand Down Expand Up @@ -71,6 +74,7 @@ const signifyClient = jest.mocked({
notifications: () => ({
list: jest.fn(),
mark: jest.fn(),
delete: deleteNotificationMock,
}),
ipex: () => ({
admit: jest.fn(),
Expand Down Expand Up @@ -411,6 +415,24 @@ describe("Multisig sig service of agent", () => {
});
});

test("Cannot join the multisig if deleting KERIA fails", async () => {
Agent.agent.getKeriaOnlineStatus = jest.fn().mockReturnValueOnce(true);
groupGetRequestMock = jest.fn().mockResolvedValue([]);

deleteNotificationMock.mockRejectedValue(
new Error(SignifyNotificationService.FAILED_TO_DELETE_NOTIFICATION)
);
multiSigService.hasJoinedMultisig = jest.fn().mockResolvedValue(true);
await expect(
multiSigService.joinMultisig("id", "d", {
theme: 0,
displayName: "Multisig",
})
).rejects.toThrowError(
SignifyNotificationService.FAILED_TO_DELETE_NOTIFICATION
);
});

test("can join the multisig inception", async () => {
Agent.agent.getKeriaOnlineStatus = jest.fn().mockReturnValueOnce(true);
const multisigIdentifier = "newMultisigIdentifierAid";
Expand Down Expand Up @@ -489,6 +511,8 @@ describe("Multisig sig service of agent", () => {
],
};
});

deleteNotificationMock.mockResolvedValue(null);
expect(
await multiSigService.joinMultisig("id", "d", {
theme: 0,
Expand Down Expand Up @@ -943,6 +967,8 @@ describe("Multisig sig service of agent", () => {
i: metadata.id,
},
});

deleteNotificationMock.mockResolvedValue(null);
expect(
await multiSigService.joinMultisigRotation({
id: "id",
Expand Down
22 changes: 21 additions & 1 deletion src/core/agent/services/multiSigService.ts
Original file line number Diff line number Diff line change
Expand Up @@ -315,11 +315,18 @@ class MultiSigService extends AgentService {
aid,
multiSig.signifyName
);

await this.props.signifyClient
.notifications()
.delete(notification.id)
.catch((message) => {
throw message;
});
await this.notificationStorage.deleteById(notification.id);
return res.op.name.split(".")[1];
}

private async hasJoinedMultisig(msgSaid: string): Promise<boolean> {
async hasJoinedMultisig(msgSaid: string): Promise<boolean> {
const notifications: MultiSigExnMessage[] = await this.props.signifyClient
.groups()
.getRequest(msgSaid)
Expand Down Expand Up @@ -417,6 +424,12 @@ class MultiSigService extends AgentService {
// @TODO - foconnor: getMultisigDetails already has much of this done so this method signature could be adjusted.
const hasJoined = await this.hasJoinedMultisig(notificationSaid);
if (hasJoined) {
await this.props.signifyClient
iFergal marked this conversation as resolved.
Show resolved Hide resolved
.notifications()
.delete(notificationId)
.catch((message) => {
throw message;
});
await this.notificationStorage.deleteById(notificationId);
return;
}
Expand Down Expand Up @@ -458,6 +471,13 @@ class MultiSigService extends AgentService {
.get(identifier?.signifyName);
const signifyName = uuidv4();
const res = await this.joinMultisigKeri(exn, aid, signifyName);

await this.props.signifyClient
.notifications()
.delete(notificationId)
.catch((message) => {
throw message;
});
await this.notificationStorage.deleteById(notificationId);
const op = res.op;
const multisigId = op.name.split(".")[1];
Expand Down
20 changes: 20 additions & 0 deletions src/core/agent/services/signifyNotificationService.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,8 @@ const groupGetRequestMock = jest.fn();
const oobiResolveMock = jest.fn();
const queryKeyStateMock = jest.fn();

const deleteNotificationMock = jest.fn();

const signifyClient = jest.mocked({
connect: jest.fn(),
boot: jest.fn(),
Expand Down Expand Up @@ -54,6 +56,7 @@ const signifyClient = jest.mocked({
notifications: () => ({
list: jest.fn(),
mark: jest.fn(),
delete: deleteNotificationMock,
}),
ipex: () => ({
admit: jest.fn(),
Expand Down Expand Up @@ -189,10 +192,27 @@ describe("Signify notification service of agent", () => {
).rejects.toThrowError(SignifyNotificationService.NOTIFICATION_NOT_FOUND);
});

test("can delete keri notification by ID if KERIA fails", async () => {
const id = "uuid";

deleteNotificationMock.mockRejectedValueOnce(
new Error(SignifyNotificationService.FAILED_TO_DELETE_NOTIFICATION)
);

await expect(
signifyNotificationService.deleteNotificationRecordById(id)
).rejects.toThrowError(
SignifyNotificationService.FAILED_TO_DELETE_NOTIFICATION
);
});

test("can delete keri notification by ID", async () => {
const id = "uuid";

deleteNotificationMock.mockResolvedValue(null);
await signifyNotificationService.deleteNotificationRecordById(id);
expect(notificationStorage.deleteById).toBeCalled();
expect(deleteNotificationMock).toBeCalled();
});

test("Should skip if there is no valid multi-sig notification", async () => {
Expand Down
Loading
Loading