feat: Allow File adapter to create file with specific locations or dynamic filenames

[AdrianCurtin]

trafficstars

Allow file adapter to override file name and location + give createFile access to config to allow getFileLocation calls internally

File adapter may specify a different filename or location during creation which should be returned after content storage.

Example, adapter timestamps upload filenames. getFileLocation may result in a slightly different url than the one used to create the file.

In the case where the url returned is identical this is not a problem. Additionally file adapter could call getFile location internally if it wanted to (and should probably have access to config for that reason).

In principle, if user wanted to name every file uploaded in an ordinal fashion (eg 1.jpg,2.jpg,3.jpg), they should allowed to do this.

As a separate improvement, this allows the url to be assigned during createFile as well (to prevent extra calls if the url is already known). Passing config to createFile allows getFileLocation to be called internally). Otherwise if url is not assigned, then getFileLocation can be called to retrieve it with the updated filename.

If url is not provided and filename is unchanged by createFile, then this code will not alter current functionality

Pull Request

Issue

Closes: https://github.com/parse-community/parse-server/issues/9556

Approach

1. Allow file location and filename as generated to be overwritten by adapter.createFile if the file adapter desires
2. Pass config as additional argument to fileadapter so that the adapter can call getFileLocation during createFile if desired (optional)

Tasks

• [x] Add tests
• [x] Add changes to documentation (guides, repository pages, code comments)
• [x] Add security check

[parse-github-assistant[bot]]

Thanks for opening this pull request!

[AdrianCurtin]

@mtrezza Tests added

[AdrianCurtin]

@mtrezza Adjusted cloud code createFileSpy and mock adapter in tests, if you can re-run

[mtrezza]

@AdrianCurtin Thank you for picking this up again, I've restarted the CI, let's see...

[coderabbitai[bot]]

📝 Walkthrough

Walkthrough

FilesAdapter.createFile gains a final config parameter; FilesController.createFile supplies that config to the adapter, uses adapter-returned name to override filename and adapter-returned url (or getFileLocation fallback) for the stored URL. Tests updated/added to validate new call signature and adapter return permutations.

Changes

Cohort / File(s)	Summary
Tests: saveFile hooks and adapter signature spec/CloudCode.spec.js	Updated tests to expect adapter.createFile called with 5 arguments, asserting the 5th arg is the config (applicationId: 'test').
Tests: FilesController createFile branching spec/FilesController.spec.js	Added tests covering adapter returns: { name, url }, undefined, url only, and name only; verify final filename and URL resolution and fallback to getFileLocation.
Adapter interface update src/Adapters/Files/FilesAdapter.js	Public API change: createFile(filename, data, contentType, options, config); JSDoc updated to document config and broadened return description (may resolve to object with name/url/location or undefined).
Controller flow update src/Controllers/FilesController.js	FilesController.createFile now calls adapter.createFile(..., config), applies createResult?.name to filename, and uses createResult?.url or getFileLocation(config, filename) for the URL; removed pre-create getFileLocation call.

Sequence Diagram(s)

sequenceDiagram
  participant Client
  participant FilesController
  participant FilesAdapter

  Client->>FilesController: createFile(filename, data, contentType, options)
  FilesController->>FilesAdapter: createFile(filename, data, contentType, options, config)
  FilesAdapter-->>FilesController: { name?, url? } or undefined
  alt adapter provides name
    FilesController->>FilesController: filename = result.name
  end
  alt adapter provides url
    FilesController->>FilesController: url = result.url
  else
    FilesController->>FilesAdapter: getFileLocation(config, filename)
    FilesAdapter-->>FilesController: url
  end
  FilesController-->>Client: { url, name: filename }

Estimated code review effort

🎯 3 (Moderate) | ⏱️ ~20 minutes

Pre-merge checks (4 passed, 1 warning)

❌ Failed checks (1 warning)

Check name	Status	Explanation	Resolution
Description Check	⚠️ Warning	The pull request description does not adhere to the repository template because the “## Pull Request” section is empty and missing the required security and license bullet points, and the free-form summary is not structured under the prescribed headings.	Please restructure the description to match the repository’s template by populating the “## Pull Request” section with the security policy and license statements, moving the summary under “## Approach,” and ensuring all required headings and information from the template are filled out.

✅ Passed checks (4 passed)

Check name	Status	Explanation
Title Check	✅ Passed	The title clearly and concisely summarizes the primary change of allowing file adapters to specify dynamic filenames and custom file locations during file creation, matching the core functionality introduced by the pull request without unnecessary detail or noise.
Linked Issues Check	✅ Passed	All changes align with issue #9556 by adding a fifth config parameter to createFile, updating the FilesController to use createFile’s returned name and URL or fallback via getFileLocation, and extending tests and documentation to validate adapters can dynamically set final filenames and locations while preserving existing behavior.
Out of Scope Changes Check	✅ Passed	The pull request does not introduce any changes outside the scope of issue #9556, as all modifications are limited to extending the createFile signature, adjusting FilesController behavior, and updating tests and documentation to support dynamic filenames and locations.
Docstring Coverage	✅ Passed	No functions found in the changes. Docstring coverage check skipped.

---

📜 Recent review details

Configuration used: CodeRabbit UI

Review profile: CHILL

Plan: Pro

📥 Commits

Reviewing files that changed from the base of the PR and between 731988421d1d708a8470abab5428bdc827174af0 and 46e277b2f8d664e49e45bb3979f38e84707718d0.

📒 Files selected for processing (1)

• spec/CloudCode.spec.js (3 hunks)

🚧 Files skipped from review as they are similar to previous changes (1)

• spec/CloudCode.spec.js

---

Thanks for using CodeRabbit! It's free for OSS, and your support helps us grow. If you like it, consider giving us a shout-out.

❤️ Share

• X
• Mastodon
• Reddit
• LinkedIn

_{Comment @coderabbitai help to get the list of available commands and usage tips.}

[parseplatformorg]

:tada: Snyk checks have passed. No issues have been found so far.

:white_check_mark: security/snyk check is complete. No issues have been found. (View Details)

[AdrianCurtin]

@mtrezza

In order to get this to pass the CI checks, I limited the server config access to just app id, mount and the legacy filekey, since that's what it looks like all of the current file adapters are using. If something else is required we could add it or weaken the tests so that a less precise config could be checked. lmk what you think

[codecov[bot]]

Codecov Report

:white_check_mark: All modified and coverable lines are covered by tests. :white_check_mark: Project coverage is 92.57%. Comparing base (82fdb0d) to head (46e277b).

Additional details and impacted files

@@            Coverage Diff             @@
##            alpha    #9557      +/-   ##
==========================================
- Coverage   92.99%   92.57%   -0.43%     
==========================================
  Files         187      187              
  Lines       15096    15097       +1     
  Branches      174      174              
==========================================
- Hits        14039    13976      -63     
- Misses       1045     1105      +60     
- Partials       12       16       +4     

:umbrella: View full report in Codecov by Sentry.
:loudspeaker: Have feedback on the report? Share it here.

:rocket: New features to boost your workflow:

• :snowflake: Test Analytics: Detect flaky tests, report on failures, and find test suite problems.
• :package: JS Bundle Analysis: Save yourself from yourself by tracking and limiting bundle sizes in JS merges.

[Moumouls]

@mtrezza any chance to run the workflow ?

[mtrezza]

Done

[Moumouls]

@AdrianCurtin how does the S3 PR play with this one https://github.com/parse-community/parse-server-s3-adapter/pull/242

Both are needed ?

[AdrianCurtin]

@Moumouls

This PR was designed to respect legacy behavior (or else personally I would have moved config to the first argument to match getFileLocation's argument order).

For a fileadapter that does not return result.name or result.url, there should be no breaking changes. So old s3 adapters and other fileadapters should continue to function as is.

The pr for the s3 adapter https://github.com/parse-community/parse-server-s3-adapter/pull/242 will take this argument but won't actually return the proper location (url) without the config being passed.

So if this PR is merged then 242 can function and preserve filenames. But if this PR is not merged, 242 will still run, but won't preserve the filenames (legacy behavior).

Both need to be merged in order to fix the issue, but if either is merged (or for instance an older parse-server with a newer s3 adapter), it should behave with legacy behaviors.

[Moumouls]

I think i've a last question @AdrianCurtin : how the fallback approach of createdFile.url || getFileLocation, play in case of using "publicServerUrl", does the adapter is aware of the publicServerUrl to return appropriate path (like https://example.com/parse/files/xxxxxxx.jpeg) and not the url of the Storage provider, since parse-server support proxying files

[AdrianCurtin]

@Moumouls In the s3 adapter modification, the config is only used to return the getFileLocation internally AFTER creating the file/ modifying the file key

So really calling getFileLocation here is just a thing that would happen if the url wasn't provided by the createFile itself (current behavior). As long as getFileLocation respects the public url it should still work as expected.

As implemented, a user who does not use some argument (like generate key) to modify the filename would ideally not see any difference in behavior and the createdFile.url || getFileLocation fallback would simply prevent getFileLocation from being called twice when it already gave you an answer once. (getFileLocation function always being accessible within the file adapter itself, but requiring the config to be called).

On the other hand, if an adapter createFile opted to return a url that was different than the call from getFileLocation (for instance hypothetically a resolved ip address or something) then this would break the functionality you mentioned. But if they instead opted to not return anything or call getFileLocation internally, then there would be no change in behavior.

I delegated this to the file adapter in the case the getFileLocation might be pre-computed or some other variation and prevent some redundant processing, or some file adapter that might need to modify the filename based on the result of getFileLocation (for instance an invalid character) or something. (but I didn't do any of this in the s3 adapter pr, it just calls getFileLocation so that create file directly returns whatever it was supposed to.

[Moumouls]

So to summarize, if developer use generateKey and public server url, the system give priority to created URL not the public one.

question: does the current generateKey usage with public server url and parse file proxy activated behave this way ? it could be interesting to cover this, and check if we have a related bug, or to unsure we don't break existing parse server using generateKey with parse proxy, but seems weird since your current PR here and on S3 actually fix the implementation of generateKey which is currently broken if i understand correctly

[AdrianCurtin]

@Moumouls

generate_key i think is a specific s3 adapter functionality and the preserve filenames also must be enabled too, so its a double-opt-in there to even begin to play with the filenames.

Frankly i don't know if the parse file proxy works at all with S3 adapter (i have never tried).

But in theory if it worked, generate key should have nothing to do with the parse file proxy since it's exclusively modifying the filename (and potentially s3 path by extension).

The generate key does not play with the s3 root itself.

But lets say that the internal call in the s3 adapter was not to url = await getFileLocation, but instead to url = await getResolvedFileLocationNoProxy()

Then in this hypothetical example the url's would not match because getFileLocation and getResolvedFileLocationNoProxy would produce different results.

So the only way to break parse proxy behavior is to either a) use a getFileLocation that does not respect parse proxy behavior in the first place, or b) use a novel getFileLocation internally or method that does not respect the parse proxy behavior.

But option a) would obviously break that function anyways and b) might be more particular.

Maybe a note for future developers or a test should basically say that createFileresult.url should match getFileLocation unless there is a specific requirement for them to differ.

We could use an additional test for this, but that actually could be a valid behavior? Consider a situation where you want the file to be accessible once and never again, createFileResult.url could be an accessor returned by file creation and getFileLocation for generic file access.

In either case this is an opt-in behavior by the developer of the adapter, opt-out should simply return the file location.

[AdrianCurtin]

Maybe a simpler answer would be that yes it gives preference to the created url, but in most cases this is expected to be the same as the public server url. Generate key should have nothing to do with it. The developer of the adapter would need to intentionally return a different created url than would be expected.

[Moumouls]

okay @mtrezza seems good to me may be we need another final approval from another contributor @dplewis ? to move forward ?

Thanks @AdrianCurtin for your detailed answers and the time spent into this PR !

[mtrezza]

Could you please look at the open conversations and close the ones that are done? Would also be good to have another AI agent look over the changes and potential implications.

[AdrianCurtin]

@Moumouls I think most of the conversations here are resolved. Anything I got back for this part from AI agents was mostly concerning passing the full config for the server var into the file adapter. Which could contain information if the file adapter wasn't privileged to know it. However the fact that the file adapter gets the information from getFileLocation anyways kind of makes that a moot point.

It wouldn't be a bad idea to in the future restrict which extensions/adapters get the full file adapter info, but for the sake of this particular PR that would be a much more significant revision and could break behaviors where other people use such terms. (not during create file specifically but elsewhere).