Skip to content

Commit

Permalink
Improve file cache key
Browse files Browse the repository at this point in the history
- 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
  • Loading branch information
amvanbaren committed Feb 4, 2025
1 parent b8f25b0 commit 5b05b86
Show file tree
Hide file tree
Showing 6 changed files with 69 additions and 56 deletions.
2 changes: 2 additions & 0 deletions server/build.gradle
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand All @@ -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 = [
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -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;

Expand Down Expand Up @@ -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) {
Expand Down
13 changes: 13 additions & 0 deletions server/src/test/java/org/eclipse/openvsx/IntegrationTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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;
Expand All @@ -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")
Expand All @@ -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();
Expand Down
104 changes: 52 additions & 52 deletions server/src/test/java/org/eclipse/openvsx/adapter/VSCodeAPITest.java
Original file line number Diff line number Diff line change
Expand Up @@ -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 {

Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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")));
}
Expand All @@ -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())
Expand All @@ -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())
Expand All @@ -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());
}

Expand All @@ -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())
Expand All @@ -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())
Expand Down Expand Up @@ -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"));
}
Expand All @@ -534,15 +534,15 @@ 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"));
}

@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"));
}
Expand Down Expand Up @@ -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) {
Expand Down

0 comments on commit 5b05b86

Please sign in to comment.