fix: dont require select:bucket permissions to delete files (nhost#87)

controller/controller.go

@@ -60,7 +60,7 @@ type MetadataStorage interface {
  headers http.Header) (FileMetadata, *APIError,
  )
  SetIsUploaded(ctx context.Context, fileID string, isUploaded bool, headers http.Header) *APIError
- DeleteFileByID(ctx context.Context, fileID string, headers http.Header) (FileMetadataWithBucket, *APIError)
+ DeleteFileByID(ctx context.Context, fileID string, headers http.Header) *APIError
  ListFiles(ctx context.Context, headers http.Header) ([]FileSummary, *APIError)
 }
 

controller/delete_broken_metadata.go

@@ -14,7 +14,7 @@ func (ctrl *Controller) deleteBrokenMetadata(ctx *gin.Context) ([]FileSummary, *
  }
 
  for _, m := range missing {
- if _, apiErr := ctrl.metadataStorage.DeleteFileByID(ctx.Request.Context(), m.ID, ctx.Request.Header); apiErr != nil {
+ if apiErr := ctrl.metadataStorage.DeleteFileByID(ctx.Request.Context(), m.ID, ctx.Request.Header); apiErr != nil {
  return nil, apiErr
  }
  }

controller/delete_broken_metadata_test.go

@@ -96,10 +96,10 @@ func TestDeleteBrokenMetadata(t *testing.T) {
 
  metadataStorage.EXPECT().DeleteFileByID(
  gomock.Any(), "b3b4e653-ca59-412c-a165-92d251c3fe86", gomock.Any(),
- ).Return(controller.FileMetadataWithBucket{}, nil)
+ ).Return(nil)
  metadataStorage.EXPECT().DeleteFileByID(
  gomock.Any(), "e6aad336-ad79-4df7-a09b-5782f71948f4", gomock.Any(),
- ).Return(controller.FileMetadataWithBucket{}, nil)
+ ).Return(nil)
 
  ctrl := controller.New("http://asd", "asdasd", metadataStorage, contentStorage, nil, logger)
 

controller/delete_file.go

@@ -7,37 +7,31 @@ import (
  "github.com/gin-gonic/gin"
 )
 
-// this type is used to ensure we respond consistently no matter the case.
-type DeleteFileResponse struct {
- ProcessedFile FileMetadata `json:"processedFile,omitempty"`
-}
-
-func (ctrl *Controller) deleteFile(ctx *gin.Context) (FileMetadata, *APIError) {
+func (ctrl *Controller) deleteFile(ctx *gin.Context) *APIError {
  id := ctx.Param("id")
 
- fileMetadata, apiErr := ctrl.metadataStorage.DeleteFileByID(ctx.Request.Context(), id, ctx.Request.Header)
+ apiErr := ctrl.metadataStorage.DeleteFileByID(ctx.Request.Context(), id, ctx.Request.Header)
  if apiErr != nil {
- return FileMetadata{}, apiErr
+ return apiErr
  }
 
- if apiErr := ctrl.contentStorage.DeleteFile(fileMetadata.ID); apiErr != nil {
- return FileMetadata{}, apiErr
+ if apiErr := ctrl.contentStorage.DeleteFile(id); apiErr != nil {
+ return apiErr
  }
 
  ctx.Set("FileChanged", id)
 
- return fileMetadata.FileMetadata, nil
+ return nil
 }
 
 func (ctrl *Controller) DeleteFile(ctx *gin.Context) {
- fileMetadata, apiErr := ctrl.deleteFile(ctx)
- if apiErr != nil {
+ if apiErr := ctrl.deleteFile(ctx); apiErr != nil {
  _ = ctx.Error(fmt.Errorf("problem processing request: %w", apiErr))
 
  ctx.JSON(apiErr.statusCode, apiErr.PublicResponse())
 
  return
  }
 
- ctx.JSON(http.StatusNoContent, DeleteFileResponse{fileMetadata})
+ ctx.Status(http.StatusNoContent)
 }

controller/delete_file_test.go

@@ -43,31 +43,7 @@ func TestDeleteFile(t *testing.T) {
 
  metadataStorage.EXPECT().DeleteFileByID(
  gomock.Any(), "55af1e60-0f28-454e-885e-ea6aab2bb288", gomock.Any(),
- ).Return(
- controller.FileMetadataWithBucket{
- FileMetadata: controller.FileMetadata{
- ID: "55af1e60-0f28-454e-885e-ea6aab2bb288",
- Name: "my-file.txt",
- Size: 64,
- BucketID: "default",
- ETag: "\"55af1e60-0f28-454e-885e-ea6aab2bb288\"",
- CreatedAt: "2021-12-27T09:58:11Z",
- UpdatedAt: "2021-12-27T09:58:11Z",
- IsUploaded: true,
- MimeType: "text/plain; charset=utf-8",
- UploadedByUserID: "0f7f0ff0-f945-4597-89e1-3636b16775cd",
- },
- Bucket: controller.BucketMetadata{
- ID: "default",
- MinUploadFile: 0,
- MaxUploadFile: 100,
- PresignedURLsEnabled: true,
- DownloadExpiration: 30,
- CreatedAt: "2021-12-15T13:26:52.082485+00:00",
- UpdatedAt: "2021-12-15T13:26:52.082485+00:00",
- CacheControl: "max-age=3600",
- },
- }, nil)
+ ).Return(nil)
 
  contentStorage.EXPECT().DeleteFile(
  "55af1e60-0f28-454e-885e-ea6aab2bb288",

controller/upload_file.go

@@ -92,7 +92,7 @@ func (ctrl *Controller) upload(
 
  etag, contentType, err := ctrl.uploadSingleFile(file, file.ID)
  if err != nil {
- _, _ = ctrl.metadataStorage.DeleteFileByID(
+ _ = ctrl.metadataStorage.DeleteFileByID(
  ctx,
  file.ID,
  http.Header{"x-hasura-admin-secret": []string{ctrl.hasuraAdminSecret}},

example_curl.sh

@@ -1,6 +1,6 @@
 #!/usr/bin/env bash
 
-URL=http://localhost:8000/v1/storage/files
+URL=http://localhost:8000/v1/files
 AUTH="Authorization: Bearer $(make dev-jwt)"
 BUCKET=default
 

metadata/hasura.go

@@ -287,13 +287,11 @@ func (h *Hasura) SetIsUploaded(
  return nil
 }
 
-func (h *Hasura) DeleteFileByID(
- ctx context.Context,
- fileID string,
- headers http.Header,
-) (controller.FileMetadataWithBucket, *controller.APIError) {
+func (h *Hasura) DeleteFileByID(ctx context.Context, fileID string, headers http.Header) *controller.APIError {
  var query struct {
- StorageFileByPK FileMetadataWithBucket `graphql:"deleteFile(id: $id)"`
+ StorageFileByPK struct {
+ ID graphql.String `graphql:"id"`
+ } `graphql:"deleteFile(id: $id)"`
  }
 
  variables := map[string]interface{}{
@@ -304,14 +302,14 @@ func (h *Hasura) DeleteFileByID(
  err := client.Mutate(ctx, &query, variables)
  if err != nil {
  aerr := parseGraphqlError(err)
- return controller.FileMetadataWithBucket{}, aerr.ExtendError("problem executing query")
+ return aerr.ExtendError("problem executing query")
  }
 
- if query.StorageFileByPK.ID == graphql.String("") {
- return controller.FileMetadataWithBucket{}, controller.ErrFileNotFound
+ if query.StorageFileByPK.ID == "" {
+ return controller.ErrFileNotFound
  }
 
- return query.StorageFileByPK.ToControllerType(), nil
+ return nil
 }
 
 func (h *Hasura) ListFiles(ctx context.Context, headers http.Header) ([]controller.FileSummary, *controller.APIError) {

metadata/hasura_test.go

@@ -449,38 +449,13 @@ func TestDeleteFileByID(t *testing.T) {
  name string
  fileID string
  headers http.Header
- expected controller.FileMetadataWithBucket
  expectedStatusCode int
  expectedPublicResponse *controller.ErrorResponse
  }{
  {
- name: "success",
- fileID: fileID,
- headers: getAuthHeader(),
- expected: controller.FileMetadataWithBucket{
- FileMetadata: controller.FileMetadata{
- ID: fileID,
- Name: "name",
- Size: 123,
- BucketID: "default",
- ETag: "asdasd",
- CreatedAt: "",
- UpdatedAt: "",
- IsUploaded: true,
- MimeType: "text",
- UploadedByUserID: "",
- },
- Bucket: controller.BucketMetadata{
- ID: "default",
- MinUploadFile: 1,
- MaxUploadFile: 50000000,
- PresignedURLsEnabled: true,
- DownloadExpiration: 30,
- CreatedAt: "2022-01-05T19:02:58.387709+00:00",
- UpdatedAt: "2022-01-05T19:02:58.387709+00:00",
- CacheControl: "max-age=3600",
- },
- },
+ name: "success",
+ fileID: fileID,
+ headers: getAuthHeader(),
  expectedStatusCode: 0,
  expectedPublicResponse: &controller.ErrorResponse{},
  },
@@ -519,22 +494,14 @@ func TestDeleteFileByID(t *testing.T) {
  t.Parallel()
  tc := tc
 
- got, err := hasura.DeleteFileByID(context.Background(), tc.fileID, tc.headers)
+ err := hasura.DeleteFileByID(context.Background(), tc.fileID, tc.headers)
  if tc.expectedStatusCode != err.StatusCode() {
  t.Errorf("wrong status code, expected %d, got %d", tc.expectedStatusCode, err.StatusCode())
  }
  if err != nil {
  if !cmp.Equal(err.PublicResponse(), tc.expectedPublicResponse) {
  t.Error(cmp.Diff(err.PublicResponse(), tc.expectedPublicResponse))
  }
- } else {
- opts := cmp.Options{
- cmpopts.IgnoreFields(controller.FileMetadata{}, "CreatedAt", "UpdatedAt"),
- cmpopts.IgnoreFields(controller.BucketMetadata{}, "CreatedAt", "UpdatedAt"),
- }
- if !cmp.Equal(got, tc.expected, opts...) {
- t.Error(cmp.Diff(got, tc.expected, opts...))
- }
  }
  })
  }