[SYNPY-1671] Adding store & delete factory method

[BryanFauble]

Problem:

• The synapseclient library requires users to instantiate entity classes and call methods on those instances for common operations like storing and deleting, which creates unnecessary friction.
• Following the factory-style pattern established with the get/get_async implementation, additional methods need to be exposed as standalone functions for a more streamlined developer experience.
• This PR addresses a subset of the methods identified in the Jira ticket: store and delete.

Solution:

• Implemented new factory functions following the pattern established by get/get_async:
  • delete / delete_async: Unified interface for deleting any Synapse entity type (File, Folder, Project, Table, Dataset, Team, etc.) with support for:
    • Deletion by entity object or string Synapse ID (e.g., "syn123456" or "syn123456.4")
    • Version-specific deletion with clear precedence rules (explicit version parameter > entity's version_number attribute > ID string version)
    • Safety validation requiring version_only=True when deleting specific versions
    • Appropriate warnings for entity types that don't support version-specific deletion
  • store / store_async: Unified interface for storing any Synapse entity type with type-specific option classes:
    • StoreFileOptions: Controls for synapse_store, content_type, merge_existing_annotations, associate_activity_to_new_version
    • StoreContainerOptions: Controls for failure_strategy (LOG_EXCEPTION vs RAISE_EXCEPTION)
    • StoreTableOptions: Controls for dry_run and job_timeout
    • StoreJSONSchemaOptions: Required options for JSONSchema entities including schema_body, version, dry_run
    • StoreGridOptions: Controls for attach_to_previous_session and timeout
• Updated synapseclient/operations/__init__.py to expose all new functions and option classes at the package level.

Testing:

• Added comprehensive integration test suites covering both synchronous and asynchronous variants:
  • test_delete_operations_async.py / test_delete_operations.py: Tests for file deletion by ID string and object, version-specific deletion with various precedence scenarios, error handling for invalid IDs and missing version numbers, warning logging for unsupported version deletion on entity types like Project/Folder
  • test_factory_operations_store_async.py / test_factory_operations_store.py: Tests for storing all supported entity types (Project, Folder, File, Table, Dataset, EntityView, Team, Evaluation, CurationTask, JSONSchema, Grid, etc.), option class functionality, update workflows, and error handling for unsupported types

[Copilot]

Pull request overview

This PR introduces factory-style methods store, store_async, delete, and delete_async to simplify common operations on Synapse entities. The implementation follows the pattern established by the existing get/get_async methods and provides a unified interface for storing and deleting various entity types with type-specific configuration options.

Key Changes:

• New store/store_async functions supporting 15+ entity types with 5 specialized option classes (StoreFileOptions, StoreContainerOptions, StoreTableOptions, StoreJSONSchemaOptions, StoreGridOptions)
• New delete/delete_async functions with version-specific deletion support and clear precedence rules for version parameters
• Comprehensive integration test suites covering both synchronous and asynchronous variants

Reviewed changes

Copilot reviewed 7 out of 7 changed files in this pull request and generated 10 comments.

Show a summary per file

File	Description
synapseclient/operations/store_operations.py	Implements store/store_async factory methods with entity-specific handlers and option classes
synapseclient/operations/delete_operations.py	Implements delete/delete_async factory methods with version handling and validation
synapseclient/operations/init.py	Exports new functions and option classes at package level
tests/integration/synapseclient/operations/synchronous/test_factory_operations_store.py	Integration tests for synchronous store operations covering all supported entity types
tests/integration/synapseclient/operations/synchronous/test_delete_operations.py	Integration tests for synchronous delete operations including version-specific deletion
tests/integration/synapseclient/operations/async/test_factory_operations_store_async.py	Async variants of store operation integration tests
tests/integration/synapseclient/operations/async/test_delete_operations_async.py	Async variants of delete operation integration tests

---

💡 Add Copilot custom instructions for smarter, more guided reviews. Learn how to get started.

[Copilot]

The test description says "Grid deletion is fire-and-forget, no need to verify" but this is misleading - the delete operation is still being called and could potentially fail silently. Either verify the deletion succeeded or document why verification is not necessary for Grid entities specifically.

Suggested change

	# Grid deletion is fire-and-forget, no need to verify
	# For Grid entities, this test only checks that delete() can be called without raising.

[Copilot]

The error message includes a hardcoded suggestion with 'delete()' but does not specify the async variant. When this error is raised from delete_async(), the suggestion should say delete_async() instead. Consider making the function name dynamic or providing context-appropriate suggestions.

Suggested change

	f"Deleting a specific version requires version_only=True. "
	f"Use delete('{entity}', version_only=True) to delete version {final_version}."
	"Deleting a specific version requires version_only=True. "
	f"Pass version_only=True when calling this function to delete version {final_version}."

[Copilot]

The type hint for version parameter allows Union[int, str], but the comparison at line 493 version != entity_version doesn't account for type coercion (e.g., version=2 as string "2" vs entity_version=2 as int). This could cause false positive warnings. Consider normalizing both to the same type before comparison, or document that version should always be an int.

Suggested change

	# Emit warning only when there's an actual version conflict (both are set and different)
	if (
	version_only
	and version is not None
	and entity_version is not None
	and version != entity_version
	# Normalize versions for comparison to avoid false conflicts between str/int
	normalized_version = (
	int(version) if isinstance(version, str) and version.isdigit() else version
	)
	normalized_entity_version = (
	int(entity_version)
	if isinstance(entity_version, str) and entity_version.isdigit()
	else entity_version
	)
	# Emit warning only when there's an actual version conflict (both are set and different)
	if (
	version_only
	and normalized_version is not None
	and normalized_entity_version is not None
	and normalized_version != normalized_entity_version

[Copilot]

The version parameter type is Union[int, str] but this is assigned directly to entity.version_number at line 516 without type conversion. If final_version_for_entity is a string, this could cause issues if version_number expects an int. Consider converting to int or clarifying the expected type.

Suggested change

	# Set the entity's version_number to the final version so delete_async uses it
	entity.version_number = final_version_for_entity
	# Normalize final_version_for_entity to an int before assigning
	if isinstance(final_version_for_entity, str):
	try:
	final_version_for_entity_int = int(final_version_for_entity)
	except ValueError as exc:
	raise ValueError(
	f"Invalid version value '{final_version_for_entity}'; an integer is required."
	) from exc
	else:
	final_version_for_entity_int = final_version_for_entity
	# Set the entity's version_number to the final version so delete_async uses it
	entity.version_number = final_version_for_entity_int

[andrewelamb]

I think I've seen this logic elsewhere. It might make sense to have a SynapseID object that does this check. Then this function could take in a str or SynapseID type.

class SynapseID(id: str):


      if not is_synapse_id_str(id):
            raise ValueError(
                f"Invalid Synapse ID: {entity}. "
                "Expected a valid Synapse ID string (e.g., 'syn123456' or 'syn123456.4')."
            )

    self.id, self.version = get_synid_and_version(id)


Then it could do:

if isinstance(entity, str):
entity = SynapseID(entity)

[BryanFauble]

I think that it needs to be a little easier to verify the Synapse ID format, but putting it into a class doesn't seem like the right thing. Rather it probably makes sense to be a utility function.

[andrewelamb]

LGTM!