Skip to content

Commit

Permalink
Merge pull request #62 from xmtp/rygine/attachments-update
Browse files Browse the repository at this point in the history
Refactor processing of attachment content types
  • Loading branch information
rygine authored Aug 24, 2023
2 parents e26ea06 + 0da898c commit 0cbf18e
Show file tree
Hide file tree
Showing 8 changed files with 181 additions and 70 deletions.
2 changes: 1 addition & 1 deletion .changeset/config.json
Original file line number Diff line number Diff line change
Expand Up @@ -12,5 +12,5 @@
"access": "public",
"baseBranch": "main",
"updateInternalDependencies": "patch",
"ignore": []
"ignore": ["@xmtp/react-quickstart-example", "@xmtp/react-components"]
}
5 changes: 5 additions & 0 deletions .changeset/gentle-snails-shave.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@xmtp/react-sdk": minor
---

Refactor processing of remote attachment content types. This update removes eager loading of remote data in the message processor in favor of the new `useAttachment` hook. With the `useAttachment` hook, developers can now respond to loading and error states when fetching the remote data.
16 changes: 11 additions & 5 deletions packages/react-components/src/components/AttachmentContent.tsx
Original file line number Diff line number Diff line change
@@ -1,8 +1,10 @@
import type { Attachment } from "@xmtp/content-type-remote-attachment";
import type { CachedMessage } from "@xmtp/react-sdk";
import { useAttachment } from "@xmtp/react-sdk";
import styles from "./Attachment.module.css";

export type AttachmentProps = {
attachment?: Attachment;
message: CachedMessage;
};

/**
Expand All @@ -27,10 +29,14 @@ const getBlobURL = (attachment: Attachment) => {
return blobCache.get(attachment.data)!;
};

export const AttachmentContent: React.FC<AttachmentProps> = ({
attachment,
}) => {
if (!attachment) {
export const AttachmentContent: React.FC<AttachmentProps> = ({ message }) => {
const { attachment, isLoading, error } = useAttachment(message);

if (error) {
return "Unable to load attachment";
}

if (isLoading || !attachment) {
return "Loading...";
}

Expand Down
4 changes: 2 additions & 2 deletions packages/react-components/src/components/MessageContent.tsx
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
import { ContentTypeId, ContentTypeText, getAttachment } from "@xmtp/react-sdk";
import { ContentTypeId, ContentTypeText } from "@xmtp/react-sdk";
import type { CachedMessage } from "@xmtp/react-sdk";
import {
ContentTypeAttachment,
Expand Down Expand Up @@ -33,7 +33,7 @@ export const MessageContent: React.FC<MessageContentProps> = ({
contentType.sameAs(ContentTypeAttachment) ||
contentType.sameAs(ContentTypeRemoteAttachment)
) {
content = <AttachmentContent attachment={getAttachment(message)} />;
content = <AttachmentContent message={message} />;
}

return (
Expand Down
111 changes: 72 additions & 39 deletions packages/react-sdk/src/helpers/caching/contentTypes/attachment.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,7 @@ describe("ContentTypeRemoteAttachment caching", () => {
});

describe("processAttachment", () => {
it("should save a message to the cache with attachment metadata", async () => {
it("should save a message to the cache", async () => {
const testClient = await Client.create(testWallet, { env: "local" });
const testConversation = {
id: 1,
Expand Down Expand Up @@ -89,9 +89,7 @@ describe("ContentTypeRemoteAttachment caching", () => {
updateConversationMetadata,
processors: attachmentContentTypeConfig.processors,
});
expect(persist).toHaveBeenCalledWith({
metadata: testMessage.content,
});
expect(persist).toHaveBeenCalledWith();
});

it("should not process a message with the wrong content type", async () => {
Expand Down Expand Up @@ -136,15 +134,7 @@ describe("ContentTypeRemoteAttachment caching", () => {
});

describe("processRemoteAttachment", () => {
it("should save a message to the cache with attachment metadata", async () => {
const testMetadata = {
filename: "testFilename",
mimeType: "testMimeType",
data: new Uint8Array(),
} satisfies Attachment;
const spy = vi
.spyOn(RemoteAttachmentCodec, "load")
.mockImplementationOnce(async () => Promise.resolve(testMetadata));
it("should save a message to the cache", async () => {
const testClient = await Client.create(testWallet, { env: "local" });
const testConversation = {
id: 1,
Expand Down Expand Up @@ -190,10 +180,7 @@ describe("ContentTypeRemoteAttachment caching", () => {
updateConversationMetadata,
processors: attachmentContentTypeConfig.processors,
});
expect(spy).toHaveBeenCalledWith(testMessage.content, testClient);
expect(persist).toHaveBeenCalledWith({
metadata: testMetadata,
});
expect(persist).toHaveBeenCalledWith();
});

it("should not process a message with the wrong content type", async () => {
Expand Down Expand Up @@ -238,8 +225,8 @@ describe("ContentTypeRemoteAttachment caching", () => {
});

describe("getAttachment", () => {
it("should return an attachment from cached message metadata (if present)", () => {
const testMetadata = {
it("should return an attachment from cached message", () => {
const testContent = {
filename: "testFilename",
mimeType: "testMimeType",
data: new Uint8Array(),
Expand All @@ -248,7 +235,7 @@ describe("ContentTypeRemoteAttachment caching", () => {
id: 1,
walletAddress: testWallet.address,
conversationTopic: "testTopic",
content: testMetadata,
content: testContent,
contentType: ContentTypeAttachment.toString(),
isSending: false,
hasSendError: false,
Expand All @@ -257,25 +244,62 @@ describe("ContentTypeRemoteAttachment caching", () => {
senderAddress: "testWalletAddress",
uuid: "testUuid",
xmtpID: "testXmtpId",
metadata: {
[attachmentContentTypeConfig.namespace]: testMetadata,
},
} satisfies CachedMessageWithId<Attachment>;

const attachment = getAttachment(testMessage);
expect(attachment).toEqual(testMetadata);
expect(attachment).toEqual(testContent);

const attachment2 = getAttachment({
...testMessage,
metadata: {},
});
expect(attachment2).toBeUndefined();
const testContent2 = {
url: "testUrl",
contentDigest: "testContentDigest",
salt: new Uint8Array(),
nonce: new Uint8Array(),
secret: new Uint8Array(),
scheme: "testScheme",
contentLength: 0,
filename: "testFilename",
} satisfies RemoteAttachment;
const testMessage2 = {
id: 2,
walletAddress: testWallet.address,
conversationTopic: "testTopic",
content: testContent2,
contentType: ContentTypeRemoteAttachment.toString(),
isSending: false,
hasSendError: false,
sentAt: new Date(),
status: "unprocessed",
senderAddress: "testWalletAddress",
uuid: "testUuid",
xmtpID: "testXmtpId",
} satisfies CachedMessageWithId<RemoteAttachment>;

const attachment2 = getAttachment(testMessage2);
expect(attachment2).toEqual(testContent2);

const testMessage3 = {
id: 3,
walletAddress: testWallet.address,
conversationTopic: "testTopic",
content: "foo",
contentType: ContentTypeText.toString(),
isSending: false,
hasSendError: false,
sentAt: new Date(),
status: "unprocessed",
senderAddress: "testWalletAddress",
uuid: "testUuid",
xmtpID: "testXmtpId",
} satisfies CachedMessageWithId;

const attachment3 = getAttachment(testMessage3);
expect(attachment3).toBeUndefined();
});
});

describe("hasAttachment", () => {
it("should return true if attachment metadata exists", () => {
const testMetadata = {
it("should return true if message is an attachment content type", () => {
const testContent = {
filename: "testFilename",
mimeType: "testMimeType",
data: new Uint8Array(),
Expand All @@ -284,7 +308,7 @@ describe("ContentTypeRemoteAttachment caching", () => {
id: 1,
walletAddress: testWallet.address,
conversationTopic: "testTopic",
content: testMetadata,
content: testContent,
contentType: ContentTypeAttachment.toString(),
isSending: false,
hasSendError: false,
Expand All @@ -293,18 +317,27 @@ describe("ContentTypeRemoteAttachment caching", () => {
senderAddress: "testWalletAddress",
uuid: "testUuid",
xmtpID: "testXmtpId",
metadata: {
[attachmentContentTypeConfig.namespace]: testMetadata,
},
} satisfies CachedMessageWithId<Attachment>;

const attachment = hasAttachment(testMessage);
expect(attachment).toBe(true);

const attachment2 = hasAttachment({
...testMessage,
metadata: {},
});
const testMessage2 = {
id: 2,
walletAddress: testWallet.address,
conversationTopic: "testTopic",
content: "foo",
contentType: ContentTypeText.toString(),
isSending: false,
hasSendError: false,
sentAt: new Date(),
status: "unprocessed",
senderAddress: "testWalletAddress",
uuid: "testUuid",
xmtpID: "testXmtpId",
} satisfies CachedMessageWithId;

const attachment2 = hasAttachment(testMessage2);
expect(attachment2).toBe(false);
});
});
Expand Down
40 changes: 18 additions & 22 deletions packages/react-sdk/src/helpers/caching/contentTypes/attachment.ts
Original file line number Diff line number Diff line change
Expand Up @@ -18,16 +18,23 @@ import { type CachedMessage } from "../messages";

const NAMESPACE = "attachment";

export type CachedAttachmentsMetadata = Attachment | undefined;

/**
* Get the attachment data from a cached message
*
* @param message Cached message
* @returns The attachment data, or `undefined` if the message has no attachment
* @returns The attachment data, or `undefined` if the message is not an
* attachment content type
*/
export const getAttachment = (message: CachedMessage) =>
message?.metadata?.[NAMESPACE] as CachedAttachmentsMetadata;
export const getAttachment = (message: CachedMessage) => {
switch (message.contentType) {
case ContentTypeAttachment.toString():
return message.content as Attachment;
case ContentTypeRemoteAttachment.toString():
return message.content as RemoteAttachment;
default:
return undefined;
}
};

/**
* Check if a cached message has an attachment
Expand Down Expand Up @@ -60,7 +67,7 @@ const isValidAttachmentContent = (content: unknown) => {
/**
* Process an attachment message
*
* The message content is also saved to the metadata of the message.
* Saves the message to the cache.
*/
export const processAttachment: ContentTypeMessageProcessor = async ({
message,
Expand All @@ -71,10 +78,8 @@ export const processAttachment: ContentTypeMessageProcessor = async ({
ContentTypeAttachment.sameAs(contentType) &&
isValidAttachmentContent(message.content)
) {
// save message to cache with the attachment metadata
await persist({
metadata: message.content as Attachment,
});
// save message to cache
await persist();
}
};

Expand Down Expand Up @@ -103,11 +108,9 @@ const isValidRemoveAttachmentContent = (content: unknown) => {
/**
* Process a remote attachment message
*
* Loads the attachment from the remote URL and saves it to the metadata
* of the message.
* Saves the message to the cache.
*/
export const processRemoteAttachment: ContentTypeMessageProcessor = async ({
client,
message,
persist,
}) => {
Expand All @@ -116,15 +119,8 @@ export const processRemoteAttachment: ContentTypeMessageProcessor = async ({
ContentTypeRemoteAttachment.sameAs(contentType) &&
isValidRemoveAttachmentContent(message.content)
) {
const attachment = await RemoteAttachmentCodec.load<Attachment>(
message.content as RemoteAttachment,
client,
);

// save message to cache with the attachment metadata
await persist({
metadata: attachment,
});
// save message to cache
await persist();
}
};

Expand Down
69 changes: 69 additions & 0 deletions packages/react-sdk/src/hooks/useAttachment.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,69 @@
import { useCallback, useEffect, useState } from "react";
import {
ContentTypeAttachment,
ContentTypeRemoteAttachment,
RemoteAttachmentCodec,
} from "@xmtp/content-type-remote-attachment";
import type {
RemoteAttachment,
Attachment,
} from "@xmtp/content-type-remote-attachment";
import { useDb } from "./useDb";
import type { CachedMessage } from "@/helpers/caching/messages";
import { useClient } from "@/hooks/useClient";

/**
* This hook returns the attachment data of a cached message
*/
export const useAttachment = (message: CachedMessage) => {
const { client } = useClient();
const { db } = useDb();
const [isLoading, setIsLoading] = useState(false);
const [error, setError] = useState<Error | undefined>(undefined);
const [attachment, setAttachment] = useState<Attachment | undefined>(
undefined,
);

const loadRemoteAttachment = useCallback(async () => {
if (client) {
try {
setIsLoading(true);
const loadedAttachment = await RemoteAttachmentCodec.load<Attachment>(
message.content as RemoteAttachment,
client,
);
setIsLoading(false);
setAttachment(loadedAttachment);
} catch (e) {
setError(new Error("Unable to load remote attachment"));
}
} else {
setError(new Error("XMTP client is required to load remote attachments"));
}
}, [client, message.content]);

useEffect(() => {
const getAttachment = async () => {
switch (message.contentType) {
case ContentTypeAttachment.toString(): {
setAttachment(message.content as Attachment);
break;
}
case ContentTypeRemoteAttachment.toString(): {
await loadRemoteAttachment();
break;
}
default:
setError(new Error("Message is not an attachment content type"));
}
};
void getAttachment();
}, [db, loadRemoteAttachment, message]);

return {
attachment,
error,
isLoading,
retry: loadRemoteAttachment,
};
};
Loading

0 comments on commit 0cbf18e

Please sign in to comment.