Skip to content
Draft
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions server/src/main/java/org/eclipse/openvsx/ExtensionService.java
Original file line number Diff line number Diff line change
Expand Up @@ -229,6 +229,7 @@ public void updateExtension(Extension extension) {
cache.evictNamespaceDetails(extension);
cache.evictLatestExtensionVersion(extension);
cache.evictExtensionJsons(extension);
evictReferencingExtensionJsons(extension);

if (extension.getVersions().stream().anyMatch(ExtensionVersion::isActive)) {
// There is at least one active version => activate the extension
Expand All @@ -243,6 +244,23 @@ public void updateExtension(Extension extension) {
extension.setLastUpdatedDate(TimeUtil.getCurrentUTC());
}

/**
* Evict the cached extension.json of every extension that references the given one as a bundled
* extension or dependency. Their cached {@code available} flag for that reference (see
* {@link org.eclipse.openvsx.LocalRegistryService#resolveExtensionReferences}) is derived from
* whether this extension currently resolves to an active extension, so it goes stale whenever
* this extension's own active status changes, e.g. it is published for the first time or purged.
*/
private void evictReferencingExtensionJsons(Extension extension) {
var referencingExtensions = new LinkedHashSet<Extension>();
repositories.findBundledExtensionsReference(extension)
.forEach(version -> referencingExtensions.add(version.getExtension()));
repositories.findDependenciesReference(extension)
.forEach(version -> referencingExtensions.add(version.getExtension()));

referencingExtensions.forEach(cache::evictExtensionJsons);
}

/**
* Reactivate all extension versions that have been published by the given user.
*/
Expand Down
41 changes: 18 additions & 23 deletions server/src/main/java/org/eclipse/openvsx/LocalRegistryService.java
Original file line number Diff line number Diff line change
Expand Up @@ -1101,16 +1101,8 @@ public ExtensionJson toExtensionVersionJson(
if (json.getFiles().containsKey(DOWNLOAD_SIG)) {
json.getFiles().put(PUBLIC_KEY, UrlUtil.getPublicKeyUrl(extVersion));
}
if (json.getDependencies() != null) {
for (var ref : json.getDependencies()) {
ref.setUrl(createApiUrl(serverUrl, "api", ref.getNamespace(), ref.getExtension()));
}
}
if (json.getBundledExtensions() != null) {
for (var ref : json.getBundledExtensions()) {
ref.setUrl(createApiUrl(serverUrl, "api", ref.getNamespace(), ref.getExtension()));
}
}
resolveExtensionReferences(json.getDependencies(), serverUrl);
resolveExtensionReferences(json.getBundledExtensions(), serverUrl);
return json;
}

Expand Down Expand Up @@ -1181,16 +1173,8 @@ public ExtensionJson toExtensionVersionJson(
}

json.setFiles(files);
if (json.getDependencies() != null) {
for (var ref : json.getDependencies()) {
ref.setUrl(createApiUrl(serverUrl, "api", ref.getNamespace(), ref.getExtension()));
}
}
if (json.getBundledExtensions() != null) {
for (var ref : json.getBundledExtensions()) {
ref.setUrl(createApiUrl(serverUrl, "api", ref.getNamespace(), ref.getExtension()));
}
}
resolveExtensionReferences(json.getDependencies(), serverUrl);
resolveExtensionReferences(json.getBundledExtensions(), serverUrl);
return json;
}

Expand Down Expand Up @@ -1230,18 +1214,29 @@ public ExtensionJson toExtensionVersionJsonV2(
json.getTargetPlatform(),
json.getVersion());
json.setFiles(toFilesJson(extVersion, resources, fileBaseUrl));
setExtensionReferenceUrls(json.getDependencies(), serverUrl);
setExtensionReferenceUrls(json.getBundledExtensions(), serverUrl);
resolveExtensionReferences(json.getDependencies(), serverUrl);
resolveExtensionReferences(json.getBundledExtensions(), serverUrl);
return json;
}

private void setExtensionReferenceUrls(List<ExtensionReferenceJson> refs, String serverUrl) {
/**
* Fills in the URL and {@code available} flag of every extension-pack / dependency reference.
* These are free-form {@code namespace.extension} strings recorded at publish time with no
* existence check against the registry (unlike regular dependencies, an extension pack may
* legitimately reference extensions that are not, or not yet, published here), so the URL alone
* cannot tell a caller whether following it will actually resolve.
* <p>
* Package-private so a test can exercise it directly rather than mocking everything else
* {@link #toExtensionVersionJsonV2} needs just to reach it.
*/
void resolveExtensionReferences(List<ExtensionReferenceJson> refs, String serverUrl) {
if (refs == null) {
return;
}

for (var ref : refs) {
ref.setUrl(createApiUrl(serverUrl, "api", ref.getNamespace(), ref.getExtension()));
ref.setAvailable(repositories.findActiveExtension(ref.getExtension(), ref.getNamespace()) != null);
}
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -38,6 +38,14 @@ public class ExtensionReferenceJson {
@Schema(hidden = true)
private String version;

@Schema(
description = "Whether the referenced extension currently exists in the registry. An extension "
+ "pack or dependency list may reference extensions that have not been published here (yet), "
+ "or have since been removed - the URL above still points at where it would be, but following "
+ "it returns a 404 while this is false."
)
private boolean available;

public String getUrl() {
return url;
}
Expand Down Expand Up @@ -70,6 +78,14 @@ public void setVersion(String version) {
this.version = version;
}

public boolean isAvailable() {
return available;
}

public void setAvailable(boolean available) {
this.available = available;
}

@Override
public boolean equals(Object o) {
if (this == o) {
Expand All @@ -79,14 +95,15 @@ public boolean equals(Object o) {
return false;
}
ExtensionReferenceJson that = (ExtensionReferenceJson) o;
return Objects.equals(url, that.url)
return available == that.available
&& Objects.equals(url, that.url)
&& Objects.equals(namespace, that.namespace)
&& Objects.equals(extension, that.extension)
&& Objects.equals(version, that.version);
}

@Override
public int hashCode() {
return Objects.hash(url, namespace, extension, version);
return Objects.hash(url, namespace, extension, version, available);
}
}
42 changes: 42 additions & 0 deletions server/src/test/java/org/eclipse/openvsx/ExtensionServiceTest.java
Original file line number Diff line number Diff line change
Expand Up @@ -84,6 +84,16 @@ void setUp() {
scheduler,
scanService,
scanPersistenceService);

// updateExtension() looks up extensions referencing the given one as a bundled extension or
// dependency to evict their cached extension.json; irrelevant to most tests here, so stub it
// leniently rather than in every test that ends up calling updateExtension.
Mockito.lenient()
.when(repositories.findBundledExtensionsReference(Mockito.any()))
.thenReturn(Streamable.empty());
Mockito.lenient()
.when(repositories.findDependenciesReference(Mockito.any()))
.thenReturn(Streamable.empty());
}

@Test
Expand Down Expand Up @@ -348,6 +358,38 @@ void shouldRejectAPackageExceedingTheMaxContentSize() {
.isEqualTo(HttpStatus.CONTENT_TOO_LARGE);
}

/**
* The {@code available} flag cached in the extension.json of an extension pack / dependent extension
* is derived from whether the referenced extension currently resolves (see
* {@link LocalRegistryService#resolveExtensionReferences}), so it goes stale as soon as that
* referenced extension's own active status changes, e.g. it gets published for the first time.
* updateExtension(...) must therefore evict the referencing extensions' cached json too, not just
* the extension's own.
*/
@Test
void shouldEvictExtensionJsonOfExtensionsReferencingThisOneAsBundledOrDependency() {
var extension = mockExtension();

var bundlingExtension = new Extension();
bundlingExtension.setId(10);
var bundlingVersion = plainExtensionVersion(bundlingExtension, "1.0.0");

var dependingExtension = new Extension();
dependingExtension.setId(20);
var dependingVersion = plainExtensionVersion(dependingExtension, "1.0.0");

Mockito.when(repositories.findBundledExtensionsReference(extension))
.thenReturn(Streamable.of(bundlingVersion));
Mockito.when(repositories.findDependenciesReference(extension))
.thenReturn(Streamable.of(dependingVersion));

svc.updateExtension(extension);

Mockito.verify(cache).evictExtensionJsons(extension);
Mockito.verify(cache).evictExtensionJsons(bundlingExtension);
Mockito.verify(cache).evictExtensionJsons(dependingExtension);
}

// ---------- UTILITY ----------//

/**
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -42,6 +42,7 @@
import org.eclipse.openvsx.entities.NamespaceMembership;
import org.eclipse.openvsx.entities.PersonalAccessToken;
import org.eclipse.openvsx.entities.UserData;
import org.eclipse.openvsx.json.ExtensionReferenceJson;
import org.eclipse.openvsx.json.NamespaceJson;
import org.eclipse.openvsx.publish.ExtensionVersionIntegrityService;
import org.eclipse.openvsx.publish.PublishingConfig;
Expand Down Expand Up @@ -267,6 +268,91 @@ void shouldCreateNamespaceAndAssignContributorRole() {
assertThat(persistedMembership.getRole()).isEqualTo(NamespaceMembership.ROLE_CONTRIBUTOR);
}

/**
* Regression coverage for eclipse-openvsx/openvsx#224: extension packs (and dependency lists) may
* legitimately reference extensions that are not, or not yet, published here - publishing them is
* not blocked on it - so the API needs to say which references actually resolve rather than just
* handing out a URL that may 404.
*/
@Test
void shouldFlagWhetherEachExtensionReferenceIsAvailable() {
var published = new ExtensionReferenceJson();
published.setNamespace("foo");
published.setExtension("published-one");
var unpublished = new ExtensionReferenceJson();
unpublished.setNamespace("foo");
unpublished.setExtension("not-here-yet");

when(repositories.findActiveExtension("published-one", "foo")).thenReturn(new Extension());
when(repositories.findActiveExtension("not-here-yet", "foo")).thenReturn(null);

registryService.resolveExtensionReferences(List.of(published, unpublished), "https://open-vsx.org");

assertThat(published.getUrl()).isEqualTo("https://open-vsx.org/api/foo/published-one");
assertThat(published.isAvailable()).isTrue();
assertThat(unpublished.getUrl()).isEqualTo("https://open-vsx.org/api/foo/not-here-yet");
assertThat(unpublished.isAvailable()).isFalse();
}

@Test
void shouldTolerateNullExtensionReferenceList() {
// getDependencies()/getBundledExtensions() are null whenever the version records none - must
// be a no-op rather than a NullPointerException.
registryService.resolveExtensionReferences(null, "https://open-vsx.org");
}

/**
* toExtensionVersionJson(ExtensionVersion, String, String) backs the v1 getExtension(...) API and
* used to fill in only the URL of each bundled-extension/dependency reference, duplicating (and
* missing half of) what resolveExtensionReferences(...) does for v2 - so a reference to an
* extension that is not (yet) published always came back without the {@code available} flag.
*/
@Test
void shouldFlagAvailabilityOfReferencesInV1ExtensionJson() {
var extVersion = mockExtensionVersionWithBundledExtension();
var extension = extVersion.getExtension();
when(repositories.findLatestVersionForAllUrls(extension, null, false, true)).thenReturn(null);
when(repositories.findLatestVersionForAllUrls(extension, null, true, true)).thenReturn(null);
when(repositories.findVersionStringsSorted(extension, null, true)).thenReturn(List.of());
when(storageUtilService.getFileUrls(any(), any(), any(String[].class))).thenReturn(Map.of(1L, Map.of()));
when(repositories.findActiveExtension("bar", "foo")).thenReturn(null);

var json = registryService.toExtensionVersionJson(extVersion, null, true);

assertThat(json.getBundledExtensions()).hasSize(1);
assertThat(json.getBundledExtensions().getFirst().isAvailable()).isFalse();
}

/**
* Same regression as above, for the other v1 overload of toExtensionVersionJson(...) (the one
* backing the v1 query(...) API).
*/
@Test
void shouldFlagAvailabilityOfReferencesInV1QueryExtensionJson() {
var extVersion = mockExtensionVersionWithBundledExtension();
when(repositories.findActiveExtension("bar", "foo")).thenReturn(new Extension());

var json = registryService
.toExtensionVersionJson(extVersion, null, null, 0L, false, null, null, List.of(), Map.of());

assertThat(json.getBundledExtensions()).hasSize(1);
assertThat(json.getBundledExtensions().getFirst().isAvailable()).isTrue();
}

private ExtensionVersion mockExtensionVersionWithBundledExtension() {
var namespace = new Namespace();
namespace.setName("foo");
var extension = new Extension();
extension.setNamespace(namespace);
extension.setName("baz");
var extVersion = new ExtensionVersion();
extVersion.setId(1L);
extVersion.setExtension(extension);
extVersion.setVersion("1.0.0");
extVersion.setBundledExtensions(List.of("foo.bar"));
return extVersion;
}

@Test
void shouldHoldBackTheMostRecentChanges() {
// A request that reaches the present is clamped to the lag, so an entry whose transaction may
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -3351,6 +3351,7 @@ private PersonalAccessToken mockForDelete(boolean isOwner, boolean isMember) {
.then(
(Answer<Boolean>) invocation -> ((TargetPlatformVersion[]) invocation
.getRawArguments()[2]).length == versions.size());
Mockito.when(repositories.findBundledExtensionsReference(extension)).thenReturn(Streamable.empty());
Mockito.when(repositories.findDependenciesReference(extension)).thenReturn(Streamable.empty());
return token;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -106,6 +106,16 @@ void setup() {
eclipse.publisherAgreementAllowedVersions = List.of("1", "1.0", "1.1");
eclipse.publisherAgreementVersion = "1.1";
eclipse.eclipseApiUrl = "https://test.openvsx.eclipse.org/";

// Reactivating an extension goes through ExtensionService.updateExtension(...), which looks up
// extensions referencing it as a bundled extension or dependency to evict their cached
// extension.json; irrelevant here, so stub it leniently rather than in every affected test.
Mockito.lenient()
.when(repositories.findBundledExtensionsReference(any()))
.thenReturn(Streamable.empty());
Mockito.lenient()
.when(repositories.findDependenciesReference(any()))
.thenReturn(Streamable.empty());
}

@Test
Expand Down
1 change: 1 addition & 0 deletions webui/CHANGELOG.md
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ This change log covers only the frontend library (webui) of Open VSX.
- Add a "Data Consistency" page to the admin dashboard (#1622): a live overview of every registered consistency check's finding count, with actions to refresh it and to fix findings one at a time or all at once
- Show a "Namespace not verified" state on an extension card when it can't be activated because its namespace already exists in a referenced external gallery and hasn't been verified, in both the "My Extensions" and namespace member extension lists. The card keeps its colour and takes a warning-toned frame and icon, since this is the publisher's to fix rather than an extension that is simply switched off
- Show a warning notice with a claim action wherever an unverified namespace is holding something back — the extension settings page when the extension has a namespace ownership conflict, and the namespace settings page for any unverified namespace — making clear the namespace must be claimed (verified) first. The action is the deployment's configured `elements.claimNamespace`, falling back to the namespace access documentation when none is configured. The admin dashboard's extension and namespace views show the same explanation without the claim action, since claiming is the publisher's action to take, not an admin's on someone else's behalf
- Mark a bundled extension or dependency on the extension detail page as "(not available)" instead of linking to it, when the registry says it doesn't exist here — an extension pack may legitimately reference extensions that were never published, or have since been removed (#224)

### Changed

Expand Down
5 changes: 5 additions & 0 deletions webui/src/extension-registry-types.ts
Original file line number Diff line number Diff line change
Expand Up @@ -135,6 +135,11 @@ export interface ExtensionReference {
namespace: string;
extension: string;
version?: string;
// Whether the referenced extension currently exists in the registry. An extension pack or
// dependency list may reference extensions that have not been published here (yet), or have
// since been removed. Absent is treated the same as available, for compatibility with older
// registries that don't send this field.
available?: boolean;
}

export interface TargetPlatformActive {
Expand Down
17 changes: 15 additions & 2 deletions webui/src/pages/extension-detail/extension-detail-overview.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -273,10 +273,23 @@ export const ExtensionDetailOverview: FunctionComponent<ExtensionDetailOverviewP
}));

const renderExtensionRef = (ref: ExtensionReference): ReactNode => {
const label = `${ref.namespace}.${ref.extension}`;
// A pack/dependency may name an extension that isn't published here (yet) - publishing
// isn't blocked on it - so a link would just 404. Say so instead of linking out. Absent
// (older registry, field not yet sent) is treated as available, same as the field's own docs.
if (ref.available === false) {
return (
<Box key={label}>
<Typography component='span' variant='body2' color='text.disabled'>
{label} (not available)
</Typography>
</Box>
);
}
return (
<Box key={`${ref.namespace}.${ref.extension}`}>
<Box key={label}>
<StyledRouteLink to={createRoute([ExtensionDetailRoutes.ROOT, ref.namespace, ref.extension])}>
{ref.namespace}.{ref.extension}
{label}
</StyledRouteLink>
</Box>
);
Expand Down
Loading