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 1 commit
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"
}
}
60 changes: 60 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,60 @@
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: any;

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

// converts JSON to binary data (Buffer)
static async fromJson(json: object): Promise<NewPayload> {
const jsonString = JSON.stringify(json);
const encoder = new TextEncoder();
const buffer = encoder.encode(jsonString);
return new NewPayload(buffer);
}

// converts file to binary data (Buffer)
static fromPath(path: string): NewPayload {
const realPath = realpathSync(path);
const contents = readFileSync(realPath);
return new NewPayload(new Uint8Array(contents));
}

// converts text to binary data (Buffer)
static async fromPlainText(text: string): Promise<NewPayload> {
Copy link
Member

Choose a reason for hiding this comment

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

return type is not a promise and remove async from function

const arrayBytes = new TextEncoder().encode(text);
return new NewPayload(arrayBytes);
}

async fromBinary(): Promise<NewPayload> {
return new NewPayload(this.data);
}

// converts binary data (Buffer) to JSON
async toJson(): Promise<any> {
Copy link
Member

Choose a reason for hiding this comment

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

again no async and return type shouldn't be a promise..

Also instead of any, is there any other type we can return that more closely resembles a JSON object? any means it can be string, number etc.

const decoder = new TextDecoder();
const jsonString = decoder.decode(this.data);
return JSON.parse(jsonString);
}

// convert binary data (Buffer) to file
async toFile(fileName: string): Promise<File> {
Copy link
Contributor

Choose a reason for hiding this comment

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

toPath might also make sense. It would use fs.writeFileSync to write it to disk, similarly how fromPath reads it using fs

const blob = new Blob([this.data]);
return new File([blob], fileName);
Copy link
Contributor

Choose a reason for hiding this comment

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

Isn't File class used in HTML File input? Or it is Node class? I am not familiar with it

}

// converts binary data (Buffer) to text
async toPlainText(): Promise<string> {
const decoder = new TextDecoder();
return decoder.decode(this.data);
}

toBinary(): Uint8Array {
return Uint8Array.from(this.data);
}

public getData(): any {
Copy link
Contributor

Choose a reason for hiding this comment

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

Lets avoid any

Copy link
Member

Choose a reason for hiding this comment

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

If you make the Payload class parameterised like Payload
Then you would be able to return proper types.

return this.data;
}
}
30 changes: 29 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 @@ -32,6 +35,16 @@ class AppwriteException extends Error {
}
}

function getBoundary(str: string): string {
const lines = str.replaceAll("\r\n", "\n").split("\n").reverse();
for (const line of lines) {
if (line !== "") {
return line.slice(0, -2).slice(2);
}
}
return "";
}

function getUserAgent() {
let ua = 'AppwriteNodeJSSDK/13.0.0';

Expand Down Expand Up @@ -331,7 +344,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 = getBoundary(body.toString());
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