Skip to content

[Test PR] SDK Multipart support #93

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

Open
wants to merge 4 commits into
base: main
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 2 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
7 changes: 4 additions & 3 deletions package.json
Original file line number Diff line number Diff line change
@@ -1,5 +1,5 @@
{
"name": "node-appwrite",
"name": "node-appwrite-1",
"homepage": "https://appwrite.io/support",
"description": "Appwrite is an open-source self-hosted backend server that abstract and simplify complex and repetitive development tasks behind a very simple REST API",
"version": "13.0.0",
Expand Down Expand Up @@ -42,12 +42,13 @@
},
"devDependencies": {
"@types/node": "20.11.25",
"tsup": "7.2.0",
"esbuild-plugin-file-path-extensions": "^2.0.0",
"tslib": "2.6.2",
"tsup": "7.2.0",
"typescript": "5.4.2"
},
"dependencies": {
"node-fetch-native-with-agent": "1.7.2"
"node-fetch-native-with-agent": "1.7.2",
"parse-multipart-data": "^1.5.0"
}
}
47 changes: 47 additions & 0 deletions src/NewPayload.ts
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I dont see type change.. like ExecutionModel.. How do they know about this new Payload class being used for responseBody?

Copy link
Member Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I have added Payload response model in appwrite. Do we need to make any other change in SDK to use it?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Original file line number Diff line number Diff line change
@@ -0,0 +1,47 @@
import { realpathSync, readFileSync } from "fs";
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If we import fs this can't be used from Edge runtimes, such as Vercel/Cloudflare or Deno

export class NewPayload {
private data: Buffer;

constructor(data: Buffer) {
this.data = data;
}

public getData(): Buffer {
return this.data;
}

public static fromBuffer(buffer: Buffer): Buffer {
return Buffer.from(buffer);
}

public toBuffer(): Buffer {
return this.data;
}

public static fromString(string: string): Buffer {
return Buffer.from(string);
}

public toString(encoding: BufferEncoding = "utf8"): string {
return this.data.toString(encoding);
}

public static fromJSON(json: object): Buffer {
return Buffer.from(JSON.stringify(json));
}

public toJSON(): object {
return JSON.parse(this.data.toString());
}

public static fromPath(path: string): Buffer {
const realPath = realpathSync(path);
const contents = readFileSync(realPath);
return Buffer.from(contents);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Manually QA fromFile and fromPath. Upload a picture to Storage service. Then see the picture in Appwrite Console. Does it know it's PNG? Can you preview it? If not, we need to fix it.

}

public toFile(fileName: string): File {
const blob = new Blob([this.data]);
return new File([blob], fileName);
}
}
20 changes: 19 additions & 1 deletion src/client.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,9 @@
import { fetch, FormData, File } from 'node-fetch-native-with-agent';
import { createAgent } from 'node-fetch-native-with-agent/agent';
import { Models } from './models';
import { NewPayload } from './NewPayload';
import * as multipart from 'parse-multipart-data';
const { buffer } = require('node:stream/consumers');

type Payload = {
[key: string]: any;
Expand Down Expand Up @@ -331,7 +334,22 @@ class Client {
data = await response.json();
} else if (responseType === 'arrayBuffer') {
data = await response.arrayBuffer();
} else {
} else if (response.headers.get('content-type')?.includes('multipart/form-data')) {
const body = await buffer(response.body);
const boundary = multipart.getBoundary(response.headers.get('content-type') || '');
const parts = multipart.parse(body, boundary);
const partsObject = parts.reduce<{ [key: string]: Buffer }>((acc, part) => {
if (part.name) {
acc[part.name] = part.data;
}
return acc;
}, {});
data = {
...partsObject,
Copy link
Contributor

@Meldiron Meldiron Aug 14, 2024

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I think everything in multipart response is string. Try createExecution, and print it. See what is type of responseStatusCode, for example. It should be integer and I am worried it will be string. We need proper support for all types, as well as JSON (array/object).

responseBody: new NewPayload(partsObject.responseBody)
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ResponseBody can be empty. in which case it would probably be.. empty string? in which case, it might not fit the buffer type? Just thinking out lit. its something to test - to make async execution (that doesn't have body) and see if all works fine

}
}
else {
data = {
message: await response.text()
};
Expand Down
2 changes: 2 additions & 0 deletions src/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -33,3 +33,5 @@ export { ImageGravity } from './enums/image-gravity';
export { ImageFormat } from './enums/image-format';
export { PasswordHash } from './enums/password-hash';
export { MessagingProviderType } from './enums/messaging-provider-type';
export { NewPayload } from './NewPayload';
export { InputFile } from './inputFile';
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

We should remove all mention of InputFile. It should no longer exist.

With that, we also need to update existing endpoints that used to need it. they all should now use payload class

10 changes: 6 additions & 4 deletions src/services/functions.ts
Original file line number Diff line number Diff line change
Expand Up @@ -2,6 +2,7 @@ import { AppwriteException, Client, type Payload, UploadProgress } from '../clie
import type { Models } from '../models';
import { Runtime } from '../enums/runtime';
import { ExecutionMethod } from '../enums/execution-method';
import { NewPayload } from '../NewPayload';

export class Functions {
client: Client;
Expand Down Expand Up @@ -624,22 +625,22 @@ Use the &quot;command&quot; param to set the entrypoint used to execute your cod
* Trigger a function execution. The returned object will return you the current execution status. You can ping the `Get Execution` endpoint to get updates on the current execution status. Once this endpoint is called, your function execution process will start asynchronously.
*
* @param {string} functionId
* @param {string} body
* @param {NewPayload} body
* @param {boolean} async
* @param {string} xpath
* @param {ExecutionMethod} method
* @param {object} headers
* @throws {AppwriteException}
* @returns {Promise<Models.Execution>}
*/
async createExecution(functionId: string, body?: string, async?: boolean, xpath?: string, method?: ExecutionMethod, headers?: object): Promise<Models.Execution> {
async createExecution(functionId: string, body?: NewPayload, async?: boolean, xpath?: string, method?: ExecutionMethod, headers?: object): Promise<Models.Execution> {
if (typeof functionId === 'undefined') {
throw new AppwriteException('Missing required parameter: "functionId"');
}
const apiPath = '/functions/{functionId}/executions'.replace('{functionId}', functionId);
const payload: Payload = {};
if (typeof body !== 'undefined') {
payload['body'] = body;
payload['body'] = body ? body.getData : body;
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Why not getData(), with ()?

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Also, I dont understand the check. It doesn't make sense, right?

}
if (typeof async !== 'undefined') {
payload['async'] = async;
Expand All @@ -656,7 +657,8 @@ Use the &quot;command&quot; param to set the entrypoint used to execute your cod
const uri = new URL(this.client.config.endpoint + apiPath);

const apiHeaders: { [header: string]: string } = {
'content-type': 'application/json',
'content-type': 'multipart/form-data',
'accept': 'multipart/form-data',
}

return await this.client.call(
Expand Down
2 changes: 1 addition & 1 deletion tsconfig.json
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,7 @@
"strict": true,
"module": "es2022",
"moduleResolution": "Bundler",
"lib": ["es2022"]
"lib": ["dom", "es2022"]
},
"compileOnSave": false,
"exclude": ["node_modules", "dist"],
Expand Down