Improve application asset management (#22564)
App manifests could point the logo and screenshots at either external
URLs or public folder paths, and that was handled inconsistently across
install, sync and the marketplace.
This makes assets always bundled files:
- Manifests now use `logo` and `galleryImages` (a `string[]` of public
folder paths) instead of `logoUrl` and `screenshots`. The old fields
still work but are deprecated. Gallery order comes from the array index.
Normalization (deprecated-field migration, and warning about + ignoring
external URLs) happens in `defineApplication`, so the warnings surface
at define time.
- Logo is stored as a File record (`logoFileId`).
- The registration gallery is configured via a `settings` jsonb column
on `applicationRegistration` (`{ galleryImages: string[] }`) — populated
from the manifest, read by the marketplace detail (falling back to the
legacy `screenshots` column, then the manifest). No dedicated gallery
table.
- The marketplace detail DTO and front now use `galleryImages`.
Verified against a local Postgres: the fast instance commands run with
no pending-migration diff, the schema is correct, and the server boots.
Typecheck, lint, codegen and the application unit tests pass.
Not included yet: rehosting assets into storage for npm catalog and
tarball registrations, versioned cache busting on the serving route, and
a backfill for existing installs.
<a
href="https://cubic.dev/pr/twentyhq/twenty/pull/22564?utm_source=github"
rel="nofollow noreferrer noopener" target="_blank">``<img alt="Review
in cubic"
src="https://www.cubic.dev/buttons/review-in-cubic-dark.svg">``</a>
This commit is contained in:
+16
-7
@@ -14,6 +14,7 @@ import {
|
||||
ApplicationException,
|
||||
ApplicationExceptionCode,
|
||||
} from 'src/engine/core-modules/application/application.exception';
|
||||
import { isImageFilePath } from 'src/engine/core-modules/application/application-registration/utils/is-image-file-path.util';
|
||||
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';
|
||||
@@ -559,17 +560,25 @@ export class ApplicationInstallService {
|
||||
applicationUniversalIdentifier: string;
|
||||
workspaceId: string;
|
||||
}): Promise<string | null> {
|
||||
const logoUrl = manifest.application.logoUrl;
|
||||
const logo = manifest.application.logo ?? manifest.application.logoUrl;
|
||||
|
||||
if (
|
||||
!isDefined(logoUrl) ||
|
||||
logoUrl.startsWith('http://') ||
|
||||
logoUrl.startsWith('https://')
|
||||
!isDefined(logo) ||
|
||||
logo.startsWith('http://') ||
|
||||
logo.startsWith('https://')
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const absolutePath = this.resolveWithinDirOrThrow(extractedDir, logoUrl);
|
||||
if (!isImageFilePath(logo)) {
|
||||
this.logger.warn(
|
||||
`Logo "${logo}" is not a supported image type; skipping logo import for ${applicationUniversalIdentifier}`,
|
||||
);
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
const absolutePath = this.resolveWithinDirOrThrow(extractedDir, logo);
|
||||
|
||||
let content: Buffer;
|
||||
|
||||
@@ -577,7 +586,7 @@ export class ApplicationInstallService {
|
||||
content = await fs.readFile(absolutePath);
|
||||
} catch {
|
||||
this.logger.warn(
|
||||
`Logo "${logoUrl}" declared in manifest but not found in package for ${applicationUniversalIdentifier}; skipping logo import`,
|
||||
`Logo "${logo}" declared in manifest but not found in package for ${applicationUniversalIdentifier}; skipping logo import`,
|
||||
);
|
||||
|
||||
return null;
|
||||
@@ -588,7 +597,7 @@ export class ApplicationInstallService {
|
||||
fileFolder: FileFolder.PublicAsset,
|
||||
applicationUniversalIdentifier,
|
||||
workspaceId,
|
||||
resourcePath: logoUrl,
|
||||
resourcePath: logo,
|
||||
settings: { isTemporaryFile: false, toDelete: false },
|
||||
});
|
||||
|
||||
|
||||
+6
-1
@@ -90,9 +90,14 @@ export class MarketplaceAppDetailDTO {
|
||||
@Field({ nullable: true })
|
||||
issueReportUrl?: string;
|
||||
|
||||
@Field(() => [String])
|
||||
@Field(() => [String], {
|
||||
deprecationReason: 'Use galleryImages instead',
|
||||
})
|
||||
screenshots: string[];
|
||||
|
||||
@Field(() => [String])
|
||||
galleryImages: string[];
|
||||
|
||||
@IsOptional()
|
||||
@IsString()
|
||||
@Field({ nullable: true })
|
||||
|
||||
+11
-3
@@ -13,6 +13,7 @@ import {
|
||||
ApplicationRegistrationException,
|
||||
ApplicationRegistrationExceptionCode,
|
||||
} from 'src/engine/core-modules/application/application-registration/application-registration.exception';
|
||||
import { toGalleryImagePaths } from 'src/engine/core-modules/application/application-registration/utils/to-gallery-image-paths.util';
|
||||
import { ApplicationRegistrationService } from 'src/engine/core-modules/application/application-registration/application-registration.service';
|
||||
|
||||
@Injectable()
|
||||
@@ -81,6 +82,14 @@ export class MarketplaceQueryService {
|
||||
private toMarketplaceAppDetailDTO(
|
||||
registration: ApplicationRegistrationEntity,
|
||||
): MarketplaceAppDetailDTO {
|
||||
// TODO: simplify in a follow-up PR to read galleryImages only, once the
|
||||
// deprecated screenshots column and manifest fallback are backfilled away.
|
||||
const galleryImagePaths = isNonEmptyArray(registration.galleryImages)
|
||||
? registration.galleryImages.map((galleryImage) => galleryImage.path)
|
||||
: isNonEmptyArray(registration.screenshots)
|
||||
? registration.screenshots
|
||||
: toGalleryImagePaths(registration.manifest?.application);
|
||||
|
||||
return {
|
||||
id: registration.id,
|
||||
universalIdentifier: registration.universalIdentifier,
|
||||
@@ -123,9 +132,8 @@ export class MarketplaceQueryService {
|
||||
registration.issueReportUrl ??
|
||||
registration.manifest?.application?.issueReportUrl ??
|
||||
undefined,
|
||||
screenshots: isNonEmptyArray(registration.screenshots)
|
||||
? registration.screenshots
|
||||
: (registration.manifest?.application?.screenshots ?? []),
|
||||
screenshots: galleryImagePaths,
|
||||
galleryImages: galleryImagePaths,
|
||||
defaultRoleUniversalIdentifier:
|
||||
registration.manifest?.application?.defaultRoleUniversalIdentifier,
|
||||
roles: registration.manifest?.roles?.map((role) =>
|
||||
|
||||
+41
@@ -131,6 +131,47 @@ describe('resolveManifestAssetUrls', () => {
|
||||
expect(result.application.screenshots).toEqual([]);
|
||||
});
|
||||
|
||||
it('should resolve a relative logo', () => {
|
||||
const manifest = buildMinimalManifest({ logo: 'logo.png' });
|
||||
|
||||
const result = resolveManifestAssetUrls(manifest, urlBuilder);
|
||||
|
||||
expect(result.application.logo).toBe(
|
||||
'https://cdn.example.com/pkg/logo.png',
|
||||
);
|
||||
});
|
||||
|
||||
it('should not modify an absolute logo', () => {
|
||||
const manifest = buildMinimalManifest({
|
||||
logo: 'https://example.com/logo.png',
|
||||
});
|
||||
|
||||
const result = resolveManifestAssetUrls(manifest, urlBuilder);
|
||||
|
||||
expect(result.application.logo).toBe('https://example.com/logo.png');
|
||||
});
|
||||
|
||||
it('should resolve galleryImages paths and leave absolute ones untouched', () => {
|
||||
const manifest = buildMinimalManifest({
|
||||
galleryImages: ['a.png', 'https://example.com/b.png'],
|
||||
});
|
||||
|
||||
const result = resolveManifestAssetUrls(manifest, urlBuilder);
|
||||
|
||||
expect(result.application.galleryImages).toEqual([
|
||||
'https://cdn.example.com/pkg/a.png',
|
||||
'https://example.com/b.png',
|
||||
]);
|
||||
});
|
||||
|
||||
it('should handle undefined galleryImages', () => {
|
||||
const manifest = buildMinimalManifest();
|
||||
|
||||
const result = resolveManifestAssetUrls(manifest, urlBuilder);
|
||||
|
||||
expect(result.application.galleryImages).toEqual([]);
|
||||
});
|
||||
|
||||
it('should preserve all other manifest properties', () => {
|
||||
const manifest = buildMinimalManifest({
|
||||
logoUrl: 'logo.png',
|
||||
|
||||
+4
@@ -17,7 +17,11 @@ export const resolveManifestAssetUrls = (
|
||||
logoUrl: manifest.application.logoUrl
|
||||
? resolveUrl(manifest.application.logoUrl)
|
||||
: undefined,
|
||||
logo: manifest.application.logo
|
||||
? resolveUrl(manifest.application.logo)
|
||||
: undefined,
|
||||
screenshots: (manifest.application.screenshots ?? []).map(resolveUrl),
|
||||
galleryImages: (manifest.application.galleryImages ?? []).map(resolveUrl),
|
||||
},
|
||||
};
|
||||
};
|
||||
|
||||
+14
-1
@@ -20,6 +20,7 @@ import {
|
||||
import { UUIDScalarType } from 'src/engine/api/graphql/workspace-schema-builder/graphql-types/scalars';
|
||||
import { type Manifest } from 'twenty-shared/application';
|
||||
import { ApplicationRegistrationVariableEntity } from 'src/engine/core-modules/application/application-registration-variable/application-registration-variable.entity';
|
||||
import { type ApplicationRegistrationGalleryImage } from 'src/engine/core-modules/application/application-registration/types/application-registration-gallery-image.type';
|
||||
import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum';
|
||||
import { FileEntity } from 'src/engine/core-modules/file/entities/file.entity';
|
||||
import { WasIntroducedInUpgrade } from 'src/engine/core-modules/upgrade/decorators/was-introduced-in-upgrade.decorator';
|
||||
@@ -214,7 +215,12 @@ export class ApplicationRegistrationEntity {
|
||||
|
||||
@Field(() => String, { nullable: true })
|
||||
get logoUrl(): string | null {
|
||||
return this.logo ?? this.manifest?.application?.logoUrl ?? null;
|
||||
return (
|
||||
this.logo ??
|
||||
this.manifest?.application?.logo ??
|
||||
this.manifest?.application?.logoUrl ??
|
||||
null
|
||||
);
|
||||
}
|
||||
|
||||
@OneToMany(
|
||||
@@ -224,6 +230,13 @@ export class ApplicationRegistrationEntity {
|
||||
)
|
||||
variables: Relation<ApplicationRegistrationVariableEntity[]>;
|
||||
|
||||
@Column({ type: 'jsonb', nullable: true })
|
||||
@WasIntroducedInUpgrade({
|
||||
upgradeCommandName:
|
||||
'2.20.0_AddGalleryImagesToApplicationRegistrationFastInstanceCommand_1783615890055',
|
||||
})
|
||||
galleryImages: ApplicationRegistrationGalleryImage[] | null;
|
||||
|
||||
@Field()
|
||||
@CreateDateColumn({ type: 'timestamptz' })
|
||||
createdAt: Date;
|
||||
|
||||
+1
@@ -65,6 +65,7 @@ const APPLICATION_REGISTRATION_WITHOUT_MANIFEST_SELECT: (keyof ApplicationRegist
|
||||
'emailSupport',
|
||||
'issueReportUrl',
|
||||
'screenshots',
|
||||
'galleryImages',
|
||||
'createdAt',
|
||||
'updatedAt',
|
||||
];
|
||||
|
||||
+102
-2
@@ -3,10 +3,10 @@ import { InjectRepository } from '@nestjs/typeorm';
|
||||
|
||||
import { promises as fs } from 'fs';
|
||||
import { tmpdir } from 'os';
|
||||
import { join } from 'path';
|
||||
import { isAbsolute, join, relative, resolve } from 'path';
|
||||
|
||||
import semver from 'semver';
|
||||
import { FileFolder } from 'twenty-shared/types';
|
||||
import { FileFolder, ServerFileFolder } from 'twenty-shared/types';
|
||||
import { isDefined } from 'twenty-shared/utils';
|
||||
import { Repository } from 'typeorm';
|
||||
import { type QueryDeepPartialEntity } from 'typeorm/query-builder/QueryPartialEntity';
|
||||
@@ -24,9 +24,14 @@ import {
|
||||
ApplicationRegistrationExceptionCode,
|
||||
} from 'src/engine/core-modules/application/application-registration/application-registration.exception';
|
||||
import { ApplicationRegistrationSourceType } from 'src/engine/core-modules/application/application-registration/enums/application-registration-source-type.enum';
|
||||
import { type ApplicationRegistrationGalleryImage } from 'src/engine/core-modules/application/application-registration/types/application-registration-gallery-image.type';
|
||||
import { fromManifestApplicationToDisplayFields } from 'src/engine/core-modules/application/application-registration/utils/from-manifest-application-to-display-fields.util';
|
||||
import { isImageFilePath } from 'src/engine/core-modules/application/application-registration/utils/is-image-file-path.util';
|
||||
import { toGalleryImagePaths } from 'src/engine/core-modules/application/application-registration/utils/to-gallery-image-paths.util';
|
||||
import { ApplicationService } from 'src/engine/core-modules/application/application.service';
|
||||
import { FileStorageService } from 'src/engine/core-modules/file-storage/services/file-storage.service';
|
||||
import { ServerFileStorageService } from 'src/engine/core-modules/file-storage/services/server-file-storage.service';
|
||||
import { prepareFileForStorageOrThrow } from 'src/engine/core-modules/file-storage/utils/prepare-file-for-storage-or-throw.util';
|
||||
import type { ApplicationManifest } from 'twenty-shared/application';
|
||||
|
||||
@Injectable()
|
||||
@@ -37,6 +42,7 @@ export class ApplicationTarballService {
|
||||
@InjectRepository(ApplicationRegistrationEntity)
|
||||
private readonly appRegistrationRepository: Repository<ApplicationRegistrationEntity>,
|
||||
private readonly fileStorageService: FileStorageService,
|
||||
private readonly serverFileStorageService: ServerFileStorageService,
|
||||
private readonly applicationService: ApplicationService,
|
||||
private readonly applicationRegistrationVariableService: ApplicationRegistrationVariableService,
|
||||
private readonly applicationVersionValidationService: ApplicationVersionValidationService,
|
||||
@@ -202,6 +208,12 @@ export class ApplicationTarballService {
|
||||
ownerWorkspaceId: params.ownerWorkspaceId,
|
||||
} as QueryDeepPartialEntity<ApplicationRegistrationEntity>);
|
||||
|
||||
await this.storeGalleryImageFiles({
|
||||
applicationRegistrationId: appRegistration.id,
|
||||
manifestApplication: manifest.application,
|
||||
contentDir,
|
||||
});
|
||||
|
||||
if (manifest.application?.serverVariables) {
|
||||
await this.applicationRegistrationVariableService.syncVariableSchemas(
|
||||
appRegistration.id,
|
||||
@@ -220,4 +232,92 @@ export class ApplicationTarballService {
|
||||
await fs.rm(tempDir, { recursive: true, force: true });
|
||||
}
|
||||
}
|
||||
|
||||
private async storeGalleryImageFiles({
|
||||
applicationRegistrationId,
|
||||
manifestApplication,
|
||||
contentDir,
|
||||
}: {
|
||||
applicationRegistrationId: string;
|
||||
manifestApplication: ApplicationManifest | undefined;
|
||||
contentDir: string;
|
||||
}): Promise<void> {
|
||||
const galleryImages: ApplicationRegistrationGalleryImage[] = [];
|
||||
|
||||
for (const path of toGalleryImagePaths(manifestApplication)) {
|
||||
const fileId = await this.storeGalleryImageFile({
|
||||
applicationRegistrationId,
|
||||
contentDir,
|
||||
path,
|
||||
});
|
||||
|
||||
if (isDefined(fileId)) {
|
||||
galleryImages.push({ path, fileId });
|
||||
}
|
||||
}
|
||||
|
||||
await this.appRegistrationRepository.update(applicationRegistrationId, {
|
||||
galleryImages,
|
||||
} as QueryDeepPartialEntity<ApplicationRegistrationEntity>);
|
||||
}
|
||||
|
||||
private async storeGalleryImageFile({
|
||||
applicationRegistrationId,
|
||||
contentDir,
|
||||
path,
|
||||
}: {
|
||||
applicationRegistrationId: string;
|
||||
contentDir: string;
|
||||
path: string;
|
||||
}): Promise<string | null> {
|
||||
if (
|
||||
path.startsWith('http://') ||
|
||||
path.startsWith('https://') ||
|
||||
!isImageFilePath(path)
|
||||
) {
|
||||
return null;
|
||||
}
|
||||
|
||||
const absolutePath = resolve(contentDir, path);
|
||||
const relativeToContentDir = relative(contentDir, absolutePath);
|
||||
|
||||
if (
|
||||
relativeToContentDir === '..' ||
|
||||
relativeToContentDir.startsWith('../') ||
|
||||
isAbsolute(relativeToContentDir)
|
||||
) {
|
||||
this.logger.warn(
|
||||
`Gallery image "${path}" escapes the package directory; skipping`,
|
||||
);
|
||||
|
||||
return null;
|
||||
}
|
||||
|
||||
try {
|
||||
const contents = await fs.readFile(absolutePath);
|
||||
|
||||
const { sourceFile, mimeType } = await prepareFileForStorageOrThrow({
|
||||
sourceFile: contents,
|
||||
resourcePath: path,
|
||||
});
|
||||
|
||||
const savedFile = await this.serverFileStorageService.writeServerFile({
|
||||
fileFolder: ServerFileFolder.ApplicationRegistration,
|
||||
applicationRegistrationId,
|
||||
resourcePath: path,
|
||||
contents: Buffer.isBuffer(sourceFile)
|
||||
? sourceFile
|
||||
: Buffer.from(sourceFile),
|
||||
mimeType,
|
||||
});
|
||||
|
||||
return savedFile.id;
|
||||
} catch (error) {
|
||||
this.logger.warn(
|
||||
`Failed to store gallery image "${path}" for registration ${applicationRegistrationId}: ${error.message}`,
|
||||
);
|
||||
|
||||
return null;
|
||||
}
|
||||
}
|
||||
}
|
||||
|
||||
+4
@@ -0,0 +1,4 @@
|
||||
export type ApplicationRegistrationGalleryImage = {
|
||||
path: string;
|
||||
fileId: string | null;
|
||||
};
|
||||
+26
@@ -0,0 +1,26 @@
|
||||
import { isImageFilePath } from 'src/engine/core-modules/application/application-registration/utils/is-image-file-path.util';
|
||||
|
||||
describe('isImageFilePath', () => {
|
||||
it.each([
|
||||
'public/logo.png',
|
||||
'public/shot.JPG',
|
||||
'a/b/c.jpeg',
|
||||
'image.webp',
|
||||
'anim.gif',
|
||||
'icon.svg',
|
||||
'photo.avif',
|
||||
])('returns true for image path %s', (filePath) => {
|
||||
expect(isImageFilePath(filePath)).toBe(true);
|
||||
});
|
||||
|
||||
it.each([
|
||||
'public/data.json',
|
||||
'public/styles.css',
|
||||
'script.mjs',
|
||||
'README',
|
||||
'archive.tar.gz',
|
||||
'noextension',
|
||||
])('returns false for non-image path %s', (filePath) => {
|
||||
expect(isImageFilePath(filePath)).toBe(false);
|
||||
});
|
||||
});
|
||||
+7
-2
@@ -1,9 +1,12 @@
|
||||
import { type ApplicationManifest } from 'twenty-shared/application';
|
||||
|
||||
import { type ApplicationRegistrationGalleryImage } from 'src/engine/core-modules/application/application-registration/types/application-registration-gallery-image.type';
|
||||
import { toGalleryImagePaths } from 'src/engine/core-modules/application/application-registration/utils/to-gallery-image-paths.util';
|
||||
|
||||
export const fromManifestApplicationToDisplayFields = (
|
||||
application: ApplicationManifest | undefined,
|
||||
) => ({
|
||||
logo: application?.logoUrl ?? null,
|
||||
logo: application?.logo ?? application?.logoUrl ?? null,
|
||||
description: application?.description ?? null,
|
||||
author: application?.author ?? null,
|
||||
category: application?.category ?? null,
|
||||
@@ -12,5 +15,7 @@ export const fromManifestApplicationToDisplayFields = (
|
||||
termsUrl: application?.termsUrl ?? null,
|
||||
emailSupport: application?.emailSupport ?? null,
|
||||
issueReportUrl: application?.issueReportUrl ?? null,
|
||||
screenshots: application?.screenshots ?? [],
|
||||
galleryImages: toGalleryImagePaths(application).map(
|
||||
(path): ApplicationRegistrationGalleryImage => ({ path, fileId: null }),
|
||||
),
|
||||
});
|
||||
|
||||
+21
@@ -0,0 +1,21 @@
|
||||
const ALLOWED_IMAGE_EXTENSIONS = new Set([
|
||||
'.png',
|
||||
'.jpg',
|
||||
'.jpeg',
|
||||
'.webp',
|
||||
'.gif',
|
||||
'.svg',
|
||||
'.avif',
|
||||
]);
|
||||
|
||||
export const isImageFilePath = (filePath: string): boolean => {
|
||||
const lastDotIndex = filePath.lastIndexOf('.');
|
||||
|
||||
if (lastDotIndex === -1) {
|
||||
return false;
|
||||
}
|
||||
|
||||
return ALLOWED_IMAGE_EXTENSIONS.has(
|
||||
filePath.slice(lastDotIndex).toLowerCase(),
|
||||
);
|
||||
};
|
||||
+13
@@ -0,0 +1,13 @@
|
||||
import { type ApplicationManifest } from 'twenty-shared/application';
|
||||
|
||||
export const toGalleryImagePaths = (
|
||||
application: ApplicationManifest | undefined,
|
||||
): string[] => {
|
||||
const galleryImages = application?.galleryImages;
|
||||
|
||||
if (galleryImages && galleryImages.length > 0) {
|
||||
return galleryImages;
|
||||
}
|
||||
|
||||
return application?.screenshots ?? [];
|
||||
};
|
||||
Reference in New Issue
Block a user