stavro / arc

:paperclip: Flexible file upload and attachment library for Elixir
1.17k stars 211 forks source link

Deleting file in S3 problem. #222

Open aislanmaia opened 7 years ago

aislanmaia commented 7 years ago

I'm trying to delete a file on S3 but don't seems working right now.

Below is my bit of code, and I'm trying to get the object deleted right there in the delete function on controller. (using Phoenix 1.2)

def delete(conn, %{"id" => id}) do
    activity = Repo.get!(Activity, id)
    IO.inspect "activity to be deleted..."
    IO.inspect activity
    if activity.file do
      # Deletes the preview file uploaded so then we wont have
      # orphan files in S3 storage.
      path = Api.File.url({activity.file, activity})
      [path | _] = String.split path, "?" # strips the "?v=1234" from the URL string
      IO.inspect "path"
      IO.inspect path
      Api.File.delete({path, activity})
    end

    # Here we use delete! (with a bang) because we expect
    # it to always work (and if it does not, it will raise).
    Repo.delete!(activity)

    send_resp(conn, :no_content, "")
  end

The weird thing is that trying to use the almost same bit of code in the update function, the object in S3 gets replaced by a new one (working like expected, the code first delete on S3, and the library adds the new one on the params)

  def update(conn, %{"id" => id, "activity" => activity_params}) do
    activity = Repo.get!(Activity, id)
    if activity_params["file"] do
      # Deletes the preview file uploaded so then we always will have
      # just one file per activity.
      path = Api.File.url({activity.file, activity})
      [path | _] = String.split path, "?" # strips the "?v=1234" from the URL string
      Api.File.delete({path, activity})
    end
    changeset = Activity.changeset(activity, activity_params)

    case Repo.update(changeset) do
      {:ok, activity} ->
        render(conn, "show.json", activity: activity)
      {:error, changeset} ->
        conn
        |> put_status(:unprocessable_entity)
        |> render(Api.ChangesetView, "error.json", changeset: changeset)
    end
  end

Here is my Activity schema:

defmodule Api.Activity do
  use Api.Web, :model
  use Arc.Ecto.Schema

  schema "activities" do
    field :name, :string
    field :description, :string
    field :due_date, Ecto.Date
    field :file, Api.File.Type
    field :uuid, :string

    timestamps()
  end

  @doc """
  Builds a changeset based on the `struct` and `params`.
  """
  def changeset(struct, params \\ %{}) do
    struct
    |> cast(params, [:name, :description, :due_date])
    |> put_change(:uuid, UUID.uuid4())
    |> cast_attachments(params, [:file])
    |> validate_required([:name, :description, :due_date])
  end
end

A detail is that I'm using sub directories to storage matching the scope:

# Override the storage directory:
  def storage_dir(version, {file, scope}) do
    info = IEx.Info.info(scope)
    type = Enum.find_value(info, fn({k, v}) -> v end)
    if type == "Api.Classplan" do 
      "uploads/classplans/#{scope.uuid}"
    else
      "uploads/activities/#{scope.uuid}"
    end
  end
pedep commented 6 years ago

I'm pretty sure you can delete object by doing Api.File.delete({activity.file, activity}) (might be wrong, but thats how i've been doing it) Not sure if that makes a difference..

The problem could also be related to the UUID juggling you are doing, when you update your activity using put_change(:uuid, UUID.uuid4()), it might leave the uploads on an unreachable path, since the UUID by which the file was stored has since been overwritten?

ps: you can tag your code blocks with elixir, to make them easier to read start the block like this: ```elixir

aislanmaia commented 6 years ago

Thanks @pedep I already have accomplished to get the delete process working just fine some time ago. The problem with :uuid was real... exactly what you said, and because that, I added a new changeset function only to update process excluding the call to put_change function from the pipeline. Now I have a changeset for inserts and update_changeset for updates. However, the code part for deleting a file, actually my code is a little different to what I was doing above. Here is my delete function:


  def delete_file(file, activity) do
    if file do
      # Deletes the preview file uploaded so then we wont have
      # orphan files in S3 storage.
      list = ExAws.S3.list_objects(System.get_env("AWS_S3_BUCKET_UPLOADS"), prefix: "uploads/activities")
      |> ExAws.stream!
      |> Enum.to_list #=> [list, of, objects]

      Enum.each list, fn obj ->
        [_, _, _, file_name] = String.split obj.key, "/"
        if file_name == activity.file.file_name do
          ExAws.S3.delete_object(System.get_env("AWS_S3_BUCKET_UPLOADS"), obj.key)
          |> ExAws.request()
        end
      end
    end

Just giving my reply here for helping others that get trapped in the same problem with :uuid thing and the deleting business.

And thanks for the tips regarding the elixir tag and remember me this issue I have created exists :smile: so I can close it :laughing:

pedep commented 6 years ago

Ah, great :smiley: Good to see you got it working, and to hear my intuition wasn't compelely off :smile: