Skip to content

Commit

Permalink
fix: returns a FileResult with size/path #1081
Browse files Browse the repository at this point in the history
  • Loading branch information
bamthomas committed Dec 4, 2023
1 parent c8947f8 commit 7829cf0
Show file tree
Hide file tree
Showing 9 changed files with 50 additions and 29 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -41,8 +41,8 @@ public Integer call() {

if (currentTask != null && !POISON.equals(currentTask)) {
BatchDownloadRunner downloadRunner = factory.createDownloadRunner(currentTask, taskSupplier::progress);
File zipResult = downloadRunner.call();
taskSupplier.result(currentTask.id, zipResult);
FileResult result = downloadRunner.call();
taskSupplier.result(currentTask.id, result);
nbTasks++;
}
} catch (Throwable ex) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,14 +47,13 @@
import static java.lang.Integer.min;
import static java.lang.Integer.parseInt;
import static java.lang.String.valueOf;
import static java.util.Optional.ofNullable;
import static java.util.stream.Collectors.toList;
import static org.icij.datashare.cli.DatashareCliOptions.BATCH_DOWNLOAD_MAX_NB_FILES;
import static org.icij.datashare.cli.DatashareCliOptions.BATCH_DOWNLOAD_MAX_SIZE;
import static org.icij.datashare.cli.DatashareCliOptions.BATCH_THROTTLE;
import static org.icij.datashare.cli.DatashareCliOptions.SCROLL_SIZE;

public class BatchDownloadRunner implements Callable<File>, Monitorable, UserTask {
public class BatchDownloadRunner implements Callable<FileResult>, Monitorable, UserTask {
private final static Logger logger = LoggerFactory.getLogger(BatchDownloadRunner.class);
static final int MAX_SCROLL_SIZE = 3500;
static final int MAX_BATCH_RESULT_SIZE = 10000;
Expand Down Expand Up @@ -83,7 +82,7 @@ public BatchDownloadRunner(Indexer indexer, PropertiesProvider propertiesProvide
}

@Override
public File call() throws Exception {
public FileResult call() throws Exception {
int throttleMs = parseInt(propertiesProvider.get(BATCH_THROTTLE).orElse("0"));
int maxResultSize = parseInt(propertiesProvider.get(BATCH_DOWNLOAD_MAX_NB_FILES).orElse(valueOf(MAX_BATCH_RESULT_SIZE)));
int scrollSize = min(parseInt(propertiesProvider.get(SCROLL_SIZE).orElse("1000")), MAX_SCROLL_SIZE);
Expand Down Expand Up @@ -120,16 +119,15 @@ public File call() throws Exception {
if (addedBytes > 0) {
zippedFilesSize += addedBytes;
numberOfResults.incrementAndGet();
batchDownload.setZipSize(zippedFilesSize);
progressCallback.apply(task.id, getProgressRate());
}
}
docsToProcess = searcher.scroll().collect(toList());
}
}
logger.info("created batch download file {} ({} bytes/{} entries) for user {}",
batchDownload.filename, Files.size(batchDownload.filename), numberOfResults, batchDownload.user.getId());
return batchDownload.filename.toFile();
FileResult result = new FileResult(batchDownload.filename.toFile(), Files.size(batchDownload.filename));
logger.info("created batch download file {} of {} entries for user {}", result, numberOfResults.get(), batchDownload.user.getId());
return result;
}

private Zipper createZipper(BatchDownload batchDownload, PropertiesProvider propertiesProvider, Function<URI, MailSender> mailSenderSupplier) throws URISyntaxException, IOException {
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
package org.icij.datashare.tasks;

import com.fasterxml.jackson.annotation.JsonCreator;
import com.fasterxml.jackson.annotation.JsonProperty;

import java.io.File;
import java.io.Serializable;

import static java.lang.String.format;

public class FileResult implements Serializable {
public final File file;
public final long size;

@JsonCreator
public FileResult(@JsonProperty("file") File file, @JsonProperty("size") long size) {
this.file = file;
this.size = size;
}

@Override
public String toString() {
return format("%s (%d bytes)", file, size);
}
}
Original file line number Diff line number Diff line change
@@ -1,20 +1,17 @@
package org.icij.datashare.tasks;

import org.icij.datashare.batch.BatchDownload;
import org.icij.datashare.batch.BatchSearch;
import org.icij.datashare.function.TerFunction;
import org.icij.datashare.nlp.NlpApp;
import org.icij.datashare.text.Document;
import org.icij.datashare.text.nlp.Pipeline;
import org.icij.datashare.user.User;

import java.io.File;
import java.nio.file.Path;
import java.util.List;
import java.util.Properties;
import java.util.Set;
import java.util.function.BiFunction;
import java.util.function.Function;

public interface TaskFactory {
ResumeNlpTask createResumeNlpTask(final User user, Set<Pipeline.Type> pipelines, Properties taskProperties);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -10,7 +10,6 @@
import io.swagger.v3.oas.annotations.media.Schema;
import io.swagger.v3.oas.annotations.parameters.RequestBody;
import io.swagger.v3.oas.annotations.responses.ApiResponse;
import io.swagger.v3.oas.models.security.SecurityScheme;
import net.codestory.http.Context;
import net.codestory.http.annotations.*;
import net.codestory.http.errors.ForbiddenException;
Expand Down Expand Up @@ -94,11 +93,11 @@ public TaskView<?> getTask(@Parameter(name = "id", description = "task id", in =
public Payload getTaskResult(@Parameter(name = "id", description = "task id", in = ParameterIn.PATH) String id, Context context) {
TaskView<?> task = forbiddenIfNotSameUser(context, notFoundIfNull(taskManager.getTask(id)));
Object result = task.getResult();
if (result instanceof File) {
final Path appPath = ((File) result).isAbsolute() ?
if (result instanceof FileResult) {
final Path appPath = ((FileResult) result).file.isAbsolute() ?
get(System.getProperty("user.dir")).resolve(context.env().appFolder()) :
get(context.env().appFolder());
Path resultPath = appPath.relativize(((File) result).toPath());
Path resultPath = appPath.relativize(((FileResult) result).file.toPath());
return new Payload(resultPath).withHeader("Content-Disposition", "attachment;filename=\"" + resultPath.getFileName() + "\"");
}
return result == null ? new Payload(204) : new Payload(result);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -90,10 +90,9 @@ public void test_update_batch_download_zip_size() throws Exception {
new IndexerHelper(es.client).indexFile("doc2.txt", "Portez ce vieux whisky au juge blond qui fume", fs);

BatchDownload bd = createBatchDownload("*");
long sizeAtCreation = bd.zipSize;
new BatchDownloadRunner(indexer, createProvider(), createTaskView(bd), taskModifier::progress).call();
FileResult result = new BatchDownloadRunner(indexer, createProvider(), createTaskView(bd), taskModifier::progress).call();

assertThat(bd.zipSize).isGreaterThan(sizeAtCreation);
assertThat(result.size).isGreaterThan(0);
}

@Test
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,7 +21,6 @@
import java.nio.file.Files;
import java.nio.file.Path;
import java.util.HashMap;
import java.util.function.Function;
import java.util.stream.IntStream;
import java.util.zip.ZipFile;

Expand Down Expand Up @@ -49,24 +48,24 @@ public void test_max_default_results() throws Exception {
Document[] documents = IntStream.range(0, 3).mapToObj(i -> createDoc("doc" + i).with(createFile(i)).build()).toArray(Document[]::new);
mockSearch.willReturn(2, documents);
BatchDownload batchDownload = new BatchDownload(singletonList(project("test-datashare")), User.local(), "query");
File zip = new BatchDownloadRunner(indexer, new PropertiesProvider(new HashMap<>() {{
FileResult result = new BatchDownloadRunner(indexer, new PropertiesProvider(new HashMap<>() {{
put(BATCH_DOWNLOAD_MAX_NB_FILES, "3");
put(SCROLL_SIZE, "3");
}}), getTaskView(batchDownload), updater::progress).call();

assertThat(new ZipFile(zip).size()).isEqualTo(3);
assertThat(new ZipFile(result.file).size()).isEqualTo(3);
}

@Test
public void test_max_zip_size() throws Exception {
Document[] documents = IntStream.range(0, 3).mapToObj(i -> createDoc("doc" + i).with(createFile(i)).with("hello world " + i).build()).toArray(Document[]::new);
mockSearch.willReturn(2, documents);
File zip = new BatchDownloadRunner(indexer, new PropertiesProvider(new HashMap<>() {{
FileResult result = new BatchDownloadRunner(indexer, new PropertiesProvider(new HashMap<>() {{
put(BATCH_DOWNLOAD_MAX_SIZE, valueOf("hello world 1".getBytes(StandardCharsets.UTF_8).length * 3));
put(SCROLL_SIZE, "3");
}}), getTaskView(new BatchDownload(singletonList(project("test-datashare")), User.local(), "query")), updater::progress).call();

assertThat(new ZipFile(zip).size()).isEqualTo(4);
assertThat(new ZipFile(result.file).size()).isEqualTo(4);
}

@Test(expected = ElasticsearchStatusException.class)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.junit.Test;

import java.io.File;
import java.nio.file.Files;
import java.nio.file.Paths;
import java.util.HashMap;
import java.util.concurrent.ArrayBlockingQueue;
Expand Down Expand Up @@ -82,9 +83,11 @@ public void test_get_task_result_with_int_result() {
}

@Test
public void test_get_task_result_with_file_result__should_relativize_result_with_app_folder() {
public void test_get_task_result_with_file_result__should_relativize_result_with_app_folder() throws Exception {
setupAppWith("foo");
TaskView<File> t = taskManager.startTask(new DummyUserTask<>("foo", () -> Paths.get("app", "index.html").toFile()));
File file = new File(ClassLoader.getSystemResource("app/index.html").toURI());
FileResult indexHtml = new FileResult(file, Files.size(file.toPath()));
TaskView<FileResult> t = taskManager.startTask(new DummyUserTask<>("foo", () -> indexHtml));
get("/api/task/" + t.id + "/result").withPreemptiveAuthentication("foo", "qux").
should().respond(200).
should().haveType("text/html;charset=UTF-8").
Expand All @@ -95,8 +98,9 @@ public void test_get_task_result_with_file_result__should_relativize_result_with
@Test
public void test_get_task_result_with_file_result_and_absolute_path__should_relativize_result_with_app_folder() throws Exception {
setupAppWith("foo");
File indexHtml = new File(ClassLoader.getSystemResource("app/index.html").toURI());
TaskView<File> t = taskManager.startTask(new DummyUserTask<>("foo", () -> indexHtml));
File file = new File(ClassLoader.getSystemResource("app/index.html").toURI());
FileResult indexHtml = new FileResult(file, Files.size(file.toPath()));
TaskView<FileResult> t = taskManager.startTask(new DummyUserTask<>("foo", () -> indexHtml));

get("/api/task/" + t.id + "/result").withPreemptiveAuthentication("foo", "qux").should().respond(200);
}
Expand Down
2 changes: 1 addition & 1 deletion pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -76,7 +76,7 @@
<maven.compiler.source>11</maven.compiler.source>
<maven.compiler.target>11</maven.compiler.target>

<datashare-api.version>12.4.2</datashare-api.version>
<datashare-api.version>12.4.3</datashare-api.version>
<datashare-cli.version>13.5.0</datashare-cli.version>

<datashare-mitie.version>${project.version}</datashare-mitie.version>
Expand Down

0 comments on commit 7829cf0

Please sign in to comment.