WebOfTrust / keria

KERI Agent in the cloud
https://keria.readthedocs.io/en/latest/
Apache License 2.0
18 stars 29 forks source link

Contact not added correctly after a previous deletion #292

Open lenkan opened 1 week ago

lenkan commented 1 week ago

Steps to reproduce

  1. Create wallet with identifier X and authorize the agent end role
  2. Create wallet with identifier Y and authorize the agent end role
  3. Resolve the OOBI of identifier Y from the wallet of identifier X, use an alias to ensure that the contact is added
  4. GET /contacts/{prefix} => verify that the correct alias and id is returned.
  5. DELETE /contacts/{prefix}
  6. Resolve the OOBI of identifier Y again, also using an alias.
  7. GET /contacts/{prefix} => verify that the correct alias and id is returned.

Actual result

HTTP 404 is returned.

Notes

See reproduction https://github.com/nordlei/vlei-sandbox/blob/main/src/issues/contact-not-added-after-deletion.test.ts

git clone git@github.com:nordlei/vlei-sandbox.git
cd vlei-sandbox
docker compose run --rm --build test npm start src/issues/contact-not-added-after-deletion.test.ts

It seems like the contact attributes are removed from the contact.

See test

import { beforeAll, expect, test } from "vitest";
import { TestWallet } from "./test-wallet.ts";
import { randomUUID } from "crypto";

const wallet1 = new TestWallet({
  alias: "alias1",
});

const wallet2 = new TestWallet({
  alias: "alias2",
});

function sleep(ms: number) {
  return new Promise((resolve) => setTimeout(resolve, ms));
}

beforeAll(async () => {
  await Promise.all([wallet1.init(), wallet2.init()]);
});

test("Resolve OOBI, verify contact", async () => {
  let operation = await wallet1.client.oobis().resolve(await wallet2.generateOobi(), wallet2.identifier.name);
  operation = await wallet1.client.operations().wait(operation);

  expect(operation).toMatchObject({ done: true });

  expect(await wallet1.client.contacts().get(wallet2.identifier.prefix)).toMatchObject({
    id: wallet2.identifier.prefix,
    alias: wallet2.identifier.name,
  });
});

test("Delete contact, resolve OOBI, get contact", async () => {
  await wallet1.client.contacts().delete(wallet2.identifier.prefix);

  let operation = await wallet1.client.oobis().resolve(await wallet2.generateOobi(), wallet2.identifier.name);
  operation = await wallet1.client.operations().wait(operation);

  expect(operation).toMatchObject({ done: true });

  // Need to add this sleep for the contact to become available, even though the operation is finished
  // await sleep(1000);

  expect(await wallet1.client.contacts().get(wallet2.identifier.prefix)).toMatchObject({
    id: wallet2.identifier.prefix,
    alias: wallet2.identifier.name,
  });
});

It looks like I need to add a delay after the operation has resolved before the contact is properly added to the list of contacts.

iFergal commented 2 days ago

The problem here is the long running operation is related to the OOBI and not the contact. The OOBI has been previously resolved (and is never removed (we don't unresolve for contact deletion)) so the operation is done: True immediately - while the actual second client call to oobi().resolve() is still processing again and creating the new contact.

I think the cleanest solution might be to have a new OpTypes.contact that is returned if the alias is specified instead of OpTypes.oobi. What do you think?

lenkan commented 2 days ago

The problem here is the long running operation is related to the OOBI and not the contact. The OOBI has been previously resolved (and is never removed (we don't unresolve for contact deletion)) so the operation is done: True immediately - while the actual second client call to oobi().resolve() is still processing again and creating the new contact.

I think the cleanest solution might be to have a new OpTypes.contact that is returned if the alias is specified instead of OpTypes.oobi. What do you think?

Thanks for putting some thought into it! I agree that would be cleaner. I think we can even take it one step further even - what if the "resolve oobi" is completely independent from contacts. If you want to add a contact you would POST / PUT a contact resource that contains alias and AID. The creation of contact would fail if the oobi is not yet resolved.

This is what we have to do often times anyway if we want to add more attributes to the contact.

If we do it that way, the resolving of an oobi would never affect the contact database.

What do you think?

iFergal commented 2 days ago

Yeah I wouldn't be opposed to that since the second call would be necessary anyway for the attributes, and the OOBI resolution is idempotent if it fails before creating the contact.

This is inherited from keripy though so we'd have to see if it makes sense with @pfeairheller and others.