peek-travel / ecto_hashids

A library to seamlessly convert between sequential IDs and hashids for ecto schemas
MIT License
16 stars 4 forks source link

Characters option not being respected? #2

Open thiagomajesk opened 2 years ago

thiagomajesk commented 2 years ago

Hi! I'm conducting some tests with hashids using a specific set of characters and noticed that the configuration is somehow not being respected?

config :ecto_hashids,
  prefix_separator: "-",
  characters: "abcdefghijklmnopqrstuvwxyz-+_.~:#[]@!$()",
  salt: "my-salted-salt",
  prefix_descriptions: %{
    char: MyModule
  }
iex(1)> EctoHashids.id!({"char", 1})
%{hashid: "char-yabda", pkey: 1, prefix: "char"}
iex(2)> EctoHashids.id!({"char", 2})
%{hashid: "char-7pdma", pkey: 2, prefix: "char"} # <----- where did this 7 came from?
iex(3)> EctoHashids.id!({"char", 3})
%{hashid: "char-ypwxa", pkey: 3, prefix: "char"}
iex(4)> EctoHashids.id!({"char", 4})
%{hashid: "char-raqka", pkey: 4, prefix: "char"}
iex(5)> EctoHashids.id!({"char", 5})
%{hashid: "char-danwp", pkey: 5, prefix: "char"}

I'm not sure where lies the problems because the configuration is passed directly to the respective alphabet option. Moreover, after testing the lib Hashids directly, it seems to work properly:

Hashids.new(alphabet: "abcdefghijklmnopqrstuvwxyz-+_.~:#[]@!$()", min_len: 5, salt: "my-salted-salt") |> Hashids.encode(2)
coladarci commented 2 years ago

Sorry for this! I could of sworn I had a note about this in the Readme; I'm pretty sure you have to mix deps.clean ecto_hashids after making this config change for it to pick it up.. It's a problem w/ my code that I haven't been able to get in and fix. Can you try that out and see if that fixes it?

thiagomajesk commented 2 years ago

Yeah, that was it @coladarci! I think I had this problem with other libs before, but I'm not sure why this happens. Would using https://hexdocs.pm/elixir/1.12/Application.html#compile_env/3 help with that?

coladarci commented 2 years ago

It's a touch more complicated; if you take a look at EctoHashids.Types you can see at compile time I'm "doing a lot" 😆 . There are definitely ways to fix this, I just haven't had the time to experiment; any help would be much appreciated as it's really the only problem I have w/ this library :)

thiagomajesk commented 2 years ago

I think Application.compile_env/3 will actually give a helpful message if you don't want to have this behavior documented in the README. However, I do think that a better solution overall would not do that much at compilation time tbh haha 😅.

An example:

ERROR! the application :ecto_hashids has a different value set for key :characters during runtime compared to compile time. Since this application environment entry was marked as compile time, this difference can lead to different behaviour than expected:

  * Compile time value was set to: "abcdefghijklmnopqrstuvwxyz☻♠♣♥♦☺♤♧♡♢♪♫☼§▲▼⌂«»αßΓπΣσµτΦΘΩδ∞φε∩≡≈†‡‹›¤øÞðþ"
  * Runtime value was set to: "12345"

To fix this error, you might:

  * Make the runtime value match the compile time one

  * Recompile your project. If the misconfigured application is a dependency, you may need to run "mix deps.compile ecto_hashids --force"

  * Alternatively, you can disable this check. If you are using releases, you can set :validate_compile_env to false in your release configuration. If you are using Mix to start your system, you can pass the --no-validate-compile-env flag

I've prepared a PR if you want to merge it: https://github.com/peek-travel/ecto_hashids/pull/3.