[thomasyu888]

Problem:

• When the caller has READ but not DOWNLOAD permission on a file (for example, an unmet access requirement), File(id=..., download_file=False).get() returns data_file_handle_id=None, even though Synapse includes dataFileHandleId on the entity in the POST /entity/{id}/bundle2 response.
• As a result, a follow-up store(), such as moving the file by changing parent_id, fails client-side with ValueError: Cannot store file. The file must have one of: .... The same move works in the web UI.
• Root cause: Synapse returns "fileHandles": [] in the bundle when the caller can't download the file. _handle_file_entity() in synapseclient/api/entity_factory.py then sets file_handle to an empty placeholder FileHandle() and calls _fill_from_file_handle(). That method checks if self.file_handle:, which is always true for a dataclass instance, so the placeholder overwrote data_file_handle_id, content_type, content_size and external_url with None.

Solution:

• In _handle_file_entity(), only call _fill_from_file_handle() when the bundle actually contained a matching file handle, so the dataFileHandleId from the entity is preserved.
• _handle_file_entity() handles both File and RecordSet, and it's the only caller that can pass an empty placeholder. The other callers (fill_from_dict when _file_handle is present, and _upload_file) always have a real handle. So a single guard here covers every affected path. I also tried guarding inside _fill_from_file_handle(), but it's duplicated in File and RecordSet, so that meant changing two copies for no practical gain.
• file_handle is still set to an empty FileHandle() rather than None. Existing code reads file_handle.<attr> without a None check (e.g. _get_file_migration_status() in models/services/migration.py) and would raise AttributeError on restricted files.
• No server-side or permission behavior changes. The file handle metadata is still not exposed to callers without DOWNLOAD permission.

Testing:

• Added test_get_with_unmet_access_requirement (File) and test_get_async_with_unmet_access_requirement (RecordSet). Both go through the real entity factory with a mocked bundle containing fileHandles: [] and an unmet access requirement. Both fail without the fix (assert None == 888 / assert None == '888') and pass with it.
• Full unit suite passes locally (3240 passed, 8 skipped). 4 modules weren't collected because the local env lacks optional deps (boto3, jsonschema); they don't touch this code path.
• Manually verified against an access-restricted entity on production: get() now returns the correct data_file_handle_id.
• Not yet verified: the end-to-end move (store() with a new parent_id) on a restricted entity.

🤖 Generated with Claude Code

[BryanFauble]

Thanks for this

[Copilot]

Warning

Copilot couldn't run its full agentic review because it didn't start before the timeout. Make sure your repository has a runner available, or add a copilot-code-review.yml file specifying one with the runs-on attribute. See the docs for more details.

Copilot review overview

2 open issues

• if file_handle: relies on truthiness, but the intent (per the comment) is specifically to run… · New
• The PR description’s Testing section mentions adding test_get_with_unmet_access_requirement… · New

What changed in this PR

Preserves dataFileHandleId on file-like entities when Synapse withholds file handle metadata due to missing DOWNLOAD permission (e.g., unmet access requirements), preventing client-side failures on subsequent store() operations.

Changes:

• Guard _fill_from_file_handle() so it only runs when a file handle is actually returned in the bundle.
• Add async unit tests asserting data_file_handle_id is preserved when fileHandles is empty due to unmet access requirements.

File	Description
synapseclient/​api/​entity_factory.py	Avoids overwriting entity-level file handle fields when Synapse omits file handle metadata.
tests/​unit/​synapseclient/​models/​async/​unit_test_file_async.py	Adds regression test for File.get_async() to preserve data_file_handle_id when access requirements are unmet.
tests/​unit/​synapseclient/​models/​async/​unit_test_recordset_async.py	Adds similar regression test for RecordSet.get_async() under unmet access requirements.

_{Review effort: Lite}

[thomasyu888]

Note, i have to test this specifically around moving data from a folder with unsatisfied AR to another folder with the same AR

I tested

> import synapseclient
> from synapseclient.models import File

> syn = synapseclient.login()
> temp = File(id="syn74416546", download_file=False).get()

Welcome, Thomas Yu! You are using the 'default' profile.
[WARNING]
This entity has access restrictions. Please visit the web page for this entity (syn.onweb("syn74416546")). Look for the "Access" label and the lock icon underneath the file name. Click "Request Access", and then review and fulfill the file download requirement(s).

> temp.parent_id = "syn74416514"
> temp.store()

File(id='syn74416546', name='Chili Crisp (1).xlsx', path=None, description=None, parent_id='syn74416514', version_label='2', version_comment=None, data_file_handle_id='171282387', external_url=None, activity=None, annotations={}, content_type=None, content_size=None, content_md5=None, etag='a06cbe16-edc9-4565-b90f-551d7248511f', created_on='2026-04-13T17:35:12.550Z', modified_on='2026-10-28T16:35:16.626Z', created_by='3391317', modified_by='3324230', version_number=2, is_latest_version=True, file_handle=FileHandle(id=None, etag=None, created_by=None, created_on=None, modified_on=None, concrete_type=None, content_type=None, content_md5=None, file_name=None, storage_location_id=None, content_size=None, status=None, bucket_name=None, key=None, preview_id=None, is_preview=None, external_url=None))

> temp = File(id="syn74416546", download_file=False).get()
[WARNING]
This entity has access restrictions. Please visit the web page for this entity (syn.onweb("syn74416546")). Look for the "Access" label and the lock icon underneath the file name. Click "Request Access", and then review and fulfill the file download requirement(s).
> print(temp.parent_id)
syn74416514