Skip to content

Commit b8f25b0

Browse files
amvanbarenamvanbaren
authored
Merge pull request #1097 from amvanbaren/cached-file-already-exists-exception
Synchronize resource file writing
2 parents 8de8b47 + 4122e1f commit b8f25b0

File tree

7 files changed

+86
-35
lines changed

7 files changed

+86
-35
lines changed

‎server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java‎

Lines changed: 17 additions & 14 deletions
Original file line numberDiff line numberDiff line change
@@ -16,6 +16,7 @@
1616
import org.eclipse.openvsx.repositories.RepositoryService;
1717
import org.eclipse.openvsx.storage.StorageUtilService;
1818
import org.eclipse.openvsx.util.ErrorResultException;
19+
import org.eclipse.openvsx.util.FileUtil;
1920
import org.eclipse.openvsx.util.NamingUtil;
2021
import org.eclipse.openvsx.util.UrlUtil;
2122
import org.slf4j.Logger;
@@ -24,9 +25,9 @@
2425
import org.springframework.stereotype.Component;
2526

2627
import java.io.IOException;
28+
import java.io.UncheckedIOException;
2729
import java.nio.file.Files;
2830
import java.nio.file.Path;
29-
import java.nio.file.StandardCopyOption;
3031
import java.util.stream.Collectors;
3132
import java.util.zip.ZipFile;
3233

@@ -62,12 +63,7 @@ public Path getWebResource(String namespace, String extension, String targetPlat
6263
return null;
6364
}
6465

65-
Path path;
66-
try {
67-
path = storageUtil.getCachedFile(download);
68-
} catch(IOException e) {
69-
throw new ErrorResultException("Failed to get file for download " + NamingUtil.toLogFormat(download.getExtension()));
70-
}
66+
var path = storageUtil.getCachedFile(download);
7167
if(path == null) {
7268
return null;
7369
}
@@ -83,11 +79,13 @@ public Path getWebResource(String namespace, String extension, String targetPlat
8379
var fileExtIndex = fileEntry.getName().lastIndexOf('.');
8480
var fileExt = fileExtIndex != -1 ? fileEntry.getName().substring(fileExtIndex) : "";
8581
var file = filesCacheKeyGenerator.generateCachedWebResourcePath(namespace, extension, targetPlatform, version, name, fileExt);
86-
if(!Files.exists(file)) {
82+
FileUtil.writeSync(file, (p) -> {
8783
try (var in = zip.getInputStream(fileEntry)) {
88-
Files.copy(in, file);
84+
Files.copy(in, p);
85+
} catch(IOException e) {
86+
throw new UncheckedIOException(e);
8987
}
90-
}
88+
});
9189

9290
return file;
9391
} else if (browse) {
@@ -103,22 +101,27 @@ public Path getWebResource(String namespace, String extension, String targetPlat
103101
return null;
104102
}
105103

104+
106105
var file = filesCacheKeyGenerator.generateCachedWebResourcePath(namespace, extension, targetPlatform, version, name, ".unpkg.json");
107-
if(!Files.exists(file)) {
106+
FileUtil.writeSync(file, (p) -> {
108107
var baseUrl = UrlUtil.createApiUrl(UrlUtil.getBaseUrl(), "vscode", "unpkg", namespace, extension, version);
109108
var mapper = new ObjectMapper();
110109
var node = mapper.createArrayNode();
111110
for (var entry : dirEntries) {
112111
node.add(baseUrl + "/" + entry);
113112
}
114-
mapper.writeValue(file.toFile(), node);
115-
}
113+
try {
114+
mapper.writeValue(p.toFile(), node);
115+
} catch(IOException e) {
116+
throw new UncheckedIOException(e);
117+
}
118+
});
116119

117120
return file;
118121
} else {
119122
return null;
120123
}
121-
} catch (IOException e) {
124+
} catch (IOException | UncheckedIOException e) {
122125
throw new ErrorResultException("Failed to read extension files for " + NamingUtil.toLogFormat(download.getExtension()));
123126
}
124127
}

‎server/src/main/java/org/eclipse/openvsx/storage/AwsStorageService.java‎

Lines changed: 8 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111
package org.eclipse.openvsx.storage;
1212

1313
import java.io.IOException;
14+
import java.io.UncheckedIOException;
1415
import java.net.URI;
1516
import java.net.URISyntaxException;
1617
import java.nio.file.Files;
@@ -24,6 +25,7 @@
2425
import org.eclipse.openvsx.cache.FilesCacheKeyGenerator;
2526
import org.eclipse.openvsx.entities.FileResource;
2627
import org.eclipse.openvsx.entities.Namespace;
28+
import org.eclipse.openvsx.util.FileUtil;
2729
import org.eclipse.openvsx.util.TempFile;
2830
import org.eclipse.openvsx.util.UrlUtil;
2931
import org.springframework.beans.factory.annotation.Value;
@@ -246,19 +248,21 @@ private void copy(String oldObjectKey, String newObjectKey) {
246248

247249
@Override
248250
@Cacheable(value = CACHE_EXTENSION_FILES, keyGenerator = GENERATOR_FILES)
249-
public Path getCachedFile(FileResource resource) throws IOException {
251+
public Path getCachedFile(FileResource resource) {
250252
var objectKey = getObjectKey(resource);
251253
var request = GetObjectRequest.builder()
252254
.bucket(bucket)
253255
.key(objectKey)
254256
.build();
255257

256258
var path = filesCacheKeyGenerator.generateCachedExtensionPath(resource);
257-
if(!Files.exists(path)) {
259+
FileUtil.writeSync(path, (p) -> {
258260
try (var stream = getS3Client().getObject(request)) {
259-
Files.copy(stream, path);
261+
Files.copy(stream, p);
262+
} catch(IOException e) {
263+
throw new UncheckedIOException(e);
260264
}
261-
}
265+
});
262266

263267
return path;
264268
}

‎server/src/main/java/org/eclipse/openvsx/storage/AzureBlobStorageService.java‎

Lines changed: 5 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -22,6 +22,7 @@
2222
import org.eclipse.openvsx.cache.FilesCacheKeyGenerator;
2323
import org.eclipse.openvsx.entities.FileResource;
2424
import org.eclipse.openvsx.entities.Namespace;
25+
import org.eclipse.openvsx.util.FileUtil;
2526
import org.eclipse.openvsx.util.TempFile;
2627
import org.eclipse.openvsx.util.UrlUtil;
2728
import org.springframework.beans.factory.annotation.Value;
@@ -32,7 +33,6 @@
3233

3334
import java.io.IOException;
3435
import java.net.URI;
35-
import java.nio.file.Files;
3636
import java.nio.file.Path;
3737
import java.time.Duration;
3838
import java.time.temporal.ChronoUnit;
@@ -237,17 +237,16 @@ public void copyNamespaceLogo(Namespace oldNamespace, Namespace newNamespace) {
237237

238238
@Override
239239
@Cacheable(value = CACHE_EXTENSION_FILES, keyGenerator = GENERATOR_FILES)
240-
public Path getCachedFile(FileResource resource) throws IOException {
240+
public Path getCachedFile(FileResource resource) {
241241
var blobName = getBlobName(resource);
242242
if (StringUtils.isEmpty(serviceEndpoint)) {
243243
throw new IllegalStateException(missingEndpointMessage(blobName));
244244
}
245245

246246
var path = filesCacheKeyGenerator.generateCachedExtensionPath(resource);
247-
if(!Files.exists(path)) {
248-
getContainerClient().getBlobClient(blobName).downloadToFile(path.toAbsolutePath().toString());
249-
}
250-
247+
FileUtil.writeSync(path, (p) -> {
248+
getContainerClient().getBlobClient(blobName).downloadToFile(p.toAbsolutePath().toString());
249+
});
251250
return path;
252251
}
253252
}

‎server/src/main/java/org/eclipse/openvsx/storage/GoogleCloudStorageService.java‎

Lines changed: 7 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -9,15 +9,13 @@
99
********************************************************************************/
1010
package org.eclipse.openvsx.storage;
1111

12-
import com.google.cloud.storage.BlobId;
13-
import com.google.cloud.storage.BlobInfo;
14-
import com.google.cloud.storage.Storage;
15-
import com.google.cloud.storage.StorageOptions;
12+
import com.google.cloud.storage.*;
1613
import org.apache.commons.lang3.ArrayUtils;
1714
import org.apache.commons.lang3.StringUtils;
1815
import org.eclipse.openvsx.cache.FilesCacheKeyGenerator;
1916
import org.eclipse.openvsx.entities.FileResource;
2017
import org.eclipse.openvsx.entities.Namespace;
18+
import org.eclipse.openvsx.util.FileUtil;
2119
import org.eclipse.openvsx.util.TempFile;
2220
import org.eclipse.openvsx.util.UrlUtil;
2321
import org.springframework.beans.factory.annotation.Value;
@@ -214,16 +212,16 @@ private void copy(String source, String target) {
214212

215213
@Override
216214
@Cacheable(value = CACHE_EXTENSION_FILES, keyGenerator = GENERATOR_FILES)
217-
public Path getCachedFile(FileResource resource) throws IOException {
215+
public Path getCachedFile(FileResource resource) {
218216
if (StringUtils.isEmpty(bucketId)) {
219217
throw new IllegalStateException(missingBucketIdMessage(resource.getName()));
220218
}
221219

220+
var objectId = getObjectId(resource);
222221
var path = filesCacheKeyGenerator.generateCachedExtensionPath(resource);
223-
if(!Files.exists(path)) {
224-
var objectId = getObjectId(resource);
225-
getStorage().downloadTo(BlobId.of(bucketId, objectId), path);
226-
}
222+
FileUtil.writeSync(path, (p) -> {
223+
getStorage().downloadTo(BlobId.of(bucketId, objectId), p);
224+
});
227225

228226
return path;
229227
}

‎server/src/main/java/org/eclipse/openvsx/storage/IStorageService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -62,5 +62,5 @@ public interface IStorageService {
6262

6363
void copyNamespaceLogo(Namespace oldNamespace, Namespace newNamespace);
6464

65-
Path getCachedFile(FileResource resource) throws IOException;
65+
Path getCachedFile(FileResource resource);
6666
}

‎server/src/main/java/org/eclipse/openvsx/storage/StorageUtilService.java‎

Lines changed: 1 addition & 1 deletion
Original file line numberDiff line numberDiff line change
@@ -363,7 +363,7 @@ public void copyNamespaceLogo(Namespace oldNamespace, Namespace newNamespace) {
363363
}
364364

365365
@Override
366-
public Path getCachedFile(FileResource resource) throws IOException {
366+
public Path getCachedFile(FileResource resource) {
367367
return switch (resource.getStorageType()) {
368368
case STORAGE_GOOGLE -> googleStorage.getCachedFile(resource);
369369
case STORAGE_AZURE -> azureStorage.getCachedFile(resource);

‎server/src/main/java/org/eclipse/openvsx/util/FileUtil.java‎

Lines changed: 47 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,47 @@
1+
/** ******************************************************************************
2+
* Copyright (c) 2025 Precies. Software OU and others
3+
*
4+
* This program and the accompanying materials are made available under the
5+
* terms of the Eclipse Public License v. 2.0 which is available at
6+
* http://www.eclipse.org/legal/epl-2.0.
7+
*
8+
* SPDX-License-Identifier: EPL-2.0
9+
* ****************************************************************************** */
10+
11+
package org.eclipse.openvsx.util;
12+
13+
import java.nio.file.Files;
14+
import java.nio.file.Path;
15+
import java.util.Collections;
16+
import java.util.LinkedHashMap;
17+
import java.util.Map;
18+
import java.util.function.Consumer;
19+
20+
public class FileUtil {
21+
22+
private static final Map<Path, Object> LOCKS;
23+
24+
static {
25+
var MAX_SIZE = 100;
26+
LOCKS = Collections.synchronizedMap(new LinkedHashMap<>(MAX_SIZE) {
27+
protected boolean removeEldestEntry(Map.Entry eldest){
28+
return size() > MAX_SIZE;
29+
}
30+
});
31+
}
32+
33+
private FileUtil(){}
34+
35+
/***
36+
* Write to file synchronously, if it doesn't already exist.
37+
* @param path File path to write to
38+
* @param writer Writes to file
39+
*/
40+
public static void writeSync(Path path, Consumer<Path> writer) {
41+
synchronized (LOCKS.computeIfAbsent(path, key -> new Object())) {
42+
if(!Files.exists(path)) {
43+
writer.accept(path);
44+
}
45+
}
46+
}
47+
}

0 commit comments

Comments
 (0)