KnpLabs / Gaufrette

PHP library that provides a filesystem abstraction layer − will be a feast for your files!
http://knplabs.github.io/Gaufrette
MIT License
2.47k stars 355 forks source link

S3 Adapter can't delete "folders" #182

Open burzum opened 11 years ago

burzum commented 11 years ago

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 commented 11 years ago

+1

tshafer commented 11 years ago

+1

mtdowling commented 11 years ago

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 commented 11 years ago

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

dogancelik commented 10 years ago

I don't know if this is fixed but I vote for it. :+1:

AshleyDawson commented 9 years ago

+1

cuhsat commented 9 years ago

+1

JanisGruzis commented 7 years ago

+1

burzum commented 7 years ago

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 commented 7 years ago

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 commented 7 years ago

@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 commented 7 years ago

@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.