benwinding / react-admin-firebase

A firebase data provider for the react-admin framework
https://benwinding.github.io/react-admin-firebase/
MIT License
459 stars 171 forks source link

Delete image from cloud storage when deleted from Firestore #115

Open davidstackio opened 4 years ago

davidstackio commented 4 years ago

When an image is deleted from a Firestore object, the image should also be deleted from storage as well.

When I delete an image with the react-admin UI or Firestore UI, it gets removed from the Firestore object, but still remains in Storage.

This is for version 3.1.16+ since image uploading was revamped in PR #74.

benwinding commented 4 years ago

Yes that's a good point, perhaps a flag in the options would be a good way to handle this, so that people can have the option

davidstackio commented 4 years ago

I just checked and the image in storage is still not deleted in v3.2.2.

This is especially important now that filenames can be unique with https://github.com/benwinding/react-admin-firebase/pull/134 as there could be a build up of old files. Vs the old way without the new useFileNamesInStorage option, the file is just overwritten.

benwinding commented 4 years ago

This is especially important now that filenames can be unique with #134 as there could be a build up of old files.

Good point @dhstack, thanks for reminding me. I've done something similar in another package here (angular):

https://github.com/benwinding/mat-firebase-upload/blob/bc3f46a42de75376c8c40698ffb9d637563448fe/projects/mat-firebase-upload/src/lib/firebase/uploads-manager.ts#L129-L150

The problem with doing this in react-admin is that we need to compare the current vs new document to determine whether something has been deleted or not. I'll have a look at this soon, while I'm trying to finish #127

If you have any ideas, please let me know.

Cheers, Ben

yusrimathews commented 3 years ago

I've managed to plug the gap with a cloud function in the mean time, when a document is deleted:

functions.firestore.document('places/{docId}').onDelete(async (snapshot, context) => {
  const { docId } = context.params;

  return admin.storage().bucket().deleteFiles({
    prefix: `places/${docId}`
  });
});

And when a document is updated:

functions.firestore.document('places/{docId}').onUpdate(async (snapshot, context) => {
  const oldImages = cleanImagesArray(snapshot.before.data().images);
  const newImages = cleanImagesArray(snapshot.after.data().images);

  const deletedImages = oldImages.filter(oldImage => newImages.every(newImage => newImage.src !== oldImage.src));

  const imagesDeletedPromise = deletedImages.map((image) => {
    const file = admin.storage().bucket().file(image.src);

    file.exists()
      .then(() => {
        return file.delete();
      })
      .catch(() => {
        return console.log(`${image.src} does not exist`);
      });
  });

  return Promise.all(imagesDeletedPromise);
});

The image source contains the image path, therefore I am using cleanImagesArray to clean up the array:

function cleanArray(array = []) {
  const cleanArray = array.map((image) => {
    var imageSrc = image.src;
    var lastForwardSlash = imageSrc.lastIndexOf('/');
    var firstQueryString = imageSrc.indexOf('?');
    var imagePath = imageSrc.substring(lastForwardSlash + 1, firstQueryString);
    var decodedImagePath = decodeURIComponent(imagePath);

    return {
      src: decodedImagePath,
      title: image.title
    }
  });

  return cleanArray;
}

Annoyingly, you can't get an image by image source with the admin SDK (which you can do with the web SDK, in case that helps Ben). The update function only applies where you have multiple images, however it's pretty straightforward to adapt if you have a single image.

NB: Always apply useFileNamesInStorage: true unless you have a specific reason not to, storing multiple images by index is dodgy.