From 5b05b86a1334bd643fc628751b8941441876340a Mon Sep 17 00:00:00 2001 From: amvanbaren Date: Tue, 4 Feb 2025 13:28:26 +0200 Subject: [PATCH] Improve file cache key - Differentiate between `null` and `universal` target platform. `UrlUtil.getFileApiUrl` treats `universal` as `null` and excludes it from the url. - Make namespace and extension names case insensitive, same as in the database. - Pass `targetPlatform` when evicting webresource file. - Add test logging plugin - Remove all caches after integration test --- server/build.gradle | 2 + .../openvsx/adapter/LocalVSCodeService.java | 2 +- .../openvsx/adapter/WebResourceService.java | 1 - .../openvsx/cache/FilesCacheKeyGenerator.java | 3 +- .../org/eclipse/openvsx/IntegrationTest.java | 13 +++ .../openvsx/adapter/VSCodeAPITest.java | 104 +++++++++--------- 6 files changed, 69 insertions(+), 56 deletions(-) diff --git a/server/build.gradle b/server/build.gradle index ed00a9354..b4bb71f96 100644 --- a/server/build.gradle +++ b/server/build.gradle @@ -4,6 +4,7 @@ buildscript { } dependencies { classpath "org.hibernate.orm:hibernate-gradle-plugin:6.2.2.Final" + classpath "com.adarshr.test-logger:com.adarshr.test-logger.gradle.plugin:4.0.0" } } plugins { @@ -16,6 +17,7 @@ plugins { id 'java' } apply plugin: 'org.hibernate.orm' +apply plugin: 'com.adarshr.test-logger' def jooqSrcDir = 'src/main/jooq-gen' def versions = [ diff --git a/server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java b/server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java index 1b1375e4c..f3d675d46 100644 --- a/server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java +++ b/server/src/main/java/org/eclipse/openvsx/adapter/LocalVSCodeService.java @@ -342,7 +342,7 @@ private Path getWebResource(String namespaceName, String extensionName, String t } if(!Files.exists(file)) { logger.error("File doesn't exist {}", file); - cache.evictWebResourceFile(namespaceName, extensionName, null, version, name); + cache.evictWebResourceFile(namespaceName, extensionName, targetPlatform, version, name); throw new NotFoundException(); } return file; diff --git a/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java b/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java index 2e49dbb00..c8155e612 100644 --- a/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java +++ b/server/src/main/java/org/eclipse/openvsx/adapter/WebResourceService.java @@ -101,7 +101,6 @@ public Path getWebResource(String namespace, String extension, String targetPlat return null; } - var file = filesCacheKeyGenerator.generateCachedWebResourcePath(namespace, extension, targetPlatform, version, name, ".unpkg.json"); FileUtil.writeSync(file, (p) -> { var baseUrl = UrlUtil.createApiUrl(UrlUtil.getBaseUrl(), "vscode", "unpkg", namespace, extension, version); diff --git a/server/src/main/java/org/eclipse/openvsx/cache/FilesCacheKeyGenerator.java b/server/src/main/java/org/eclipse/openvsx/cache/FilesCacheKeyGenerator.java index 355c9567a..53ad6b3e7 100644 --- a/server/src/main/java/org/eclipse/openvsx/cache/FilesCacheKeyGenerator.java +++ b/server/src/main/java/org/eclipse/openvsx/cache/FilesCacheKeyGenerator.java @@ -13,7 +13,6 @@ import org.eclipse.openvsx.adapter.WebResourceService; import org.eclipse.openvsx.entities.FileResource; import org.eclipse.openvsx.storage.IStorageService; -import org.eclipse.openvsx.util.UrlUtil; import org.springframework.cache.interceptor.KeyGenerator; import org.springframework.stereotype.Component; @@ -47,7 +46,7 @@ public String generate(FileResource resource) { } public String generate(String namespace, String extension, String targetPlatform, String version, String name) { - return UrlUtil.createApiFileUrl("", namespace, extension, targetPlatform, version, name); + return String.join("|", namespace.toLowerCase(), extension.toLowerCase(), targetPlatform, version, name); } public Path generateCachedExtensionPath(FileResource resource) { diff --git a/server/src/test/java/org/eclipse/openvsx/IntegrationTest.java b/server/src/test/java/org/eclipse/openvsx/IntegrationTest.java index fa5d208b5..2cf272b6f 100644 --- a/server/src/test/java/org/eclipse/openvsx/IntegrationTest.java +++ b/server/src/test/java/org/eclipse/openvsx/IntegrationTest.java @@ -11,6 +11,7 @@ import com.fasterxml.jackson.databind.JsonNode; import org.eclipse.openvsx.json.*; +import org.junit.jupiter.api.AfterAll; import org.junit.jupiter.api.Test; import org.slf4j.Logger; import org.slf4j.LoggerFactory; @@ -22,11 +23,14 @@ import org.springframework.http.HttpStatus; import org.springframework.test.context.ActiveProfiles; +import javax.cache.CacheManager; import java.io.IOException; import java.net.URI; import java.net.URISyntaxException; import static org.assertj.core.api.Assertions.assertThat; +import static org.eclipse.openvsx.cache.CacheService.CACHE_EXTENSION_FILES; +import static org.eclipse.openvsx.cache.CacheService.CACHE_WEB_RESOURCE_FILES; @SpringBootTest(webEnvironment = WebEnvironment.RANDOM_PORT) @ActiveProfiles("test") @@ -47,6 +51,15 @@ private String apiCall(String path) { return "http://localhost:" + port + path; } + @AfterAll + static void clearFileCaches(@Autowired CacheManager cacheManager) { + var cache = cacheManager.getCache(CACHE_WEB_RESOURCE_FILES); + cache.removeAll(); + + cache = cacheManager.getCache(CACHE_EXTENSION_FILES); + cache.removeAll(); + } + @Test void testPublishExtension() throws Exception { testService.createUser(); diff --git a/server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java b/server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java index baf33e382..f9f688063 100644 --- a/server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java +++ b/server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java @@ -76,10 +76,10 @@ @WebMvcTest(VSCodeAPI.class) @AutoConfigureWebClient @MockBean({ - ClientRegistrationRepository.class, GoogleCloudStorageService.class, AzureBlobStorageService.class, - AwsStorageService.class, AzureDownloadCountService.class, CacheService.class, UpstreamVSCodeService.class, - VSCodeIdService.class, EntityManager.class, EclipseService.class, ExtensionValidator.class, SimpleMeterRegistry.class, - FileCacheDurationConfig.class + ClientRegistrationRepository.class, GoogleCloudStorageService.class, AzureBlobStorageService.class, + AwsStorageService.class, AzureDownloadCountService.class, CacheService.class, UpstreamVSCodeService.class, + VSCodeIdService.class, EntityManager.class, EclipseService.class, ExtensionValidator.class, SimpleMeterRegistry.class, + FileCacheDurationConfig.class }) class VSCodeAPITest { @@ -104,8 +104,8 @@ void testSearch() throws Exception { mockExtensionVersions(extension, null, "universal"); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("search-yaml-query.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("search-yaml-query.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("search-yaml-response.json"))); } @@ -117,8 +117,8 @@ void testSearchMacOSXTarget() throws Exception { mockExtensionVersions(extension, targetPlatform, targetPlatform); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("search-yaml-query-darwin.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("search-yaml-query-darwin.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("search-yaml-response-darwin.json"))); } @@ -129,8 +129,8 @@ void testSearchExcludeBuiltInExtensions() throws Exception { mockExtensionVersions(extension, null,"universal"); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("search-yaml-query.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("search-yaml-query.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("search-yaml-response-builtin-extensions.json"))); } @@ -141,8 +141,8 @@ void testSearchMultipleTargetsResponse() throws Exception { mockExtensionVersions(extension, null, "darwin-x64", "linux-x64", "alpine-arm64"); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("search-yaml-query.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("search-yaml-query.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("search-yaml-response-targets.json"))); } @@ -153,8 +153,8 @@ void testFindById() throws Exception { mockExtensionVersions(extension, null, "universal"); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("findid-yaml-query.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("findid-yaml-query.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("findid-yaml-response.json"))); } @@ -166,8 +166,8 @@ void testFindByIdAlpineTarget() throws Exception { mockExtensionVersions(extension, targetPlatform, targetPlatform); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("findid-yaml-query-alpine.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("findid-yaml-query-alpine.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("findid-yaml-response-alpine.json"))); } @@ -178,8 +178,8 @@ void testFindByIdDuplicate() throws Exception { mockExtensionVersions(extension, null, "universal"); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("findid-yaml-duplicate-query.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("findid-yaml-duplicate-query.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("findid-yaml-response.json"))); } @@ -188,8 +188,8 @@ void testFindByIdDuplicate() throws Exception { void testFindByIdInactive() throws Exception { mockSearch(false); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("findid-yaml-query.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("findid-yaml-query.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("empty-response.json"))); } @@ -200,8 +200,8 @@ void testFindByName() throws Exception { mockExtensionVersions(extension, null, "universal"); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("findname-yaml-query.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("findname-yaml-query.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("findname-yaml-response.json"))); } @@ -213,8 +213,8 @@ void testFindByNameLinuxTarget() throws Exception { mockExtensionVersions(extension, targetPlatform, targetPlatform); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("findname-yaml-query-linux.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("findname-yaml-query-linux.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("findname-yaml-response-linux.json"))); } @@ -225,8 +225,8 @@ void testFindByNameDuplicate() throws Exception { mockExtensionVersions(extension, null,"universal"); mockMvc.perform(post("/vscode/gallery/extensionquery") - .content(file("findname-yaml-duplicate-query.json")) - .contentType(MediaType.APPLICATION_JSON)) + .content(file("findname-yaml-duplicate-query.json")) + .contentType(MediaType.APPLICATION_JSON)) .andExpect(status().isOk()) .andExpect(content().json(file("findname-yaml-response.json"))); } @@ -240,7 +240,7 @@ void testAsset() throws Exception { Files.writeString(path, "{\"foo\":\"bar\"}"); mockMvc.perform(get("/vscode/asset/{namespace}/{extensionName}/{version}/{assetType}", - "redhat", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.Manifest")) + "redhat", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.Manifest")) .andExpect(request().asyncStarted()) .andDo(MvcResult::getAsyncResult) .andExpect(status().isOk()) @@ -258,7 +258,7 @@ void testAssetMacOSX() throws Exception { Files.writeString(path, "{\"foo\":\"bar\",\"target\":\"darwin-arm64\"}"); mockMvc.perform(get("/vscode/asset/{namespace}/{extensionName}/{version}/{assetType}?targetPlatform={target}", - "redhat", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.Manifest", target)) + "redhat", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.Manifest", target)) .andExpect(request().asyncStarted()) .andDo(MvcResult::getAsyncResult) .andExpect(status().isOk()) @@ -272,7 +272,7 @@ void testAssetNotFound() throws Exception { Mockito.when(repositories.findFileByType("redhat", "vscode-yaml", "universal", "0.5.2", FileResource.MANIFEST)) .thenReturn(null); mockMvc.perform(get("/vscode/asset/{namespace}/{extensionName}/{version}/{assetType}", - "redhat", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.Manifest")) + "redhat", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.Manifest")) .andExpect(status().isNotFound()); } @@ -290,7 +290,7 @@ void testWebResourceAsset() throws Exception { } } mockMvc.perform(get("/vscode/asset/{namespace}/{extensionName}/{version}/{assetType}", - namespaceName, extensionName, version, "Microsoft.VisualStudio.Code.WebResources/extension/EditorConfig_icon.png")) + namespaceName, extensionName, version, "Microsoft.VisualStudio.Code.WebResources/extension/EditorConfig_icon.png")) .andExpect(request().asyncStarted()) .andDo(MvcResult::getAsyncResult) .andExpect(status().isOk()) @@ -302,14 +302,14 @@ void testWebResourceAsset() throws Exception { void testNotWebResourceAsset() throws Exception { mockExtensionVersion(); mockMvc.perform(get("/vscode/asset/{namespace}/{extensionName}/{version}/{assetType}", - "redhat", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.WebResources/img/logo.png")) + "redhat", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.WebResources/img/logo.png")) .andExpect(status().isNotFound()); } @Test void testAssetExcludeBuiltInExtensions() throws Exception { mockMvc.perform(get("/vscode/asset/{namespace}/{extensionName}/{version}/{assetType}", - "vscode", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.Manifest")) + "vscode", "vscode-yaml", "0.5.2", "Microsoft.VisualStudio.Code.Manifest")) .andExpect(request().asyncStarted()) .andDo(MvcResult::getAsyncResult) .andExpect(status().isBadRequest()) @@ -525,7 +525,7 @@ void testBrowseIcon() throws Exception { void testDownload() throws Exception { mockExtensionVersion(); mockMvc.perform(get("/vscode/gallery/publishers/{namespace}/vsextensions/{extension}/{version}/vspackage", - "redhat", "vscode-yaml", "0.5.2")) + "redhat", "vscode-yaml", "0.5.2")) .andExpect(status().isFound()) .andExpect(header().string("Location", "http://localhost/vscode/asset/redhat/vscode-yaml/0.5.2/Microsoft.VisualStudio.Services.VSIXPackage")); } @@ -534,7 +534,7 @@ void testDownload() throws Exception { void testDownloadMacOSX() throws Exception { mockExtensionVersion("darwin-arm64"); mockMvc.perform(get("/vscode/gallery/publishers/{namespace}/vsextensions/{extension}/{version}/vspackage?targetPlatform={target}", - "redhat", "vscode-yaml", "0.5.2", "darwin-arm64")) + "redhat", "vscode-yaml", "0.5.2", "darwin-arm64")) .andExpect(status().isFound()) .andExpect(header().string("Location", "http://localhost/vscode/asset/redhat/vscode-yaml/0.5.2/Microsoft.VisualStudio.Services.VSIXPackage?targetPlatform=darwin-arm64")); } @@ -542,7 +542,7 @@ void testDownloadMacOSX() throws Exception { @Test void testDownloadExcludeBuiltInExtensions() throws Exception { mockMvc.perform(get("/vscode/gallery/publishers/{namespace}/vsextensions/{extension}/{version}/vspackage", - "vscode", "vscode-yaml", "0.5.2")) + "vscode", "vscode-yaml", "0.5.2")) .andExpect(status().isBadRequest()) .andExpect(status().reason("Built-in extension namespace 'vscode' not allowed")); } @@ -592,23 +592,23 @@ private Extension mockSearch(String targetPlatform, String namespaceName, boolea } private Extension mockExtension() { - var namespace = new Namespace(); - namespace.setId(2); - namespace.setPublicId("test-2"); - namespace.setName("redhat"); - - var extension = new Extension(); - extension.setId(1); - extension.setPublicId("test-1"); - extension.setName("vscode-yaml"); - extension.setAverageRating(3.0); - extension.setReviewCount(10L); - extension.setDownloadCount(100); - extension.setPublishedDate(LocalDateTime.parse("1999-12-01T09:00")); - extension.setLastUpdatedDate(LocalDateTime.parse("2000-01-01T10:00")); - extension.setNamespace(namespace); - - return extension; + var namespace = new Namespace(); + namespace.setId(2); + namespace.setPublicId("test-2"); + namespace.setName("redhat"); + + var extension = new Extension(); + extension.setId(1); + extension.setPublicId("test-1"); + extension.setName("vscode-yaml"); + extension.setAverageRating(3.0); + extension.setReviewCount(10L); + extension.setDownloadCount(100); + extension.setPublishedDate(LocalDateTime.parse("1999-12-01T09:00")); + extension.setLastUpdatedDate(LocalDateTime.parse("2000-01-01T10:00")); + extension.setNamespace(namespace); + + return extension; } private void mockExtensionVersions(Extension extension, String queryTargetPlatform, String... targetPlatforms) {