feat(server): refresh application registration on install (#22527)
Part of the app settings architecture cleanup (twentyhq/core-team-issues#2456) — unifies registration ingestion across sources. ## Problem The dev sync, catalog sync and tarball upload flows all refresh the `applicationRegistration` row (manifest + display columns) at ingestion time, but the install/upgrade flow never did. Installing or upgrading an app relied on catalog sync having run beforehand, so a registration could serve stale display data (name, logo, description, screenshots…) after an install that shipped a newer manifest. ## Changes - `doInstallApplication` now refreshes the global registration from the resolved manifest after all install steps succeed (post-install hook included, so a hook failure that triggers uninstall can't leave the registration refreshed for a failed install). - Downgrade guard: the refresh is skipped when the installed version is provably older than `latestAvailableVersion` (per-workspace installs of an older version never downgrade the global registration). Extracted as a pure util `shouldRefreshApplicationRegistrationOnInstall` with unit tests: - `latestAvailableVersion` null or invalid semver → refresh - installed ≥ latest → refresh, and `latestAvailableVersion` is bumped to the installed version - installed < latest, or installed not valid semver while latest is → skip - Asset URLs mirror the existing per-source ingestion behavior: NPM registrations get manifest `logoUrl`/`screenshots` resolved to registry CDN URLs (same as catalog sync); tarball and other sources persist the manifest as-is (same as tarball upload). - `updateFromManifest` gains an optional `latestAvailableVersion` param (same conditional-spread style as `sourceType`). - `ApplicationRegistrationModule` added to `ApplicationInstallModule` imports (no cycle: nothing in the registration module's import graph imports the install module). The dev sync flow (`syncRegistrationMetadata`) already goes through `updateFromManifest` and writes the display columns — verified, no change needed. ## Verification - New unit spec: 6 cases on the guard util - `npx jest "application-registration|application-install|marketplace"` → 3 suites, 21 tests passed - `npx nx typecheck twenty-server` → success - `npx nx lint:diff-with-main twenty-server` → clean <!-- This is an auto-generated description by cubic. --> <a href="https://cubic.dev/pr/twentyhq/twenty/pull/22527?utm_source=github" target="_blank" rel="noopener noreferrer" data-no-image-dialog="true"><picture><source media="(prefers-color-scheme: dark)" srcset="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"><source media="(prefers-color-scheme: light)" srcset="https://www.cubic.dev/buttons/review-in-cubic-light.svg"><img alt="Review in cubic" src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg"></picture></a> <!-- End of auto-generated description by cubic. --> --------- Co-authored-by: Charles Bochet <charles@twenty.com>
This commit is contained in:
+2
@@ -4,6 +4,7 @@ import { TypeOrmModule } from '@nestjs/typeorm';
|
||||
import { CacheLockModule } from 'src/engine/core-modules/cache-lock/cache-lock.module';
|
||||
import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module';
|
||||
import { ApplicationRegistrationEntity } from 'src/engine/core-modules/application/application-registration/application-registration.entity';
|
||||
import { ApplicationRegistrationModule } from 'src/engine/core-modules/application/application-registration/application-registration.module';
|
||||
import { ApplicationModule } from 'src/engine/core-modules/application/application.module';
|
||||
import { ApplicationManifestModule } from 'src/engine/core-modules/application/application-manifest/application-manifest.module';
|
||||
import { ApplicationPackageModule } from 'src/engine/core-modules/application/application-package/application-package.module';
|
||||
@@ -19,6 +20,7 @@ import { WorkspaceCacheModule } from 'src/engine/workspace-cache/workspace-cache
|
||||
imports: [
|
||||
TypeOrmModule.forFeature([ApplicationRegistrationEntity]),
|
||||
ApplicationModule,
|
||||
ApplicationRegistrationModule,
|
||||
ApplicationManifestModule,
|
||||
ApplicationPackageModule,
|
||||
CacheLockModule,
|
||||
|
||||
+30
@@ -15,7 +15,9 @@ import {
|
||||
ApplicationExceptionCode,
|
||||
} from 'src/engine/core-modules/application/application.exception';
|
||||
import { ApplicationRegistrationEntity } from 'src/engine/core-modules/application/application-registration/application-registration.entity';
|
||||
import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service';
|
||||
import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum';
|
||||
import { ManifestAssetUrlResolverService } from 'src/engine/core-modules/application/application-registration/manifest-asset-url-resolver.service';
|
||||
import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity';
|
||||
import { ApplicationService } from 'src/engine/core-modules/application/application.service';
|
||||
import { ApplicationPackageFetcherService } from 'src/engine/core-modules/application/application-package/application-package-fetcher.service';
|
||||
@@ -55,6 +57,7 @@ export class ApplicationInstallService {
|
||||
@InjectRepository(ApplicationRegistrationEntity)
|
||||
private readonly appRegistrationRepository: Repository<ApplicationRegistrationEntity>,
|
||||
private readonly applicationService: ApplicationService,
|
||||
private readonly applicationRegistrationService: ApplicationRegistrationService,
|
||||
private readonly applicationPackageFetcherService: ApplicationPackageFetcherService,
|
||||
private readonly applicationVersionValidationService: ApplicationVersionValidationService,
|
||||
private readonly applicationSyncService: ApplicationSyncService,
|
||||
@@ -65,6 +68,7 @@ export class ApplicationInstallService {
|
||||
@InjectMessageQueue(MessageQueue.logicFunctionQueue)
|
||||
private readonly messageQueueService: MessageQueueService,
|
||||
private readonly workspaceCacheService: WorkspaceCacheService,
|
||||
private readonly manifestAssetUrlResolverService: ManifestAssetUrlResolverService,
|
||||
) {}
|
||||
|
||||
async installApplication(params: {
|
||||
@@ -270,6 +274,12 @@ export class ApplicationInstallService {
|
||||
universalIdentifier,
|
||||
});
|
||||
|
||||
await this.refreshRegistrationFromInstall({
|
||||
appRegistration,
|
||||
manifest: resolvedPackage.manifest,
|
||||
installedVersion: newVersion,
|
||||
});
|
||||
|
||||
this.logger.log(
|
||||
`Successfully installed app ${universalIdentifier} v${resolvedPackage.packageJson.version ?? 'unknown'}`,
|
||||
);
|
||||
@@ -297,6 +307,26 @@ export class ApplicationInstallService {
|
||||
}
|
||||
}
|
||||
|
||||
private async refreshRegistrationFromInstall(params: {
|
||||
appRegistration: ApplicationRegistrationEntity;
|
||||
manifest: Manifest;
|
||||
installedVersion: string;
|
||||
}): Promise<void> {
|
||||
const { appRegistration, manifest, installedVersion } = params;
|
||||
|
||||
await this.applicationRegistrationService.updateFromManifest({
|
||||
applicationRegistrationId: appRegistration.id,
|
||||
manifest: this.manifestAssetUrlResolverService.resolveFromRegistration({
|
||||
sourceType: appRegistration.sourceType,
|
||||
sourcePackage: appRegistration.sourcePackage,
|
||||
manifest,
|
||||
version: installedVersion,
|
||||
}),
|
||||
latestAvailableVersion: installedVersion,
|
||||
preventVersionDowngrade: true,
|
||||
});
|
||||
}
|
||||
|
||||
private async runPreInstallHook(params: {
|
||||
manifest: Manifest;
|
||||
workspaceId: string;
|
||||
|
||||
+57
@@ -0,0 +1,57 @@
|
||||
import { shouldRefreshApplicationRegistrationOnInstall } from 'src/engine/core-modules/application/application-install/utils/should-refresh-application-registration-on-install.util';
|
||||
|
||||
describe('shouldRefreshApplicationRegistrationOnInstall', () => {
|
||||
it('should refresh when latestAvailableVersion is null', () => {
|
||||
expect(
|
||||
shouldRefreshApplicationRegistrationOnInstall({
|
||||
installedVersion: '1.0.0',
|
||||
latestAvailableVersion: null,
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should refresh when latestAvailableVersion is not a valid semver', () => {
|
||||
expect(
|
||||
shouldRefreshApplicationRegistrationOnInstall({
|
||||
installedVersion: '1.0.0',
|
||||
latestAvailableVersion: 'not-a-version',
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should refresh when installed version is newer than latestAvailableVersion', () => {
|
||||
expect(
|
||||
shouldRefreshApplicationRegistrationOnInstall({
|
||||
installedVersion: '2.0.0',
|
||||
latestAvailableVersion: '1.0.0',
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should refresh when installed version equals latestAvailableVersion', () => {
|
||||
expect(
|
||||
shouldRefreshApplicationRegistrationOnInstall({
|
||||
installedVersion: '1.2.3',
|
||||
latestAvailableVersion: '1.2.3',
|
||||
}),
|
||||
).toBe(true);
|
||||
});
|
||||
|
||||
it('should not refresh when installed version is older than latestAvailableVersion', () => {
|
||||
expect(
|
||||
shouldRefreshApplicationRegistrationOnInstall({
|
||||
installedVersion: '1.0.0',
|
||||
latestAvailableVersion: '2.0.0',
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
|
||||
it('should not refresh when installed version is not a valid semver and latestAvailableVersion is valid', () => {
|
||||
expect(
|
||||
shouldRefreshApplicationRegistrationOnInstall({
|
||||
installedVersion: 'not-a-version',
|
||||
latestAvailableVersion: '1.0.0',
|
||||
}),
|
||||
).toBe(false);
|
||||
});
|
||||
});
|
||||
+23
@@ -0,0 +1,23 @@
|
||||
import semver from 'semver';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
export const shouldRefreshApplicationRegistrationOnInstall = ({
|
||||
installedVersion,
|
||||
latestAvailableVersion,
|
||||
}: {
|
||||
installedVersion: string;
|
||||
latestAvailableVersion: string | null;
|
||||
}): boolean => {
|
||||
if (
|
||||
!isDefined(latestAvailableVersion) ||
|
||||
!isDefined(semver.valid(latestAvailableVersion))
|
||||
) {
|
||||
return true;
|
||||
}
|
||||
|
||||
if (!isDefined(semver.valid(installedVersion))) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return semver.gte(installedVersion, latestAvailableVersion);
|
||||
};
|
||||
+8
-16
@@ -3,9 +3,7 @@ import { Injectable, Logger } from '@nestjs/common';
|
||||
import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service';
|
||||
import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum';
|
||||
import { MarketplaceService } from 'src/engine/core-modules/application/application-marketplace/marketplace.service';
|
||||
import { buildRegistryCdnUrl } from 'src/engine/core-modules/application/application-marketplace/utils/build-registry-cdn-url.util';
|
||||
import { resolveManifestAssetUrls } from 'src/engine/core-modules/application/application-marketplace/utils/resolve-manifest-asset-urls.util';
|
||||
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
import { ManifestAssetUrlResolverService } from 'src/engine/core-modules/application/application-registration/manifest-asset-url-resolver.service';
|
||||
|
||||
@Injectable()
|
||||
export class MarketplaceCatalogSyncService {
|
||||
@@ -14,7 +12,7 @@ export class MarketplaceCatalogSyncService {
|
||||
constructor(
|
||||
private readonly applicationRegistrationService: ApplicationRegistrationService,
|
||||
private readonly marketplaceService: MarketplaceService,
|
||||
private readonly twentyConfigService: TwentyConfigService,
|
||||
private readonly manifestAssetUrlResolverService: ManifestAssetUrlResolverService,
|
||||
) {}
|
||||
|
||||
async syncCatalog(): Promise<void> {
|
||||
@@ -45,18 +43,12 @@ export class MarketplaceCatalogSyncService {
|
||||
const universalIdentifier =
|
||||
fetchedManifest.application.universalIdentifier;
|
||||
|
||||
const cdnBaseUrl = this.twentyConfigService.get('APP_REGISTRY_CDN_URL');
|
||||
|
||||
const manifestWithResolvedUrls = resolveManifestAssetUrls(
|
||||
fetchedManifest,
|
||||
(filePath) =>
|
||||
buildRegistryCdnUrl({
|
||||
cdnBaseUrl,
|
||||
packageName: pkg.name,
|
||||
version: pkg.version,
|
||||
filePath,
|
||||
}),
|
||||
);
|
||||
const manifestWithResolvedUrls =
|
||||
this.manifestAssetUrlResolverService.resolveFromRegistrySource({
|
||||
manifest: fetchedManifest,
|
||||
packageName: pkg.name,
|
||||
version: pkg.version,
|
||||
});
|
||||
|
||||
await this.applicationRegistrationService.upsertFromCatalog({
|
||||
universalIdentifier,
|
||||
|
||||
+5
@@ -6,9 +6,11 @@ import { ApplicationRegistrationResolver } from 'src/engine/core-modules/applica
|
||||
import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service';
|
||||
import { ApplicationRegistrationVariableModule } from 'src/engine/core-modules/application/application-registration-variable/application-registration-variable.module';
|
||||
import { ApplicationTarballService } from 'src/engine/core-modules/application/application-registration/application-tarball.service';
|
||||
import { ManifestAssetUrlResolverService } from 'src/engine/core-modules/application/application-registration/manifest-asset-url-resolver.service';
|
||||
import { ApplicationPackageModule } from 'src/engine/core-modules/application/application-package/application-package.module';
|
||||
import { ApplicationEntity } from 'src/engine/core-modules/application/application.entity';
|
||||
import { ApplicationModule } from 'src/engine/core-modules/application/application.module';
|
||||
import { CacheLockModule } from 'src/engine/core-modules/cache-lock/cache-lock.module';
|
||||
import { DomainServerConfigModule } from 'src/engine/core-modules/domain/domain-server-config/domain-server-config.module';
|
||||
import { FeatureFlagModule } from 'src/engine/core-modules/feature-flag/feature-flag.module';
|
||||
import { FileStorageModule } from 'src/engine/core-modules/file-storage/file-storage.module';
|
||||
@@ -27,6 +29,7 @@ import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/
|
||||
ApplicationRegistrationVariableModule,
|
||||
ApplicationModule,
|
||||
ApplicationPackageModule,
|
||||
CacheLockModule,
|
||||
DomainServerConfigModule,
|
||||
FeatureFlagModule,
|
||||
PermissionsModule,
|
||||
@@ -38,10 +41,12 @@ import { WorkspaceCacheStorageModule } from 'src/engine/workspace-cache-storage/
|
||||
ApplicationRegistrationService,
|
||||
ApplicationRegistrationResolver,
|
||||
ApplicationTarballService,
|
||||
ManifestAssetUrlResolverService,
|
||||
],
|
||||
exports: [
|
||||
ApplicationRegistrationService,
|
||||
ApplicationRegistrationVariableModule,
|
||||
ManifestAssetUrlResolverService,
|
||||
],
|
||||
})
|
||||
export class ApplicationRegistrationModule {}
|
||||
|
||||
+41
-11
@@ -1,4 +1,4 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
import { Injectable, Logger } from '@nestjs/common';
|
||||
import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import crypto from 'crypto';
|
||||
@@ -10,6 +10,8 @@ import { ILike, type FindOptionsWhere, type Repository } from 'typeorm';
|
||||
import { v4 } from 'uuid';
|
||||
|
||||
import { ALL_OAUTH_SCOPES } from 'src/engine/core-modules/application/application-oauth/constants/oauth-scopes';
|
||||
import { shouldRefreshApplicationRegistrationOnInstall } from 'src/engine/core-modules/application/application-install/utils/should-refresh-application-registration-on-install.util';
|
||||
import { CacheLockService } from 'src/engine/core-modules/cache-lock/cache-lock.service';
|
||||
import { ApplicationRegistrationEntity } from 'src/engine/core-modules/application/application-registration/application-registration.entity';
|
||||
import { TWENTY_CLI_APPLICATION_REGISTRATION } from 'src/engine/workspace-manager/twenty-standard-application/constants/twenty-cli-application-registration.constant';
|
||||
import {
|
||||
@@ -79,6 +81,8 @@ export type ApplicationRegistrationCatalogCard = {
|
||||
|
||||
@Injectable()
|
||||
export class ApplicationRegistrationService {
|
||||
private readonly logger = new Logger(ApplicationRegistrationService.name);
|
||||
|
||||
constructor(
|
||||
@InjectRepository(ApplicationRegistrationEntity)
|
||||
private readonly applicationRegistrationRepository: Repository<ApplicationRegistrationEntity>,
|
||||
@@ -87,6 +91,7 @@ export class ApplicationRegistrationService {
|
||||
@InjectRepository(WorkspaceEntity)
|
||||
private readonly workspaceRepository: Repository<WorkspaceEntity>,
|
||||
private readonly applicationRegistrationVariableService: ApplicationRegistrationVariableService,
|
||||
private readonly cacheLockService: CacheLockService,
|
||||
) {}
|
||||
|
||||
async findMany(
|
||||
@@ -286,22 +291,47 @@ export class ApplicationRegistrationService {
|
||||
applicationRegistrationId,
|
||||
manifest,
|
||||
sourceType,
|
||||
latestAvailableVersion,
|
||||
preventVersionDowngrade = false,
|
||||
}: {
|
||||
applicationRegistrationId: string;
|
||||
manifest: Manifest;
|
||||
sourceType?: ApplicationRegistrationSourceType;
|
||||
latestAvailableVersion?: string;
|
||||
preventVersionDowngrade?: boolean;
|
||||
}): Promise<void> {
|
||||
const existing = await this.applicationRegistrationRepository.findOneOrFail(
|
||||
{ where: { id: applicationRegistrationId } },
|
||||
);
|
||||
await this.cacheLockService.withLock(async () => {
|
||||
const existing =
|
||||
await this.applicationRegistrationRepository.findOneOrFail({
|
||||
where: { id: applicationRegistrationId },
|
||||
});
|
||||
|
||||
await this.applicationRegistrationRepository.save({
|
||||
...existing,
|
||||
name: manifest.application.displayName,
|
||||
manifest,
|
||||
...fromManifestApplicationToDisplayFields(manifest.application),
|
||||
...(sourceType !== undefined && { sourceType }),
|
||||
});
|
||||
if (
|
||||
preventVersionDowngrade &&
|
||||
isDefined(latestAvailableVersion) &&
|
||||
!shouldRefreshApplicationRegistrationOnInstall({
|
||||
installedVersion: latestAvailableVersion,
|
||||
latestAvailableVersion: existing.latestAvailableVersion,
|
||||
})
|
||||
) {
|
||||
this.logger.log(
|
||||
`Skipping registration update for ${existing.universalIdentifier}: version ${latestAvailableVersion} is older than latest available version ${existing.latestAvailableVersion}`,
|
||||
);
|
||||
|
||||
return;
|
||||
}
|
||||
|
||||
await this.applicationRegistrationRepository.save({
|
||||
...existing,
|
||||
name: manifest.application.displayName,
|
||||
manifest,
|
||||
...fromManifestApplicationToDisplayFields(manifest.application),
|
||||
...(sourceType !== undefined && { sourceType }),
|
||||
...(latestAvailableVersion !== undefined && {
|
||||
latestAvailableVersion,
|
||||
}),
|
||||
});
|
||||
}, `application-registration-update:${applicationRegistrationId}`);
|
||||
}
|
||||
|
||||
async delete(id: string, ownerWorkspaceId: string): Promise<boolean> {
|
||||
|
||||
+55
@@ -0,0 +1,55 @@
|
||||
import { Injectable } from '@nestjs/common';
|
||||
|
||||
import { type Manifest } from 'twenty-shared/application';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
|
||||
import { buildRegistryCdnUrl } from 'src/engine/core-modules/application/application-marketplace/utils/build-registry-cdn-url.util';
|
||||
import { resolveManifestAssetUrls } from 'src/engine/core-modules/application/application-marketplace/utils/resolve-manifest-asset-urls.util';
|
||||
import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum';
|
||||
import { TwentyConfigService } from 'src/engine/core-modules/twenty-config/twenty-config.service';
|
||||
|
||||
@Injectable()
|
||||
export class ManifestAssetUrlResolverService {
|
||||
constructor(private readonly twentyConfigService: TwentyConfigService) {}
|
||||
|
||||
resolveFromRegistrySource({
|
||||
manifest,
|
||||
packageName,
|
||||
version,
|
||||
}: {
|
||||
manifest: Manifest;
|
||||
packageName: string;
|
||||
version: string;
|
||||
}): Manifest {
|
||||
const cdnBaseUrl = this.twentyConfigService.get('APP_REGISTRY_CDN_URL');
|
||||
|
||||
return resolveManifestAssetUrls(manifest, (filePath) =>
|
||||
buildRegistryCdnUrl({ cdnBaseUrl, packageName, version, filePath }),
|
||||
);
|
||||
}
|
||||
|
||||
resolveFromRegistration({
|
||||
sourceType,
|
||||
sourcePackage,
|
||||
manifest,
|
||||
version,
|
||||
}: {
|
||||
sourceType: ApplicationRegistrationSourceType;
|
||||
sourcePackage: string | null;
|
||||
manifest: Manifest;
|
||||
version: string;
|
||||
}): Manifest {
|
||||
if (
|
||||
sourceType !== ApplicationRegistrationSourceType.NPM ||
|
||||
!isDefined(sourcePackage)
|
||||
) {
|
||||
return manifest;
|
||||
}
|
||||
|
||||
return this.resolveFromRegistrySource({
|
||||
manifest,
|
||||
packageName: sourcePackage,
|
||||
version,
|
||||
});
|
||||
}
|
||||
}
|
||||
Reference in New Issue
Block a user