-
Notifications
You must be signed in to change notification settings - Fork 118
feat(condo): DOMA-13081 field type for store data externally #7412
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
Changes from all commits
399df29
85765ab
1ea42ea
7b370c8
dcc6f1a
455f0d6
4de0116
8d0ea8e
de5d904
29953a6
4ffc842
6a74af7
81cdcaf
6a1e8ba
659587e
5e0af20
13a3ee3
381ac2a
0912814
b813904
78b520d
693aa37
f669ceb
292e089
0e0978d
5607d2e
1c5a275
c1e8fd7
060e230
adf0315
cd84cd4
d198362
e67604e
b55fd6c
9a9873f
92cd97d
8a31cb4
0215003
1accb98
7beef92
13a929e
344b13f
22b1bb8
ccebead
486ec5e
2c6291c
cb72eaa
4f7f510
b2bfcf3
d1bd49f
6c04530
4605bf4
e5136bd
7164ba7
19cab0a
12ed029
3171110
3ee0d53
015b0c7
b42534f
9c076b8
a2c10b8
4dee816
371b672
f59e73e
8cf01a6
aa35249
eca8225
8153067
49544e3
02ea04a
c6f4b2e
08dd98a
5a21ad9
d554b89
0df6c8c
966b6e3
09fedc8
950a9c1
bd6294c
bf1bbed
b2d3084
15b37a0
9708cbe
93e0315
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change | ||||||||||||||||||||||||||||||||
|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|---|
| @@ -0,0 +1,299 @@ | ||||||||||||||||||||||||||||||||||
| const { randomUUID } = require('node:crypto') | ||||||||||||||||||||||||||||||||||
| const { Readable } = require('node:stream') | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const { Implementation } = require('@open-keystone/fields') | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const { ExternalContent } = require('@open-condo/keystone/fieldsUtils') | ||||||||||||||||||||||||||||||||||
| const { getLogger } = require('@open-condo/keystone/logging') | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const logger = getLogger() | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const { EXTERNAL_CONTENT_FIELD_TYPE_META, DEFAULT_PROCESSORS, isFileMeta, resolveExternalContentValue } = ExternalContent | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const DEFAULT_MAX_SIZE_BYTES = 10 * 1024 * 1024 // Default max size: 10MB | ||||||||||||||||||||||||||||||||||
|
Member
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Why not 100mb as "max size" that could be uploaded to platform?
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. The content of the file is loaded into memory. So, I set the default maxSize less than on the platform. |
||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * ExternalContent field implementation for Keystone. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Stores large data externally in files rather than directly in the database. | ||||||||||||||||||||||||||||||||||
| * Provides transparent access to file content through GraphQL with DataLoader batching. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @typedef {import('@open-condo/keystone/fieldsUtils/ExternalContent/defaultProcessors').ExternalContentProcessor} ExternalContentProcessor | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @extends {import('@keystonejs/fields').Implementation} | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| class ExternalContentImplementation extends Implementation { | ||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Creates an ExternalContent field implementation. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @param {string} path - Field path (e.g., 'raw', 'metadata') | ||||||||||||||||||||||||||||||||||
| * @param {Object} options - Configuration options | ||||||||||||||||||||||||||||||||||
| * @param {Object} options.adapter - File adapter instance (required) | ||||||||||||||||||||||||||||||||||
| * @param {string} [options.format='json'] - Data format ('json', 'xml', or 'text') | ||||||||||||||||||||||||||||||||||
| * @param {Object.<string, ExternalContentProcessor>} [options.processors={}] - Custom format processors | ||||||||||||||||||||||||||||||||||
| * @param {number} [options.maxSizeBytes=10485760] - Maximum payload size in bytes (default: 10MB) | ||||||||||||||||||||||||||||||||||
| * @param {number} [options.batchDelayMs] - DataLoader batch delay in milliseconds | ||||||||||||||||||||||||||||||||||
| * @param {string} [options.schemaDoc] - Field description for schema | ||||||||||||||||||||||||||||||||||
| * @param {string} [options.graphQLAdminFragment=''] - GraphQL fragment for admin UI | ||||||||||||||||||||||||||||||||||
| * @param {Object} [meta] - Keystone field metadata | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @throws {Error} If adapter is not provided | ||||||||||||||||||||||||||||||||||
| * @throws {Error} If format is unknown | ||||||||||||||||||||||||||||||||||
| * @throws {Error} If adapter is not properly configured | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| constructor (path, options = {}, meta = {}) { | ||||||||||||||||||||||||||||||||||
| const { | ||||||||||||||||||||||||||||||||||
| adapter, | ||||||||||||||||||||||||||||||||||
| format = 'json', | ||||||||||||||||||||||||||||||||||
| processors = {}, | ||||||||||||||||||||||||||||||||||
| } = options | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Compute processor config | ||||||||||||||||||||||||||||||||||
| const byFormat = { ...DEFAULT_PROCESSORS, ...processors } | ||||||||||||||||||||||||||||||||||
| const cfg = byFormat[format] | ||||||||||||||||||||||||||||||||||
| if (!cfg) { | ||||||||||||||||||||||||||||||||||
| throw new Error(`ExternalContent: unknown format "${format}" for ${path}`) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (!adapter) { | ||||||||||||||||||||||||||||||||||
| throw new Error(`ExternalContent: "adapter" is required for ${path}`) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Validate adapter is properly configured (not NoFileAdapter) | ||||||||||||||||||||||||||||||||||
| if (adapter.constructor.name === 'NoFileAdapter' || adapter.error?.message?.includes('NoFileAdapter')) { | ||||||||||||||||||||||||||||||||||
| throw new Error(`ExternalContent: adapter is not properly configured for ${path}. Check FILE_FIELD_ADAPTER and storage configuration.`) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| super(path, options, meta) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| this.config.format ??= 'json' | ||||||||||||||||||||||||||||||||||
| this.config.maxSizeBytes ??= DEFAULT_MAX_SIZE_BYTES | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| this.adapter = adapter | ||||||||||||||||||||||||||||||||||
|
YEgorLu marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||
| this.graphQLInputType = cfg.graphQLInputType | ||||||||||||||||||||||||||||||||||
| this.graphQLReturnType = cfg.graphQLReturnType | ||||||||||||||||||||||||||||||||||
| this.mimetype = cfg.mimetype | ||||||||||||||||||||||||||||||||||
| this.fileExt = cfg.fileExt | ||||||||||||||||||||||||||||||||||
| this.formatProcessors = byFormat | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // GQL Output | ||||||||||||||||||||||||||||||||||
| gqlOutputFields () { | ||||||||||||||||||||||||||||||||||
| // Return both raw metadata field and virtual deserialized content field | ||||||||||||||||||||||||||||||||||
| return [ | ||||||||||||||||||||||||||||||||||
| `${this.path}: String`, // Raw file metadata (JSON string) for admin UI | ||||||||||||||||||||||||||||||||||
| `${this.path}Resolved: ${this.graphQLReturnType}`, // Deserialized content for API clients | ||||||||||||||||||||||||||||||||||
| ] | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Admin | ||||||||||||||||||||||||||||||||||
| extendAdminMeta (meta) { | ||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||
| ...meta, | ||||||||||||||||||||||||||||||||||
| graphQLAdminFragment: this.config.graphQLAdminFragment || '', | ||||||||||||||||||||||||||||||||||
| format: this.config.format, | ||||||||||||||||||||||||||||||||||
| processors: this.formatProcessors, | ||||||||||||||||||||||||||||||||||
| adminConfig: this.adminConfig || {}, | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+90
to
+97
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chain🏁 Script executed: #!/bin/bash
# Compare admin-meta builders and how they propagate adminConfig.
rg -n -C2 "extendAdminMeta|this\.config\.adminConfig|this\.adminConfig|adminConfig:" packages/keystone/fieldsRepository: open-condo-software/condo Length of output: 3285 🏁 Script executed: cat -n packages/keystone/fields/ExternalContent/Implementation.js | head -150Repository: open-condo-software/condo Length of output: 6852 🏁 Script executed: rg -n "constructor|this\.adminConfig\s*=" packages/keystone/fields/ExternalContent/Implementation.jsRepository: open-condo-software/condo Length of output: 237 🏁 Script executed: rg -n "adminConfig:" packages/keystone/fields -A1 -B1 | head -60Repository: open-condo-software/condo Length of output: 972 🏁 Script executed: rg -n "adminConfig" packages/keystone/fields/ExternalContent/views -A2 -B2Repository: open-condo-software/condo Length of output: 1183 🏁 Script executed: rg -n "field\.config\.adminConfig|adminConfig\.isReadOnly" packages/keystone/fields -lRepository: open-condo-software/condo Length of output: 51 🏁 Script executed: rg -n "this\.adminConfig\s*=" packages/keystone/fields/ExternalContentRepository: open-condo-software/condo Length of output: 51 🏁 Script executed: rg -n "Object\.defineProperty.*adminConfig" packages/keystone/fields/ExternalContentRepository: open-condo-software/condo Length of output: 51 Pass
Suggested change return {
...meta,
graphQLAdminFragment: this.config.graphQLAdminFragment || '',
format: this.config.format,
processors: this.formatProcessors,
- adminConfig: this.adminConfig || {},
+ adminConfig: this.config.adminConfig || {},
}📝 Committable suggestion
Suggested change
🤖 Prompt for AI Agents
Member
Author
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nope! See node_modules/@open-keystone/fields/dist/fields.cjs.dev.js:64
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. 🧩 Analysis chainLet me check the parent 🏁 Script executed: Length of output: 100 🏁 Script executed: Length of output: 0 🏁 Script executed: Length of output: 1048 🏁 Script executed: Length of output: 995 🏁 Script executed: Length of output: 757 🏁 Script executed: Length of output: 0
✏️ Learnings added
🧠 Learnings used |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Provides both raw file metadata and deserialized content via GraphQL. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * - `fieldName` - Returns raw file metadata (JSON string from DB) for admin UI | ||||||||||||||||||||||||||||||||||
| * - `fieldNameResolved` - Virtual field that returns deserialized content for API clients | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @returns {Object} Field resolver mapping | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| gqlOutputFieldResolvers () { | ||||||||||||||||||||||||||||||||||
| return { | ||||||||||||||||||||||||||||||||||
| // Virtual field for deserialized content (for API clients) | ||||||||||||||||||||||||||||||||||
| [`${this.path}Resolved`]: async (item, args, context) => { | ||||||||||||||||||||||||||||||||||
| const value = item?.[this.path] | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| return await resolveExternalContentValue(value, { | ||||||||||||||||||||||||||||||||||
| adapter: this.adapter, | ||||||||||||||||||||||||||||||||||
| formatProcessors: this.formatProcessors, | ||||||||||||||||||||||||||||||||||
| context, | ||||||||||||||||||||||||||||||||||
| batchDelayMs: this.config.batchDelayMs, | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||
| const itemId = item?.id || 'unknown' | ||||||||||||||||||||||||||||||||||
| logger.warn({ | ||||||||||||||||||||||||||||||||||
| msg: 'Error resolving ExternalContent field', | ||||||||||||||||||||||||||||||||||
| entity: this.listKey, | ||||||||||||||||||||||||||||||||||
| entityId: itemId, | ||||||||||||||||||||||||||||||||||
| err, | ||||||||||||||||||||||||||||||||||
| data: { path: this.path }, | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| throw err | ||||||||||||||||||||||||||||||||||
|
coderabbitai[bot] marked this conversation as resolved.
|
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| }, | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // GQL Input | ||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Returns query input fields for filtering. | ||||||||||||||||||||||||||||||||||
| * We store only file references in DB, so querying by content is not supported. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @returns {Array} Empty array - no query filters available | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| gqlQueryInputFields () { | ||||||||||||||||||||||||||||||||||
| return [] | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Returns GraphQL input fields for update mutations. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @returns {Array<string>} Field definitions for GraphQL schema | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| gqlUpdateInputFields () { | ||||||||||||||||||||||||||||||||||
| return [`${this.path}: ${this.graphQLInputType}`] | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Returns GraphQL input fields for create mutations. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @returns {Array<string>} Field definitions for GraphQL schema | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| gqlCreateInputFields () { | ||||||||||||||||||||||||||||||||||
| return [`${this.path}: ${this.graphQLInputType}`] | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Hooks | ||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Validates payload size against configured maximum. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @private | ||||||||||||||||||||||||||||||||||
| * @param {number} sizeBytes - Size in bytes to validate | ||||||||||||||||||||||||||||||||||
| * @throws {Error} If size exceeds maxSizeBytes | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| _validatePayloadSize (sizeBytes) { | ||||||||||||||||||||||||||||||||||
| if (sizeBytes > this.config.maxSizeBytes) { | ||||||||||||||||||||||||||||||||||
| const sizeMB = (sizeBytes / 1024 / 1024).toFixed(2) | ||||||||||||||||||||||||||||||||||
| const maxMB = (this.config.maxSizeBytes / 1024 / 1024).toFixed(2) | ||||||||||||||||||||||||||||||||||
| throw new Error(`ExternalContent: payload size (${sizeMB}MB) exceeds maximum allowed size (${maxMB}MB) for field ${this.path}`) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Resolves input data before saving to database. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Handles file lifecycle: | ||||||||||||||||||||||||||||||||||
| * 1. If setting to null: returns null (old file deleted in afterChange) | ||||||||||||||||||||||||||||||||||
| * 2. If setting new value: saves new file to storage, returns file-meta (old file deleted in afterChange) | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Known limitation: If adapter.save() succeeds but the DB write fails, the new file will be | ||||||||||||||||||||||||||||||||||
| * orphaned in storage. This is acceptable — old file is never deleted until afterChange confirms | ||||||||||||||||||||||||||||||||||
| * the DB write succeeded. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @param {Object} params | ||||||||||||||||||||||||||||||||||
| * @param {Object} params.resolvedData - New field values | ||||||||||||||||||||||||||||||||||
| * @param {Object} params.existingItem - Current item from database | ||||||||||||||||||||||||||||||||||
| * @param {string} params.listKey - Name of the list/model | ||||||||||||||||||||||||||||||||||
| * @param {Object} [params.context] - Keystone context (used for authed user when generating public URLs) | ||||||||||||||||||||||||||||||||||
| * @returns {Promise<Object|null|undefined>} File-meta object to store in DB | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| async resolveInput ({ resolvedData, existingItem, listKey, context }) { | ||||||||||||||||||||||||||||||||||
| const nextValue = resolvedData[this.path] | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (nextValue === undefined || nextValue === null) return nextValue | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const processor = this.formatProcessors[this.config.format] | ||||||||||||||||||||||||||||||||||
| const payload = processor.serialize(nextValue) | ||||||||||||||||||||||||||||||||||
| const payloadSizeBytes = Buffer.byteLength(String(payload), 'utf-8') | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| this._validatePayloadSize(payloadSizeBytes) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const stream = Readable.from([Buffer.from(String(payload), 'utf-8')]) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const prefix = listKey || 'item' | ||||||||||||||||||||||||||||||||||
| const id = randomUUID() | ||||||||||||||||||||||||||||||||||
| const originalFilename = `${prefix}_${this.path}_${id}.${this.fileExt}` | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Save minimal metadata for OBSFilesMiddleware to identify and authorize access | ||||||||||||||||||||||||||||||||||
| // id is not available yet (generated by DB), will be set in afterChange | ||||||||||||||||||||||||||||||||||
| const fileMeta = { | ||||||||||||||||||||||||||||||||||
| listkey: listKey, | ||||||||||||||||||||||||||||||||||
| mimetype: this.mimetype, | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const saved = await this.adapter.save({ | ||||||||||||||||||||||||||||||||||
| stream, | ||||||||||||||||||||||||||||||||||
| filename: originalFilename, | ||||||||||||||||||||||||||||||||||
| mimetype: this.mimetype, | ||||||||||||||||||||||||||||||||||
| encoding: 'utf-8', | ||||||||||||||||||||||||||||||||||
| id, | ||||||||||||||||||||||||||||||||||
| meta: fileMeta, | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Generate publicUrl using adapter's method | ||||||||||||||||||||||||||||||||||
| // This ensures consistent URL generation with other file fields | ||||||||||||||||||||||||||||||||||
| // For cloud adapters, this returns middleware URL (not direct signed URL) so links don't expire | ||||||||||||||||||||||||||||||||||
| const publicUrl = this.adapter.publicUrl({ | ||||||||||||||||||||||||||||||||||
| filename: originalFilename, | ||||||||||||||||||||||||||||||||||
| originalFilename, | ||||||||||||||||||||||||||||||||||
| id, | ||||||||||||||||||||||||||||||||||
| meta: fileMeta, | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| return { ...saved, publicUrl, [EXTERNAL_CONTENT_FIELD_TYPE_META]: { format: this.config.format } } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| /** | ||||||||||||||||||||||||||||||||||
| * Deletes the old file after DB write succeeds. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * Called by Keystone after the item is saved to the database. | ||||||||||||||||||||||||||||||||||
| * Deleting here ensures the old file is only removed | ||||||||||||||||||||||||||||||||||
| * once the new value is durably committed — preventing data loss on DB failure. | ||||||||||||||||||||||||||||||||||
| * | ||||||||||||||||||||||||||||||||||
| * @param {Object} params | ||||||||||||||||||||||||||||||||||
| * @param {Object} params.existingItem - Item state before the update | ||||||||||||||||||||||||||||||||||
| * @param {Object} params.updatedItem - Item state after the update | ||||||||||||||||||||||||||||||||||
| */ | ||||||||||||||||||||||||||||||||||
| async afterChange ({ existingItem, updatedItem, listKey }) { | ||||||||||||||||||||||||||||||||||
| const parseDbValue = (v) => { | ||||||||||||||||||||||||||||||||||
| if (typeof v !== 'string') return v | ||||||||||||||||||||||||||||||||||
| try { return JSON.parse(v) } catch { return null } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| const prevValue = parseDbValue(existingItem?.[this.path]) | ||||||||||||||||||||||||||||||||||
| const nextValue = parseDbValue(updatedItem?.[this.path]) | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Update file metadata with actual item id for new files | ||||||||||||||||||||||||||||||||||
| // This allows OBSFilesMiddleware to verify access control | ||||||||||||||||||||||||||||||||||
| const prevFilename = isFileMeta(prevValue) ? prevValue.filename : null | ||||||||||||||||||||||||||||||||||
| const nextFilename = isFileMeta(nextValue) ? nextValue.filename : null | ||||||||||||||||||||||||||||||||||
|
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more.
It seems I was mistaken. For some reason I read all "if"s backwards |
||||||||||||||||||||||||||||||||||
| if (nextFilename && prevFilename !== nextFilename && this.adapter.acl?.setMeta) { | ||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| const folder = this.adapter.folder || '' | ||||||||||||||||||||||||||||||||||
| await this.adapter.acl.setMeta(`${folder}/${nextFilename}`, { | ||||||||||||||||||||||||||||||||||
| listkey: listKey, | ||||||||||||||||||||||||||||||||||
| id: updatedItem.id, | ||||||||||||||||||||||||||||||||||
| mimetype: this.mimetype, | ||||||||||||||||||||||||||||||||||
| }) | ||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||
| logger.warn({ msg: 'Failed to update file metadata with item id', err, data: { nextValue, itemId: updatedItem.id } }) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| if (!isFileMeta(prevValue)) return | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| // Old file should be deleted if field was cleared or replaced with a new file | ||||||||||||||||||||||||||||||||||
| if (prevFilename === nextFilename) return | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| try { | ||||||||||||||||||||||||||||||||||
| await this.adapter.delete(prevValue) | ||||||||||||||||||||||||||||||||||
| } catch (err) { | ||||||||||||||||||||||||||||||||||
| logger.warn({ msg: 'Failed to delete old file after change', err, data: { prevValue } }) | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
Comment on lines
+287
to
+291
Contributor
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Don’t log the full file metadata on delete failures.
Suggested change try {
await this.adapter.delete(prevValue)
} catch (err) {
- logger.warn({ msg: 'Failed to delete old file after change', err, data: { prevValue } })
+ logger.warn({
+ msg: 'Failed to delete old file after change',
+ entity: this.listKey,
+ entityId: updatedItem?.id || existingItem?.id || 'unknown',
+ err,
+ data: {
+ field: this.path,
+ filename: prevValue.filename,
+ },
+ })
}As per coding guidelines, "Use standard logging fields: msg, data, entityId, entity, count, status, and err instead of custom field names". 🤖 Prompt for AI Agents |
||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
| module.exports = { | ||||||||||||||||||||||||||||||||||
| ExternalContentImplementation, | ||||||||||||||||||||||||||||||||||
| DEFAULT_PROCESSORS, | ||||||||||||||||||||||||||||||||||
| } | ||||||||||||||||||||||||||||||||||
|
|
||||||||||||||||||||||||||||||||||
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
It'll be great to have a tool for migration from "old format" to new one
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Yes, it exists. See apps/rb/bin/backfillExternalContentField.js.