Skip to content

Commit c3b160e

Browse files
amvanbarenamvanbaren
authored
Merge pull request #1095 from amvanbaren/improve-file-caching
Use generated file name for file caching
2 parents 8eea18e + 3cd0599 commit c3b160e

File tree

9 files changed

+104
-27
lines changed

9 files changed

+104
-27
lines changed

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

Lines changed: 24 additions & 11 deletions
Original file line numberDiff line numberDiff line change
@@ -11,6 +11,7 @@
1111

1212
import com.fasterxml.jackson.databind.ObjectMapper;
1313
import org.eclipse.openvsx.cache.CacheService;
14+
import org.eclipse.openvsx.cache.FilesCacheKeyGenerator;
1415
import org.eclipse.openvsx.entities.FileResource;
1516
import org.eclipse.openvsx.repositories.RepositoryService;
1617
import org.eclipse.openvsx.storage.StorageUtilService;
@@ -40,11 +41,18 @@ public class WebResourceService {
4041
private final StorageUtilService storageUtil;
4142
private final RepositoryService repositories;
4243
private final CacheService cache;
44+
private final FilesCacheKeyGenerator filesCacheKeyGenerator;
4345

44-
public WebResourceService(StorageUtilService storageUtil, RepositoryService repositories, CacheService cache) {
46+
public WebResourceService(
47+
StorageUtilService storageUtil,
48+
RepositoryService repositories,
49+
CacheService cache,
50+
FilesCacheKeyGenerator filesCacheKeyGenerator
51+
) {
4552
this.storageUtil = storageUtil;
4653
this.repositories = repositories;
4754
this.cache = cache;
55+
this.filesCacheKeyGenerator = filesCacheKeyGenerator;
4856
}
4957

5058
@Cacheable(value = CACHE_WEB_RESOURCE_FILES, keyGenerator = GENERATOR_FILES)
@@ -74,9 +82,11 @@ public Path getWebResource(String namespace, String extension, String targetPlat
7482
if(fileEntry != null) {
7583
var fileExtIndex = fileEntry.getName().lastIndexOf('.');
7684
var fileExt = fileExtIndex != -1 ? fileEntry.getName().substring(fileExtIndex) : "";
77-
var file = Files.createTempFile("webresource_", fileExt);
78-
try(var in = zip.getInputStream(fileEntry)) {
79-
Files.copy(in, file, StandardCopyOption.REPLACE_EXISTING);
85+
var file = filesCacheKeyGenerator.generateCachedWebResourcePath(namespace, extension, targetPlatform, version, name, fileExt);
86+
if(!Files.exists(file)) {
87+
try (var in = zip.getInputStream(fileEntry)) {
88+
Files.copy(in, file);
89+
}
8090
}
8191

8292
return file;
@@ -93,14 +103,17 @@ public Path getWebResource(String namespace, String extension, String targetPlat
93103
return null;
94104
}
95105

96-
var file = Files.createTempFile("webresource_", ".unpkg.json");
97-
var baseUrl = UrlUtil.createApiUrl(UrlUtil.getBaseUrl(), "vscode", "unpkg", namespace, extension, version);
98-
var mapper = new ObjectMapper();
99-
var node = mapper.createArrayNode();
100-
for(var entry : dirEntries) {
101-
node.add(baseUrl + "/" + entry);
106+
var file = filesCacheKeyGenerator.generateCachedWebResourcePath(namespace, extension, targetPlatform, version, name, ".unpkg.json");
107+
if(!Files.exists(file)) {
108+
var baseUrl = UrlUtil.createApiUrl(UrlUtil.getBaseUrl(), "vscode", "unpkg", namespace, extension, version);
109+
var mapper = new ObjectMapper();
110+
var node = mapper.createArrayNode();
111+
for (var entry : dirEntries) {
112+
node.add(baseUrl + "/" + entry);
113+
}
114+
mapper.writeValue(file.toFile(), node);
102115
}
103-
mapper.writeValue(file.toFile(), node);
116+
104117
return file;
105118
} else {
106119
return null;

‎server/src/main/java/org/eclipse/openvsx/cache/ExpiredFileListener.java‎

Lines changed: 10 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -11,19 +11,26 @@
1111

1212
import org.ehcache.event.CacheEvent;
1313
import org.ehcache.event.CacheEventListener;
14+
import org.ehcache.event.EventType;
1415
import org.slf4j.Logger;
1516
import org.slf4j.LoggerFactory;
1617

1718
import java.io.IOException;
1819
import java.nio.file.Files;
1920
import java.nio.file.Path;
2021

21-
public class ExpiredFileListener implements CacheEventListener<String, Path> {
22+
// V can be Path or NullValue (Spring's way of caching null values), that's why Object is used as V type
23+
public class ExpiredFileListener implements CacheEventListener<String, Object> {
2224
protected final Logger logger = LoggerFactory.getLogger(ExpiredFileListener.class);
2325
@Override
24-
public void onEvent(CacheEvent<? extends String, ? extends Path> cacheEvent) {
26+
public void onEvent(CacheEvent<? extends String, ?> cacheEvent) {
2527
logger.info("Expired file cache event: {} | key: {}", cacheEvent.getType(), cacheEvent.getKey());
26-
var path = cacheEvent.getOldValue();
28+
var oldValue = cacheEvent.getOldValue();
29+
var path = oldValue instanceof Path ? (Path) oldValue : null;
30+
if(path == null || (cacheEvent.getType() == EventType.UPDATED && path.equals(cacheEvent.getNewValue()))) {
31+
return;
32+
}
33+
2734
try {
2835
var deleted = Files.deleteIfExists(path);
2936
if(deleted) {

‎server/src/main/java/org/eclipse/openvsx/cache/FilesCacheKeyGenerator.java‎

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

12+
import org.apache.commons.codec.digest.DigestUtils;
1213
import org.eclipse.openvsx.adapter.WebResourceService;
1314
import org.eclipse.openvsx.entities.FileResource;
1415
import org.eclipse.openvsx.storage.IStorageService;
@@ -17,6 +18,7 @@
1718
import org.springframework.stereotype.Component;
1819

1920
import java.lang.reflect.Method;
21+
import java.nio.file.Path;
2022

2123
@Component
2224
public class FilesCacheKeyGenerator implements KeyGenerator {
@@ -47,4 +49,20 @@ public String generate(FileResource resource) {
4749
public String generate(String namespace, String extension, String targetPlatform, String version, String name) {
4850
return UrlUtil.createApiFileUrl("", namespace, extension, targetPlatform, version, name);
4951
}
52+
53+
public Path generateCachedExtensionPath(FileResource resource) {
54+
var key = generate(resource);
55+
return generateCachedPath(key, "ce_", ".tmp");
56+
}
57+
58+
public Path generateCachedWebResourcePath(String namespace, String extension, String targetPlatform, String version, String name, String fileExtension) {
59+
var key = generate(namespace, extension, targetPlatform, version, name);
60+
return generateCachedPath(key, "cr_", fileExtension);
61+
}
62+
63+
private Path generateCachedPath(String key, String prefix, String extension) {
64+
var hash = DigestUtils.sha256Hex(key);
65+
var tmp = System.getProperty("java.io.tmpdir");
66+
return Path.of(tmp, prefix + hash + extension);
67+
}
5068
}

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

Lines changed: 10 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -21,6 +21,7 @@
2121

2222
import org.apache.commons.lang3.ArrayUtils;
2323
import org.apache.commons.lang3.StringUtils;
24+
import org.eclipse.openvsx.cache.FilesCacheKeyGenerator;
2425
import org.eclipse.openvsx.entities.FileResource;
2526
import org.eclipse.openvsx.entities.Namespace;
2627
import org.eclipse.openvsx.util.TempFile;
@@ -47,6 +48,7 @@
4748
public class AwsStorageService implements IStorageService {
4849

4950
private final FileCacheDurationConfig fileCacheDurationConfig;
51+
private final FilesCacheKeyGenerator filesCacheKeyGenerator;
5052

5153
@Value("${ovsx.storage.aws.access-key-id:}")
5254
String accessKeyId;
@@ -68,8 +70,9 @@ public class AwsStorageService implements IStorageService {
6870

6971
private S3Client s3Client;
7072

71-
public AwsStorageService(FileCacheDurationConfig fileCacheDurationConfig) {
73+
public AwsStorageService(FileCacheDurationConfig fileCacheDurationConfig, FilesCacheKeyGenerator filesCacheKeyGenerator) {
7274
this.fileCacheDurationConfig = fileCacheDurationConfig;
75+
this.filesCacheKeyGenerator = filesCacheKeyGenerator;
7376
}
7477

7578
protected S3Client getS3Client() {
@@ -250,10 +253,13 @@ public Path getCachedFile(FileResource resource) throws IOException {
250253
.key(objectKey)
251254
.build();
252255

253-
var path = Files.createTempFile("cached_file", null);
254-
try (var stream = getS3Client().getObject(request)) {
255-
Files.copy(stream, path, StandardCopyOption.REPLACE_EXISTING);
256+
var path = filesCacheKeyGenerator.generateCachedExtensionPath(resource);
257+
if(!Files.exists(path)) {
258+
try (var stream = getS3Client().getObject(request)) {
259+
Files.copy(stream, path);
260+
}
256261
}
262+
257263
return path;
258264
}
259265

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

Lines changed: 12 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -19,6 +19,7 @@
1919
import com.azure.storage.blob.models.CopyStatusType;
2020
import org.apache.commons.lang3.ArrayUtils;
2121
import org.apache.commons.lang3.StringUtils;
22+
import org.eclipse.openvsx.cache.FilesCacheKeyGenerator;
2223
import org.eclipse.openvsx.entities.FileResource;
2324
import org.eclipse.openvsx.entities.Namespace;
2425
import org.eclipse.openvsx.util.TempFile;
@@ -46,6 +47,8 @@ public class AzureBlobStorageService implements IStorageService {
4647

4748
public static final String AZURE_USER_AGENT = "OpenVSX";
4849

50+
private final FilesCacheKeyGenerator filesCacheKeyGenerator;
51+
4952
@Value("${ovsx.storage.azure.service-endpoint:}")
5053
String serviceEndpoint;
5154

@@ -57,6 +60,10 @@ public class AzureBlobStorageService implements IStorageService {
5760

5861
private BlobContainerClient containerClient;
5962

63+
public AzureBlobStorageService(FilesCacheKeyGenerator filesCacheKeyGenerator) {
64+
this.filesCacheKeyGenerator = filesCacheKeyGenerator;
65+
}
66+
6067
@Override
6168
public boolean isEnabled() {
6269
return !StringUtils.isEmpty(serviceEndpoint);
@@ -236,8 +243,11 @@ public Path getCachedFile(FileResource resource) throws IOException {
236243
throw new IllegalStateException(missingEndpointMessage(blobName));
237244
}
238245

239-
var path = Files.createTempFile("cached_file", null);
240-
getContainerClient().getBlobClient(blobName).downloadToFile(path.toAbsolutePath().toString(), true);
246+
var path = filesCacheKeyGenerator.generateCachedExtensionPath(resource);
247+
if(!Files.exists(path)) {
248+
getContainerClient().getBlobClient(blobName).downloadToFile(path.toAbsolutePath().toString());
249+
}
250+
241251
return path;
242252
}
243253
}

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

Lines changed: 13 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -15,6 +15,7 @@
1515
import com.google.cloud.storage.StorageOptions;
1616
import org.apache.commons.lang3.ArrayUtils;
1717
import org.apache.commons.lang3.StringUtils;
18+
import org.eclipse.openvsx.cache.FilesCacheKeyGenerator;
1819
import org.eclipse.openvsx.entities.FileResource;
1920
import org.eclipse.openvsx.entities.Namespace;
2021
import org.eclipse.openvsx.util.TempFile;
@@ -39,6 +40,8 @@ public class GoogleCloudStorageService implements IStorageService {
3940

4041
private static final String BASE_URL = "https://storage.googleapis.com/";
4142

43+
private final FilesCacheKeyGenerator filesCacheKeyGenerator;
44+
4245
@Value("${ovsx.storage.gcp.project-id:}")
4346
String projectId;
4447

@@ -47,6 +50,10 @@ public class GoogleCloudStorageService implements IStorageService {
4750

4851
private Storage storage;
4952

53+
public GoogleCloudStorageService(FilesCacheKeyGenerator filesCacheKeyGenerator) {
54+
this.filesCacheKeyGenerator = filesCacheKeyGenerator;
55+
}
56+
5057
@Override
5158
public boolean isEnabled() {
5259
return !StringUtils.isEmpty(bucketId);
@@ -212,9 +219,12 @@ public Path getCachedFile(FileResource resource) throws IOException {
212219
throw new IllegalStateException(missingBucketIdMessage(resource.getName()));
213220
}
214221

215-
var path = Files.createTempFile("cached_file", null);
216-
var objectId = getObjectId(resource);
217-
getStorage().downloadTo(BlobId.of(bucketId, objectId), path);
222+
var path = filesCacheKeyGenerator.generateCachedExtensionPath(resource);
223+
if(!Files.exists(path)) {
224+
var objectId = getObjectId(resource);
225+
getStorage().downloadTo(BlobId.of(bucketId, objectId), path);
226+
}
227+
218228
return path;
219229
}
220230
}

‎server/src/main/resources/ehcache.xml‎

Lines changed: 2 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -92,6 +92,7 @@
9292
<events-to-fire-on>EXPIRED</events-to-fire-on>
9393
<events-to-fire-on>EVICTED</events-to-fire-on>
9494
<events-to-fire-on>REMOVED</events-to-fire-on>
95+
<events-to-fire-on>UPDATED</events-to-fire-on>
9596
</listener>
9697
</listeners>
9798
<resources>
@@ -110,6 +111,7 @@
110111
<events-to-fire-on>EXPIRED</events-to-fire-on>
111112
<events-to-fire-on>EVICTED</events-to-fire-on>
112113
<events-to-fire-on>REMOVED</events-to-fire-on>
114+
<events-to-fire-on>UPDATED</events-to-fire-on>
113115
</listener>
114116
</listeners>
115117
<resources>

‎server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java‎

Lines changed: 7 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -892,8 +892,13 @@ TokenService tokenService(
892892
}
893893

894894
@Bean
895-
WebResourceService webResourceService(StorageUtilService storageUtil, RepositoryService repositories, CacheService cache) {
896-
return new WebResourceService(storageUtil, repositories, cache);
895+
WebResourceService webResourceService(
896+
StorageUtilService storageUtil,
897+
RepositoryService repositories,
898+
CacheService cache,
899+
FilesCacheKeyGenerator filesCacheKeyGenerator
900+
) {
901+
return new WebResourceService(storageUtil, repositories, cache, filesCacheKeyGenerator);
897902
}
898903

899904
@Bean

‎server/src/test/java/org/eclipse/openvsx/storage/AzureBlobStorageServiceTest.java‎

Lines changed: 8 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -13,6 +13,7 @@
1313

1414
import java.net.URI;
1515

16+
import org.eclipse.openvsx.cache.FilesCacheKeyGenerator;
1617
import org.eclipse.openvsx.entities.Extension;
1718
import org.eclipse.openvsx.entities.ExtensionVersion;
1819
import org.eclipse.openvsx.entities.FileResource;
@@ -60,8 +61,13 @@ void testGetLocation() {
6061
@TestConfiguration
6162
static class TestConfig {
6263
@Bean
63-
AzureBlobStorageService azureBlobStorageService() {
64-
return new AzureBlobStorageService();
64+
FilesCacheKeyGenerator filesCacheKeyGenerator() {
65+
return new FilesCacheKeyGenerator();
66+
}
67+
68+
@Bean
69+
AzureBlobStorageService azureBlobStorageService(FilesCacheKeyGenerator filesCacheKeyGenerator) {
70+
return new AzureBlobStorageService(filesCacheKeyGenerator);
6571
}
6672
}
6773
}

0 commit comments

Comments
 (0)