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

Add dataset upload test (without using frontend) #8184

Merged
merged 10 commits into from
Nov 13, 2024
98 changes: 98 additions & 0 deletions frontend/javascripts/test/backend-snapshot-tests/datasets.e2e.ts
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import {
import type { APIDataset } from "types/api_flow_types";
import * as api from "admin/admin_rest_api";
import test from "ava";
import fs from "node:fs";

async function getFirstDataset(): Promise<APIDataset> {
const datasets = await api.getActiveDatasetsOfMyOrganization();
Expand Down Expand Up @@ -108,3 +109,100 @@ test("Zarr 3 streaming", async (t) => {
const base64 = btoa(String.fromCharCode(...new Uint8Array(bytes.slice(-128))));
t.snapshot(base64);
});

test("Dataset upload", async (t) => {
const uploadId = "test-dataset-upload-" + Date.now();

await fetch("/data/datasets/reserveUpload", {
method: "POST",
headers: new Headers({
"Content-Type": "application/json",
}),
body: JSON.stringify({
filePaths: ["test-dataset-upload.zip"],
folderId: "570b9f4e4bb848d0885ea917",
initialTeams: [],
layersToLink: [],
name: "test-dataset-upload",
organization: "Organization_X",
totalFileCount: 1,
uploadId: uploadId,
}),
});

const filePath = "test/dataset/test-dataset.zip";
const testDataset = fs.readFileSync(filePath);

let formData = new FormData();
formData.append("resumableChunkNumber", "1");
formData.append("resumableChunkSize", "10485760");
formData.append("resumableCurrentChunkSize", "71988");
formData.append("resumableTotalSize", "71988");
formData.append("resumableType", "application/zip");
formData.append("resumableIdentifier", uploadId + "/test-dataset.zip");
formData.append("resumableFilename", "test-dataset.zip");
formData.append("resumableRelativePath", "test-dataset.zip");
formData.append("resumableTotalChunks", "1");

// Setting the correct content type header automatically does not work (the boundary is not included)
// We can not extract the boundary from the FormData object
// Thus we have to set the content type header ourselves and create the body manually

const boundary = "----WebKitFormBoundaryAqTsFa4N9FW7zF7I";
let bodyString = `--${boundary}\r\n`;
// @ts-ignore
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Resolve the TypeScript error instead of using // @ts-ignore

Using // @ts-ignore suppresses TypeScript errors, potentially hiding real issues. Instead, address the type mismatch by casting formData appropriately or adjusting the code to satisfy the compiler.

Apply this change to resolve the typing issue:

- // @ts-ignore
  for (const [key, value] of formData.entries()) {
+ for (const [key, value] of (formData as any).entries()) {

Committable suggestion skipped: line range outside the PR's diff.

for (const [key, value] of formData.entries()) {
bodyString += `Content-Disposition: form-data; name="${key}"\r\n\r\n${value}\r\n`;
bodyString += `--${boundary}\r\n`;
}
bodyString += `Content-Disposition: form-data; name="file"; filename="test-dataset.zip"\r\n`;
bodyString += "Content-Type: application/octet-stream\r\n\r\n";

// We have to send the file as bytes, otherwise JS does some encoding, resulting in erroneous bytes

const formBytes = new TextEncoder().encode(bodyString);
const fileBytes = new Uint8Array(testDataset);
const endBytes = new TextEncoder().encode(`\r\n--${boundary}--`);
const body = new Uint8Array(formBytes.length + fileBytes.length + endBytes.length);
body.set(formBytes, 0);
body.set(fileBytes, formBytes.length);
body.set(endBytes, formBytes.length + fileBytes.length);

Comment on lines +151 to +170
Copy link
Contributor

Choose a reason for hiding this comment

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

🛠️ Refactor suggestion

Simplify multipart/form-data handling by using built-in FormData methods

Manually constructing the multipart/form-data body and hardcoding the boundary can lead to errors and maintenance challenges. Use the built-in FormData object directly with the fetch API to handle multipart/form-data encoding automatically.

Apply this diff to simplify the upload request:

- // Manual construction of the multipart/form-data body
- const boundary = "----WebKitFormBoundaryAqTsFa4N9FW7zF7I";
- let bodyString = `--${boundary}\r\n`;
- // ...additional code to construct `bodyString`...
- const formBytes = new TextEncoder().encode(bodyString);
- const fileBytes = new Uint8Array(testDataset);
- const endBytes = new TextEncoder().encode(`\r\n--${boundary}--`);
- const body = new Uint8Array(formBytes.length + fileBytes.length + endBytes.length);
- body.set(formBytes, 0);
- body.set(fileBytes, formBytes.length);
- body.set(endBytes, formBytes.length + fileBytes.length);
- let content_type = `multipart/form-data; boundary=${boundary}`;

+ formData.append("file", new Blob([testDataset], { type: "application/octet-stream" }), "test-dataset.zip");
+
+ const uploadResult = await fetch("/data/datasets", {
+   method: "POST",
+   body: formData,
+ });

Committable suggestion skipped: line range outside the PR's diff.

let content_type = `multipart/form-data; boundary=${boundary}`;

const uploadResult = await fetch("/data/datasets", {
method: "POST",
headers: new Headers({
"Content-Type": content_type,
}),
body: body,
});

if (uploadResult.status !== 200) {
t.fail("Dataset upload failed");
}

const finishResult = await fetch("/data/datasets/finishUpload", {
method: "POST",
headers: new Headers({
"Content-Type": "application/json",
}),
body: JSON.stringify({
uploadId: uploadId,
needsConversion: false,
}),
});

if (finishResult.status !== 200) {
t.fail("Dataset upload failed at finish");
}

const result = await fetch("/api/datasets/Organization_X/test-dataset-upload/health", {
headers: new Headers(),
});

if (result.status !== 200) {
t.fail("Dataset health check after upload failed");
}
t.pass();
});
6 changes: 4 additions & 2 deletions frontend/javascripts/test/e2e-setup.ts
Original file line number Diff line number Diff line change
Expand Up @@ -3,7 +3,7 @@ import _ from "lodash";
// @ts-expect-error ts-migrate(7016) FIXME: Could not find a declaration file for module 'deep... Remove this comment to see the full error message
import deepForEach from "deep-for-each";
// @ts-expect-error ts-migrate(7016) FIXME: Could not find a declaration file for module 'node... Remove this comment to see the full error message
import fetch, { Headers, Request, Response, FetchError } from "node-fetch";
import fetch, { Headers, FormData, Request, Response, FetchError, File } from "node-fetch";
import fs from "node:fs";
// @ts-expect-error ts-migrate(7016) FIXME: Could not find a declaration file for module 'shel... Remove this comment to see the full error message
import shell from "shelljs";
Expand Down Expand Up @@ -67,7 +67,7 @@ global.fetch = function fetchWrapper(url, options) {
let newUrl = url;

// @ts-expect-error ts-migrate(2339) FIXME: Property 'indexOf' does not exist on type 'Request... Remove this comment to see the full error message
if (url.indexOf("http:") === -1) {
if (url.indexOf("http:") === -1 && url.indexOf("https:") === -1) {
newUrl = `http://localhost:9000${url}`;
}

Expand All @@ -84,6 +84,8 @@ global.Request = Request;
global.Response = Response;
// @ts-ignore FIXME: Element implicitly has an 'any' type because type ... Remove this comment to see the full error message
global.FetchError = FetchError;
global.FormData = FormData;
global.File = File;

const { JSDOM } = require("jsdom");

Expand Down
8 changes: 5 additions & 3 deletions test/e2e/End2EndSpec.scala
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
package e2e

import com.scalableminds.util.io.ZipIO
import com.scalableminds.util.io.{PathUtils, ZipIO}
import com.typesafe.scalalogging.LazyLogging
import org.scalatestplus.play.guice._
import org.specs2.main.Arguments
Expand Down Expand Up @@ -51,9 +51,11 @@ class End2EndSpec(arguments: Arguments) extends Specification with GuiceFakeAppl
private def ensureTestDataset(): Unit = {
val testDatasetPath = "test/dataset/test-dataset.zip"
val dataDirectory = new File("binaryData/Organization_X")
if (!dataDirectory.exists()) {
dataDirectory.mkdirs()
if (dataDirectory.exists()) {
println("Deleting existing data directory Organization_X")
PathUtils.deleteDirectoryRecursively(dataDirectory.toPath)
}
dataDirectory.mkdirs()
val testDatasetZip = new File(testDatasetPath)
if (!testDatasetZip.exists()) {
throw new Exception("Test dataset zip file does not exist.")
Expand Down