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

Files saved with double extension #39

Open
joshuaswilcox opened this issue Jul 26, 2016 · 4 comments
Open

Files saved with double extension #39

joshuaswilcox opened this issue Jul 26, 2016 · 4 comments

Comments

@joshuaswilcox
Copy link

when using a custom filename that include the actual name of the uploaded file, the extension is duplicated. Here is my code:

defmodule Scotchy.Image do
  use Arc.Definition
  use Arc.Ecto.Definition

  @versions [:original, :thumb]

  def __storage, do: Arc.Storage.Local

  def filename(version,  {file, _scope}), do: "#{version}_#{file.file_name}"

  # Whitelist file extensions:
  def validate({file, _}) do
    ~w(.jpg .jpeg .gif .png .bmp .JPG) |> Enum.member?(Path.extname(file.file_name))
  end

  def storage_dir(_version, {_file, scope}) do
    "uploads/bottle_images/images/#{scope.id}"
  end

  # Define a thumbnail transformation:
  def transform(:thumb, _) do
    {:convert, "-strip -thumbnail 350 -gravity center -extent 350"}
  end

  def default_url(:thumb) do
    "https://placehold.it/350x400"
  end
end

which results in images named thumb_IMG_055.jpg.jpg. I know I can String.replace(file.file_name, ".jpg", "") but that seems hacky

@t56k
Copy link

t56k commented Aug 1, 2016

This happens to me too. I lost the custom filename as a quick fix, but obvs that's not ideal.

@davidkuhta
Copy link

@joshuaswilcox
Try:

def filename(version, {file, _scope}), do: "#{version}_#{Path.basename(file.file_name, Path.extname(file.file_name))}"

See:
Arc Local Tests

@stavro
Copy link
Owner

stavro commented Aug 29, 2016

Ah - Thanks @davidkuhta.

As I currently have it, the "filename" should not include the extension, because different versions of a file may have different extensions. (i.e. you may want the original jpg file stored, but a thumbnail png).

If you have any suggestions on how to make that more clear I'm all 👂

@davidkuhta
Copy link

davidkuhta commented Aug 29, 2016

Hi @stavro
If I trace the code right I think this is actually more of an Arc consideration.
I believe the confusion stems from some examples in the Arc docs, that don't necessarily allude when you override the filename/2 function in storage.ex, you lose the default extension stripping:
def filename(_, {file, _}), do: Path.basename(file.file_name, Path.extname(file.file_name))

Particularly Local Configuration and File Names that show:
def filename(version, {file, scope}), do: "#{version}-#{file.file_name}"

So my thought would be to either:

  1. Provide a comment in the Arc docs that communicates file_name != basename (and includes Path.basename(file.file_name, Path.extname(file.file_name)))
  2. (My preference) Expose a helper function (say filename/1 in Arc.Definition.Storage) that strips the extension (ultimately what the default filename/2 was doing).
    So in storage.ex you'd now have:
def filename(_, {file, _}), do: filename(file)
def filename(file), do: Path.basename(file.file_name, Path.extname(file.file_name))

Option 2. would mean you could do things like:
def filename(version, {file, _scope}), do: "#{version}_#{filename(file)}"
It also wouldn't break anything because the filename/2 still provides the same implementation as it did before.

Note: I chose filename because basename/1 is used by Path but there's nothing to say it couldn't just be that. (Which IMHO is what I would've assumed Path.basename/1 did...) (stemname/1 was another thought)

Anyhow, let me know if either fits your vision for arc/arc_ecto and I'll be happy to send over a PR.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

4 participants