Skip to content
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

S3 Adapter can't delete "folders" #182

Open
burzum opened this issue May 21, 2013 · 12 comments
Open

S3 Adapter can't delete "folders" #182

burzum opened this issue May 21, 2013 · 12 comments
Assignees
Labels
Milestone

Comments

@burzum
Copy link

burzum commented May 21, 2013

S3 does not know folders but you can use keys that mimic folders like /some/folders/myfile.txt.

By looking at the delete method of the S3 adapter you can see it is only deleting a single object. https://github.com/KnpLabs/Gaufrette/blob/master/src/Gaufrette/Adapter/AmazonS3.php#L195

The old version of the AWS SDK Gaufrette is using offers a method delete_all_objects() https://github.com/amazonwebservices/aws-sdk-for-php/blob/master/services/s3.class.php#L2603 which takes wildcards and by this allows you to delete all objects within a given namespace like /some/* would delete /some/folders/myfile.txt.

I propose to change the adapter to check if the given object "isDirectory()" and and if yes call delete_all_objects().

Let me know if you agree and I might create a PR for that.

@jdewit
Copy link

jdewit commented May 27, 2013

+1

1 similar comment
@tshafer
Copy link

tshafer commented Jul 1, 2013

+1

@mtdowling
Copy link
Contributor

Isn't that a really, really dangerous change to make? A simple typo could delete the entire contents of a bucket.

However, I think that Gaufrette should allow you to delete single objects or delete an empty bucket.

@estahn
Copy link
Contributor

estahn commented Oct 3, 2013

@mtdowling I don't see how it is more dangerous than a typo in a usual file system path.

@dogancelik
Copy link

I don't know if this is fixed but I vote for it. 👍

@AshleyDawson
Copy link

+1

2 similar comments
@cuhsat
Copy link

cuhsat commented Feb 11, 2015

+1

@JanisGruzis
Copy link

+1

@burzum
Copy link
Author

burzum commented Apr 28, 2017

Isn't that a really, really dangerous change to make? A simple typo could delete the entire contents of a bucket.

Oh noes, putting responsibility on the developer! 😨 How do you ever delete a local folder then without getting a heart attack? 😄

@akerouanton akerouanton added this to the v1.0 milestone May 1, 2017
@akerouanton
Copy link
Contributor

I think @mtdowling is right: adding a method to delete every objects in a bucket is pretty dangerous. In the other hand I think Filesystem abstraction and underlying adapters are lacking such a feature (deleting multiple objects based on a pattern).

So I would a consider a new method in Filesystem based on a new interface and an opt-in option allow_emptying_buckets for adapters. What do you think?

@estahn
Copy link
Contributor

estahn commented May 1, 2017

@NiR- Again, how is it different from deleting a local directory? If we follow that argument i vote for removing the rmdir from the Local Adapter :) ... You have to do the same checks. And @burzum has a good point too.

@burzum
Copy link
Author

burzum commented May 1, 2017

@NiR- the behaviour across adaptors should be consistent. I think the better way is that any storage system allows deleting of folders / buckets / whatever. If some people are scared simply add a new option that disables deleting of non-empty "folders". This should solve the problem and give people the option instead of forcing a direction.

@akerouanton akerouanton added this to TODO 1.x in Gaufrette Jul 7, 2017
@nicolasmure nicolasmure moved this from TODO 1.x to DONE in Gaufrette Sep 21, 2017
@nicolasmure nicolasmure moved this from DONE to TODO 1.x in Gaufrette Sep 21, 2017
@nicolasmure nicolasmure removed this from TODO 1.x in Gaufrette Sep 21, 2017
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Projects
None yet
Development

No branches or pull requests