Skip to content

Commit b02fb56

Browse files
Ivan-ShamlIvan-Shaml
authored
Change to S3 based mod upload (#3382)
* feat(mod-upload): switch to upload via S3 * Fix comments
1 parent d71fa01 commit b02fb56

File tree

7 files changed

+182
-29
lines changed

7 files changed

+182
-29
lines changed

‎src/main/java/com/faforever/client/api/FafApiAccessor.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -192,6 +192,14 @@ public Mono<Void> postMultipartForm(String endpointPath, MultiValueMap<String, O
192192
.bodyValue(request)).doOnSuccess(aVoid -> log.trace("Posted {} to {}", request, endpointPath));
193193
}
194194

195+
public <T> Mono<Void> postJson(String endpointPath, T request) {
196+
return retrieveMonoWithErrorHandling(Void.class, apiWebClient.post()
197+
.uri(endpointPath)
198+
.contentType(MediaType.APPLICATION_JSON)
199+
.bodyValue(request)).doOnSuccess(
200+
aVoid -> log.trace("Posted json {} to {}", request, endpointPath));
201+
}
202+
195203
public <T extends ElideEntity> Mono<T> post(ElideNavigatorOnCollection<T> navigator, T request) {
196204
Class<T> type = navigator.getDtoClass();
197205
String endpointPath = navigator.build();

‎src/main/java/com/faforever/client/mod/ModUploadMetadata.java‎

Lines changed: 10 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,10 @@
1+
package com.faforever.client.mod;
2+
3+
import java.util.UUID;
4+
5+
public record ModUploadMetadata(
6+
UUID requestId,
7+
Integer licenseId,
8+
String repositoryUrl
9+
) {
10+
}

‎src/main/java/com/faforever/client/mod/ModUploadTask.java‎

Lines changed: 56 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -8,17 +8,25 @@
88
import com.faforever.client.util.Validator;
99
import com.faforever.commons.io.ByteCountListener;
1010
import com.faforever.commons.io.Zipper;
11+
import lombok.Setter;
1112
import lombok.extern.slf4j.Slf4j;
1213
import org.springframework.beans.factory.annotation.Autowired;
1314
import org.springframework.beans.factory.config.ConfigurableBeanFactory;
1415
import org.springframework.context.annotation.Scope;
16+
import org.springframework.core.io.FileSystemResource;
17+
import org.springframework.http.HttpStatusCode;
18+
import org.springframework.http.MediaType;
1519
import org.springframework.stereotype.Component;
20+
import org.springframework.web.reactive.function.BodyInserters;
21+
import org.springframework.web.reactive.function.client.WebClient;
22+
import reactor.core.publisher.Mono;
1623

1724
import java.io.OutputStream;
25+
import java.net.URI;
1826
import java.nio.file.Files;
1927
import java.nio.file.Path;
2028
import java.util.Locale;
21-
import java.util.Map;
29+
import java.util.UUID;
2230

2331
import static com.faforever.commons.io.Bytes.formatSize;
2432
import static java.nio.file.Files.createTempFile;
@@ -29,18 +37,24 @@
2937
@Slf4j
3038
public class ModUploadTask extends CompletableTask<Void> {
3139

40+
static final String MOD_UPLOAD_START_API_GET = "/mods/upload/start";
41+
static final String MOD_UPLOAD_COMPLETE_API_POST = "/mods/upload/complete";
42+
3243
private final FafApiAccessor fafApiAccessor;
3344
private final I18n i18n;
3445
private final DataPrefs dataPrefs;
46+
private final WebClient defaultWebClient;
3547

48+
@Setter
3649
private Path modPath;
3750

3851
@Autowired
39-
public ModUploadTask(FafApiAccessor fafApiAccessor, I18n i18n, DataPrefs dataPrefs) {
52+
public ModUploadTask(FafApiAccessor fafApiAccessor, I18n i18n, DataPrefs dataPrefs, WebClient defaultWebClient) {
4053
super(Priority.HIGH);
4154
this.dataPrefs = dataPrefs;
4255
this.fafApiAccessor = fafApiAccessor;
4356
this.i18n = i18n;
57+
this.defaultWebClient = defaultWebClient;
4458
}
4559

4660
@Override
@@ -69,17 +83,53 @@ protected Void call() throws Exception {
6983
.zip();
7084
}
7185

72-
log.debug("Uploading mod `{}` as `{}`", modPath, tmpFile);
86+
log.debug("Starting upload sequence. Uploading mod `{}` as `{}`", modPath, tmpFile);
7387
updateTitle(i18n.get("modVault.upload.uploading"));
7488

75-
return fafApiAccessor.uploadFile("/mods/upload", tmpFile, byteListener, Map.of()).block();
89+
return fafApiAccessor.getApiObject(MOD_UPLOAD_START_API_GET, UploadUrlResponse.class)
90+
.flatMap(response -> uploadModToS3(response, tmpFile))
91+
.flatMap(this::completeUpload)
92+
.block();
93+
7694
} finally {
7795
Files.delete(tmpFile);
7896
ResourceLocks.freeUploadLock();
7997
}
8098
}
8199

82-
public void setModPath(Path modPath) {
83-
this.modPath = modPath;
100+
private Mono<Void> completeUpload(UUID requestId) {
101+
ModUploadMetadata metadata = new ModUploadMetadata(requestId, null, null);
102+
return fafApiAccessor.postJson(MOD_UPLOAD_COMPLETE_API_POST, metadata)
103+
.doOnSuccess(response -> log.debug("Mod upload complete for requestId=[{}]", requestId));
104+
}
105+
106+
private Mono<UUID> uploadModToS3(UploadUrlResponse response, Path filePath) {
107+
final URI signedUrl = response.uploadUrl();
108+
final UUID requestId = response.requestId();
109+
final FileSystemResource resource = new FileSystemResource(filePath);
110+
111+
log.debug("Uploading mod to S3: requestId=[{}], zip filePath=[{}]", filePath, requestId);
112+
113+
return defaultWebClient.put()
114+
.uri(signedUrl)
115+
.accept(MediaType.APPLICATION_JSON)
116+
.contentType(MediaType.valueOf("application/zip"))
117+
.body(BodyInserters.fromResource(resource))
118+
.retrieve()
119+
.onStatus(HttpStatusCode::isError, errResponse -> errResponse.bodyToMono(String.class)
120+
.doOnNext(json -> log.warn(
121+
"S3 Mod Upload failed. requestId=[{}], statusCode=[{}], \n response=[{}]",
122+
requestId,
123+
errResponse.statusCode()
124+
.value(), json))
125+
.then(Mono.error(
126+
new IllegalStateException(
127+
"S3 Mod Upload failed. Request Id=[%s], Status Code=[%d]".formatted(
128+
requestId,
129+
errResponse.statusCode()
130+
.value())))))
131+
.bodyToMono(Void.class)
132+
.doOnSuccess(r -> log.debug("Successfully uploaded mod to S3: requestId=[{}]", requestId))
133+
.thenReturn(requestId);
84134
}
85135
}

‎src/main/java/com/faforever/client/mod/UploadUrlResponse.java‎

Lines changed: 8 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,8 @@
1+
package com.faforever.client.mod;
2+
3+
import java.net.URI;
4+
import java.util.UUID;
5+
6+
public record UploadUrlResponse(URI uploadUrl, UUID requestId) {
7+
8+
}

‎src/main/resources/application-test.yml‎

Lines changed: 3 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -6,6 +6,9 @@ faf-client:
66
host: irc.faforever.xyz
77
port: 6697
88

9+
user:
10+
base-url: https://user.faforever.xyz
11+
912
vault:
1013
base-url: https://content.faforever.xyz
1114
replay-download-url-format: https://replay.faforever.xyz/%s

‎src/test/java/com/faforever/client/mod/ModUploadControllerTest.java‎

Lines changed: 5 additions & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -17,6 +17,7 @@
1717
import org.mockito.InjectMocks;
1818
import org.mockito.Mock;
1919
import org.mockito.Spy;
20+
import org.springframework.web.reactive.function.client.WebClient;
2021

2122
import java.nio.file.Path;
2223
import java.util.concurrent.ExecutorService;
@@ -62,14 +63,17 @@ public class ModUploadControllerTest extends PlatformTest {
6263
@Mock
6364
private FafApiAccessor fafApiAccessor;
6465

66+
@Mock
67+
private WebClient defaultWebClient;
68+
6569
@BeforeEach
6670
public void setUp() throws Exception {
6771
lenient().doAnswer(invocation -> {
6872
((Runnable) invocation.getArgument(0)).run();
6973
return null;
7074
}).when(executorService).execute(any());
7175

72-
modUploadTask = new ModUploadTask(fafApiAccessor, i18n, dataPrefs) {
76+
modUploadTask = new ModUploadTask(fafApiAccessor, i18n, dataPrefs, defaultWebClient) {
7377
@Override
7478
protected Void call() {
7579
return null;

‎src/test/java/com/faforever/client/mod/ModUploadTaskTest.java‎

Lines changed: 92 additions & 22 deletions
Original file line numberDiff line numberDiff line change
@@ -6,64 +6,134 @@
66
import com.faforever.client.test.PlatformTest;
77
import org.junit.jupiter.api.BeforeEach;
88
import org.junit.jupiter.api.Test;
9+
import org.junit.jupiter.api.extension.ExtendWith;
910
import org.junit.jupiter.api.io.TempDir;
11+
import org.mockito.ArgumentCaptor;
12+
import org.mockito.Captor;
1013
import org.mockito.Mock;
1114
import org.mockito.Spy;
15+
import org.mockito.junit.jupiter.MockitoExtension;
16+
import org.springframework.http.MediaType;
17+
import org.springframework.web.reactive.function.BodyInserter;
18+
import org.springframework.web.reactive.function.client.WebClient;
1219
import reactor.core.publisher.Mono;
1320

21+
import java.net.URI;
1422
import java.nio.file.Files;
1523
import java.nio.file.Path;
24+
import java.util.UUID;
1625

17-
import static org.hamcrest.MatcherAssert.assertThat;
18-
import static org.hamcrest.collection.IsArrayWithSize.emptyArray;
26+
import static com.faforever.client.mod.ModUploadTask.MOD_UPLOAD_COMPLETE_API_POST;
27+
import static com.faforever.client.mod.ModUploadTask.MOD_UPLOAD_START_API_GET;
1928
import static org.junit.jupiter.api.Assertions.assertThrows;
2029
import static org.mockito.ArgumentMatchers.any;
30+
import static org.mockito.ArgumentMatchers.eq;
2131
import static org.mockito.Mockito.lenient;
2232
import static org.mockito.Mockito.verify;
33+
import static org.mockito.Mockito.when;
2334

24-
public class ModUploadTaskTest extends PlatformTest {
35+
@ExtendWith(MockitoExtension.class)
36+
class ModUploadTaskTest extends PlatformTest {
2537

2638
@TempDir
27-
public Path tempDirectory;
39+
Path tempDirectory;
2840

29-
private ModUploadTask instance;
41+
@Mock
42+
FafApiAccessor fafApiAccessor;
3043

3144
@Mock
32-
private FafApiAccessor fafApiAccessor;
45+
WebClient defaultWebClient;
46+
3347
@Mock
34-
private I18n i18n;
48+
I18n i18n;
49+
3550
@Spy
36-
private DataPrefs dataPrefs;
51+
DataPrefs dataPrefs;
52+
53+
@Captor
54+
ArgumentCaptor<ModUploadMetadata> metadataCaptor;
55+
56+
ModUploadTask underTest;
57+
58+
UUID requestId;
59+
URI signedUri;
60+
61+
@Mock
62+
WebClient.RequestBodyUriSpec requestBodySpec;
63+
@Mock
64+
WebClient.RequestHeadersSpec<?> headersSpec;
65+
@Mock
66+
WebClient.ResponseSpec responseSpec;
3767

3868
@BeforeEach
39-
public void setUp() throws Exception {
40-
instance = new ModUploadTask(fafApiAccessor, i18n, dataPrefs);
41-
dataPrefs.setBaseDataDirectory(tempDirectory);
69+
void setUp() throws Exception {
70+
underTest = new ModUploadTask(fafApiAccessor, i18n, dataPrefs, defaultWebClient);
4271

72+
dataPrefs.setBaseDataDirectory(tempDirectory);
4373
Files.createDirectories(dataPrefs.getCacheDirectory());
74+
4475
lenient().when(i18n.get(any())).thenReturn("");
45-
lenient().when(fafApiAccessor.uploadFile(any(), any(), any(), any())).thenReturn(Mono.empty());
76+
77+
requestId = UUID.randomUUID();
78+
signedUri = new URI("https://example.com/upload");
4679
}
4780

4881
@Test
49-
public void testModPathNull() throws Exception {
50-
assertThrows(NullPointerException.class, () -> instance.call());
82+
void testModPathNull() {
83+
assertThrows(NullPointerException.class, () -> underTest.call());
5184
}
5285

5386
@Test
54-
public void testProgressListenerNull() throws Exception {
55-
instance.setModPath(Path.of("."));
56-
assertThrows(NullPointerException.class, () -> instance.call());
87+
void testProgressListenerNull() {
88+
underTest.setModPath(Path.of("."));
89+
assertThrows(NullPointerException.class, () -> underTest.call());
5790
}
5891

5992
@Test
60-
public void testCall() throws Exception {
61-
instance.setModPath(Files.createDirectories(tempDirectory.resolve("test-mod")));
93+
void testCall() throws Exception {
94+
stubWebClient();
95+
96+
when(responseSpec.bodyToMono(Void.class)).thenReturn(Mono.empty());
97+
when(fafApiAccessor.postJson(eq(MOD_UPLOAD_COMPLETE_API_POST), metadataCaptor.capture())).thenReturn(Mono.empty());
98+
99+
Path modFolder = Files.createDirectory(tempDirectory.resolve("my-mod"));
100+
Files.writeString(modFolder.resolve("hello.txt"), "world");
101+
102+
underTest.setModPath(modFolder);
103+
underTest.call();
104+
105+
verify(fafApiAccessor).getApiObject(MOD_UPLOAD_START_API_GET, UploadUrlResponse.class);
106+
verify(defaultWebClient).put();
107+
verify(fafApiAccessor).postJson(eq(MOD_UPLOAD_COMPLETE_API_POST), metadataCaptor.capture());
108+
109+
assert metadataCaptor.getValue().requestId().equals(requestId);
110+
assert Files.list(dataPrefs.getCacheDirectory()).toList().isEmpty();
111+
}
62112

63-
instance.call();
113+
@Test
114+
void testCallUploadFails() throws Exception {
115+
stubWebClient();
116+
117+
when(responseSpec.bodyToMono(Void.class)).thenReturn(Mono.error(new IllegalStateException("Simulated S3 failure")));
118+
119+
Path modFolder = Files.createDirectory(tempDirectory.resolve("my-mod"));
120+
Files.writeString(modFolder.resolve("hello.txt"), "world");
121+
122+
underTest.setModPath(modFolder);
123+
124+
assertThrows(IllegalStateException.class, () -> underTest.call());
125+
}
64126

65-
verify(fafApiAccessor).uploadFile(any(), any(), any(), any());
127+
void stubWebClient() {
128+
when(fafApiAccessor.getApiObject(MOD_UPLOAD_START_API_GET, UploadUrlResponse.class)).thenReturn(
129+
Mono.just(new UploadUrlResponse(signedUri, requestId)));
66130

67-
assertThat(Files.list(dataPrefs.getCacheDirectory()).toArray(), emptyArray());
131+
when(defaultWebClient.put()).thenReturn(requestBodySpec);
132+
when(requestBodySpec.uri(signedUri)).thenReturn(requestBodySpec);
133+
when(requestBodySpec.accept(MediaType.APPLICATION_JSON)).thenReturn(requestBodySpec);
134+
when(requestBodySpec.contentType(MediaType.valueOf("application/zip"))).thenReturn(requestBodySpec);
135+
when(requestBodySpec.body(any(BodyInserter.class))).thenReturn(headersSpec);
136+
when(headersSpec.retrieve()).thenReturn(responseSpec);
137+
when(responseSpec.onStatus(any(), any())).thenReturn(responseSpec);
68138
}
69139
}

0 commit comments

Comments
 (0)