Skip to content

feat(storage): add deleteSourceObjects option to Bucket::compose()#9211

Open
salilg-eng wants to merge 8 commits into
googleapis:mainfrom
salilg-eng:feat/delete_source_objects
Open

feat(storage): add deleteSourceObjects option to Bucket::compose()#9211
salilg-eng wants to merge 8 commits into
googleapis:mainfrom
salilg-eng:feat/delete_source_objects

Conversation

@salilg-eng
Copy link
Copy Markdown
Contributor

This PR introduces the deleteSourceObjects option (boolean) to the Bucket::compose() method. When set to true, the library takes care of auto-deleting the source objects immediately and safely following a successful composition.

Transaction Safe: Cleanup is only triggered if composition succeeds. If composition fails and throws a ServiceException, the source files are safely preserved.
Polymorphic Handling: The deletion loop correctly handles lists containing both plain string paths (string[]) and typed StorageObject instances.
API Definition Consistency: deleteSourceObjects is added as a schema query parameter in the API Service Definition (storage-v1.json), ensuring consistent serialization down to the REST connection level.

@salilg-eng salilg-eng requested review from a team as code owners May 25, 2026 11:50
@product-auto-label product-auto-label Bot added the api: storage Issues related to the Cloud Storage API. label May 25, 2026
Comment thread Storage/src/Bucket.php
throw new \InvalidArgumentException('A content type could not be detected and must be provided manually.');
}

$deleteSourceObjects = $options['deleteSourceObjects'] ?? false;
Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If the option is a bool we should enforce the bool:

if (!$deleteSourceObjects instanceof bool) {
    throw new \InvalidArgumentException('The deleteSourceObjects option should be a boolean');
}

currently If the user passes a ['deleteSourceObjects'] = 52 the check for $deleteSourceObjects would be "true" even if the value is not a boolean.

It might behave as expected, but I strongly believe we should let the user know that a parameter is the wrong parameter.

Copy link
Copy Markdown
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Thank you for the excellent feedback! Enforcing strict type checking for deleteSourceObjects is a great safety addition to prevent loose 'truthy' evaluations.

Regarding the implementation: in PHP, the instanceof operator is reserved only for checking objects against class/interface structures and does not support checking primitive types (like bool, string, int). Calling $val instanceof bool on a real boolean will look for a class named bool, return false, and therefore always throw the exception.

To resolve this, I've implemented the validation using PHP's built-in primitive validation function is_bool():

if (!is_bool($deleteSourceObjects)) {
    throw new \InvalidArgumentException('The deleteSourceObjects option should be a boolean.');
}

Copy link
Copy Markdown
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Oh yeah! My bad! haha it is in fact not instanceof bool but is_bool, you got it!

@salilg-eng salilg-eng requested a review from Hectorhammett May 27, 2026 09:41
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Labels

api: storage Issues related to the Cloud Storage API.

Projects

None yet

Development

Successfully merging this pull request may close these issues.

2 participants