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

Provide a way to automatically delete multipart temporary files #5653

Merged
merged 10 commits into from
May 30, 2024
Original file line number Diff line number Diff line change
Expand Up @@ -21,6 +21,7 @@
import java.nio.file.Path;
import java.util.List;

import org.jetbrains.annotations.NotNull;
import org.openjdk.jmh.annotations.Benchmark;
import org.slf4j.helpers.NOPLogger;

Expand Down Expand Up @@ -55,42 +56,36 @@ public class RoutersBenchmark {
private static final RequestTarget METHOD1_REQ_TARGET = RequestTarget.forServer(METHOD1_HEADERS.path());

static {
final String defaultLogName = "log";
final String defaultServiceName = null;
final ServiceNaming defaultServiceNaming = ServiceNaming.of("Service");
final Route route1 = Route.builder().exact("/grpc.package.Service/Method1").build();
final Route route2 = Route.builder().exact("/grpc.package.Service/Method2").build();
final Path multipartUploadsLocation = Flags.defaultMultipartUploadsLocation();
final ServiceErrorHandler serviceErrorHandler = ServerErrorHandler.ofDefault().asServiceErrorHandler();
SERVICES = ImmutableList.of(
new ServiceConfig(route1, route1,
SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0,
false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(),
SuccessFunction.always(), 0, multipartUploadsLocation,
CommonPools.workerGroup(), ImmutableList.of(), HttpHeaders.of(),
ctx -> RequestId.random(), serviceErrorHandler, NOOP_CONTEXT_HOOK),
new ServiceConfig(route2, route2,
SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0,
false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(),
SuccessFunction.always(), 0, multipartUploadsLocation,
CommonPools.workerGroup(), ImmutableList.of(), HttpHeaders.of(),
ctx -> RequestId.random(), serviceErrorHandler, NOOP_CONTEXT_HOOK));
FALLBACK_SERVICE = new ServiceConfig(Route.ofCatchAll(), Route.ofCatchAll(), SERVICE,
defaultLogName, defaultServiceName,
defaultServiceNaming, 0, 0, false, AccessLogWriter.disabled(),
CommonPools.blockingTaskExecutor(), SuccessFunction.always(), 0,
multipartUploadsLocation, CommonPools.workerGroup(),
ImmutableList.of(), HttpHeaders.of(), ctx -> RequestId.random(),
serviceErrorHandler, NOOP_CONTEXT_HOOK);
SERVICES = ImmutableList.of(newServiceConfig(route1), newServiceConfig(route2));
FALLBACK_SERVICE = newServiceConfig(Route.ofCatchAll());
HOST = new VirtualHost(
"localhost", "localhost", 0, null, SERVICES, FALLBACK_SERVICE, RejectedRouteHandler.DISABLED,
unused -> NOPLogger.NOP_LOGGER, defaultServiceNaming, defaultLogName, 0, 0, false,
unused -> NOPLogger.NOP_LOGGER, FALLBACK_SERVICE.defaultServiceNaming(),
FALLBACK_SERVICE.defaultLogName(), 0, 0, false,
AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(), 0, SuccessFunction.ofDefault(),
multipartUploadsLocation, CommonPools.workerGroup(), ImmutableList.of(),
FALLBACK_SERVICE.multipartUploadsLocation(), MultipartRemovalStrategy.ON_RESPONSE_COMPLETION,
CommonPools.workerGroup(), ImmutableList.of(),
ctx -> RequestId.random());
ROUTER = Routers.ofVirtualHost(HOST, SERVICES, RejectedRouteHandler.DISABLED);
}

private static @NotNull ServiceConfig newServiceConfig(Route route) {
ikhoon marked this conversation as resolved.
Show resolved Hide resolved
final String defaultLogName = "log";
final String defaultServiceName = null;
final ServiceNaming defaultServiceNaming = ServiceNaming.of("Service");
final Path multipartUploadsLocation = Flags.defaultMultipartUploadsLocation();
final ServiceErrorHandler serviceErrorHandler = ServerErrorHandler.ofDefault().asServiceErrorHandler();
return new ServiceConfig(route, route,
SERVICE, defaultLogName, defaultServiceName, defaultServiceNaming, 0, 0,
false, AccessLogWriter.disabled(), CommonPools.blockingTaskExecutor(),
SuccessFunction.always(), 0, multipartUploadsLocation,
MultipartRemovalStrategy.ON_RESPONSE_COMPLETION,
CommonPools.workerGroup(), ImmutableList.of(), HttpHeaders.of(),
ctx -> RequestId.random(), serviceErrorHandler, NOOP_CONTEXT_HOOK);
}

@Benchmark
public Routed<ServiceConfig> exactMatch() {
final RoutingContext ctx = DefaultRoutingContext.of(HOST, "localhost", METHOD1_REQ_TARGET,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import com.linecorp.armeria.common.util.Sampler;
import com.linecorp.armeria.common.util.TlsEngineType;
import com.linecorp.armeria.common.util.TransportType;
import com.linecorp.armeria.server.MultipartRemovalStrategy;
import com.linecorp.armeria.server.TransientServiceOption;

import io.micrometer.core.instrument.MeterRegistry;
Expand Down Expand Up @@ -460,6 +461,11 @@ public Path defaultMultipartUploadsLocation() {
File.separatorChar + "multipart-uploads");
}

@Override
public MultipartRemovalStrategy defaultMultipartRemovalStrategy() {
return MultipartRemovalStrategy.ON_RESPONSE_COMPLETION;
}

@Override
public Sampler<? super RequestContext> requestContextLeakDetectionSampler() {
return Sampler.never();
Expand Down
13 changes: 13 additions & 0 deletions core/src/main/java/com/linecorp/armeria/common/Flags.java
Original file line number Diff line number Diff line change
Expand Up @@ -58,6 +58,7 @@
import com.linecorp.armeria.internal.common.FlagsLoaded;
import com.linecorp.armeria.internal.common.util.SslContextUtil;
import com.linecorp.armeria.server.HttpService;
import com.linecorp.armeria.server.MultipartRemovalStrategy;
import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.server.ServerErrorHandler;
import com.linecorp.armeria.server.Service;
Expand Down Expand Up @@ -399,6 +400,9 @@ private static boolean validateTransportType(TransportType transportType, String
private static final Path DEFAULT_MULTIPART_UPLOADS_LOCATION =
getValue(FlagsProvider::defaultMultipartUploadsLocation, "defaultMultipartUploadsLocation");

private static final MultipartRemovalStrategy DEFAULT_MULTIPART_REMOVAL_STRATEGY =
getValue(FlagsProvider::defaultMultipartRemovalStrategy, "defaultMultipartRemovalStrategy");

private static final Sampler<? super RequestContext> REQUEST_CONTEXT_LEAK_DETECTION_SAMPLER =
getValue(FlagsProvider::requestContextLeakDetectionSampler, "requestContextLeakDetectionSampler");

Expand Down Expand Up @@ -1442,6 +1446,15 @@ public static Path defaultMultipartUploadsLocation() {
return DEFAULT_MULTIPART_UPLOADS_LOCATION;
}

/**
* Returns the {@link MultipartRemovalStrategy} that is used to determine how to remove the uploaded files
* from {@code multipart/form-data}.
*/
@UnstableApi
public static MultipartRemovalStrategy defaultMultipartRemovalStrategy() {
return DEFAULT_MULTIPART_REMOVAL_STRATEGY;
}

/**
* Returns whether to allow double dots ({@code ..}) in a request path query string.
*
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,6 +45,7 @@
import com.linecorp.armeria.common.util.TlsEngineType;
import com.linecorp.armeria.common.util.TransportType;
import com.linecorp.armeria.server.HttpService;
import com.linecorp.armeria.server.MultipartRemovalStrategy;
import com.linecorp.armeria.server.ServerBuilder;
import com.linecorp.armeria.server.Service;
import com.linecorp.armeria.server.ServiceRequestContext;
Expand Down Expand Up @@ -1095,6 +1096,15 @@ default Path defaultMultipartUploadsLocation() {
return null;
}

/**
* Returns the {@link MultipartRemovalStrategy} that is used to determine how to remove the uploaded files
* from {@code multipart/form-data}.
*/
@Nullable
default MultipartRemovalStrategy defaultMultipartRemovalStrategy() {
return null;
}

/**
* Returns the {@link Sampler} that determines whether to trace the stack trace of request contexts leaks
* and how frequently to keeps stack trace. A sampled exception will have the stack trace while the others
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import com.linecorp.armeria.common.util.Sampler;
import com.linecorp.armeria.common.util.TlsEngineType;
import com.linecorp.armeria.common.util.TransportType;
import com.linecorp.armeria.server.MultipartRemovalStrategy;
import com.linecorp.armeria.server.TransientServiceOption;

/**
Expand Down Expand Up @@ -473,6 +474,24 @@ public Path defaultMultipartUploadsLocation() {
return getAndParse("defaultMultipartUploadsLocation", Paths::get);
}

@Nullable
@Override
public MultipartRemovalStrategy defaultMultipartRemovalStrategy() {
final String multipartRemovalStrategy = getNormalized("defaultMultipartRemovalStrategy");
if (multipartRemovalStrategy == null) {
return null;
}
switch (multipartRemovalStrategy) {
case "never":
return MultipartRemovalStrategy.NEVER;
case "on_response_completion":
return MultipartRemovalStrategy.ON_RESPONSE_COMPLETION;
default:
throw new IllegalArgumentException(
multipartRemovalStrategy + " isn't MultipartRemovalStrategy");
ikhoon marked this conversation as resolved.
Show resolved Hide resolved
}
}

@Override
public Sampler<? super RequestContext> requestContextLeakDetectionSampler() {
final String spec = getNormalized("requestContextLeakDetectionSampler");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -26,6 +26,9 @@
import java.util.concurrent.ExecutorService;
import java.util.concurrent.ScheduledExecutorService;

import org.slf4j.Logger;
import org.slf4j.LoggerFactory;

import com.google.common.collect.ImmutableListMultimap;
import com.google.common.collect.ListMultimap;
import com.google.common.collect.Maps;
Expand All @@ -39,6 +42,8 @@
import io.netty.channel.EventLoop;

public final class FileAggregatedMultipart {
private static final Logger logger = LoggerFactory.getLogger(FileAggregatedMultipart.class);

private final ListMultimap<String, String> params;
private final ListMultimap<String, MultipartFile> files;

Expand Down Expand Up @@ -72,7 +77,7 @@ public static CompletableFuture<FileAggregatedMultipart> aggregateMultipart(Serv
return resolveTmpFile(incompleteDir, filename, executor).thenCompose(path -> {
return bodyPart.writeTo(path, eventLoop, executor).thenCompose(ignore -> {
final Path completeDir = destination.resolve("complete");
return moveFile(path, completeDir, executor);
return moveFile(path, completeDir, executor, ctx);
}).thenApply(completePath -> MultipartFile.of(name, filename, completePath.toFile(),
bodyPart.headers()));
});
Expand Down Expand Up @@ -102,19 +107,40 @@ public static CompletableFuture<FileAggregatedMultipart> aggregateMultipart(Serv
}

private static CompletableFuture<Path> moveFile(Path file, Path targetDirectory,
ExecutorService blockingExecutorService) {
ExecutorService blockingExecutorService,
ServiceRequestContext ctx) {
return CompletableFuture.supplyAsync(() -> {
try {
Files.createDirectories(targetDirectory);
// Avoid name duplication, create new file at target place and replace it.
return Files.move(file, Files.createTempFile(targetDirectory, null, ".multipart"),
StandardCopyOption.REPLACE_EXISTING);
final Path tempFile = createRemovableTempFile(targetDirectory, blockingExecutorService, ctx);
return Files.move(file, tempFile, StandardCopyOption.REPLACE_EXISTING);
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}, blockingExecutorService);
}

private static Path createRemovableTempFile(Path targetDirectory,
ExecutorService blockingExecutorService,
ServiceRequestContext ctx) throws IOException {
final Path tempFile = Files.createTempFile(targetDirectory, null, ".multipart");
switch (ctx.config().multipartRemovalStrategy()) {
case NEVER:
break;
case ON_RESPONSE_COMPLETION:
ctx.log().whenComplete().thenAcceptAsync(unused -> {
try {
Files.deleteIfExists(tempFile);
} catch (IOException e) {
logger.warn("Failed to delete a temporary file: {}", tempFile, e);
}
}, blockingExecutorService);
break;
}
return tempFile;
}

private static CompletableFuture<Path> resolveTmpFile(Path directory,
String filename,
ExecutorService blockingExecutorService) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -282,6 +282,14 @@ public AbstractAnnotatedServiceConfigSetters multipartUploadsLocation(Path multi
return this;
}

@UnstableApi
@Override
public AbstractAnnotatedServiceConfigSetters multipartRemovalStrategy(
MultipartRemovalStrategy removalStrategy) {
defaultServiceConfigSetters.multipartRemovalStrategy(removalStrategy);
return this;
}

@Override
public ServiceConfigSetters serviceWorkerGroup(EventLoopGroup serviceWorkerGroup, boolean shutdownOnStop) {
defaultServiceConfigSetters.serviceWorkerGroup(serviceWorkerGroup, shutdownOnStop);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -177,6 +177,12 @@ public AbstractServiceBindingBuilder multipartUploadsLocation(Path multipartUplo
return this;
}

@Override
public AbstractServiceBindingBuilder multipartRemovalStrategy(MultipartRemovalStrategy removalStrategy) {
defaultServiceConfigSetters.multipartRemovalStrategy(removalStrategy);
return this;
}

@Override
public AbstractServiceBindingBuilder serviceWorkerGroup(EventLoopGroup serviceWorkerGroup,
boolean shutdownOnStop) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -224,6 +224,11 @@ public AnnotatedServiceBindingBuilder multipartUploadsLocation(Path multipartUpl
return (AnnotatedServiceBindingBuilder) super.multipartUploadsLocation(multipartUploadsLocation);
}

@Override
public AnnotatedServiceBindingBuilder multipartRemovalStrategy(MultipartRemovalStrategy removalStrategy) {
return (AnnotatedServiceBindingBuilder) super.multipartRemovalStrategy(removalStrategy);
}

@Override
public AnnotatedServiceBindingBuilder requestIdGenerator(
Function<? super RoutingContext, ? extends RequestId> requestIdGenerator) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -250,6 +250,12 @@ public ContextPathAnnotatedServiceConfigSetters multipartUploadsLocation(
super.multipartUploadsLocation(multipartUploadsLocation);
}

@Override
public ContextPathAnnotatedServiceConfigSetters multipartRemovalStrategy(
MultipartRemovalStrategy removalStrategy) {
return (ContextPathAnnotatedServiceConfigSetters) super.multipartRemovalStrategy(removalStrategy);
}

@Override
public ContextPathAnnotatedServiceConfigSetters requestIdGenerator(
Function<? super RoutingContext, ? extends RequestId> requestIdGenerator) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -161,17 +161,20 @@ public ContextPathServiceBindingBuilder requestAutoAbortDelay(Duration delay) {
}

@Override
public ContextPathServiceBindingBuilder requestAutoAbortDelayMillis(
long delayMillis) {
public ContextPathServiceBindingBuilder requestAutoAbortDelayMillis(long delayMillis) {
return (ContextPathServiceBindingBuilder) super.requestAutoAbortDelayMillis(delayMillis);
}

@Override
public ContextPathServiceBindingBuilder multipartUploadsLocation(
Path multipartUploadsLocation) {
public ContextPathServiceBindingBuilder multipartUploadsLocation(Path multipartUploadsLocation) {
return (ContextPathServiceBindingBuilder) super.multipartUploadsLocation(multipartUploadsLocation);
}

@Override
public ContextPathServiceBindingBuilder multipartRemovalStrategy(MultipartRemovalStrategy removalStrategy) {
return (ContextPathServiceBindingBuilder) super.multipartRemovalStrategy(removalStrategy);
}

@Override
public ContextPathServiceBindingBuilder serviceWorkerGroup(EventLoopGroup serviceWorkerGroup,
boolean shutdownOnStop) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -79,6 +79,8 @@ final class DefaultServiceConfigSetters implements ServiceConfigSetters {
@Nullable
private Path multipartUploadsLocation;
@Nullable
private MultipartRemovalStrategy multipartRemovalStrategy;
@Nullable
private EventLoopGroup serviceWorkerGroup;
@Nullable
private ServiceErrorHandler serviceErrorHandler;
Expand Down Expand Up @@ -237,6 +239,12 @@ public ServiceConfigSetters multipartUploadsLocation(Path multipartUploadsLocati
return this;
}

@Override
public ServiceConfigSetters multipartRemovalStrategy(MultipartRemovalStrategy removalStrategy) {
this.multipartRemovalStrategy = requireNonNull(removalStrategy, "removalStrategy");
return this;
}

@Override
public ServiceConfigSetters serviceWorkerGroup(EventLoopGroup serviceWorkerGroup,
boolean shutdownOnStop) {
Expand Down Expand Up @@ -377,6 +385,9 @@ ServiceConfigBuilder toServiceConfigBuilder(Route route, String contextPath, Htt
if (multipartUploadsLocation != null) {
serviceConfigBuilder.multipartUploadsLocation(multipartUploadsLocation);
}
if (multipartRemovalStrategy != null) {
serviceConfigBuilder.multipartRemovalStrategy(multipartRemovalStrategy);
}
if (serviceWorkerGroup != null) {
serviceConfigBuilder.serviceWorkerGroup(serviceWorkerGroup, false);
// Set the serviceWorkerGroup as false because it's shut down in ShutdownSupport.
Expand Down