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

Document test env setup with decorators? #155

Closed
atavistock opened this issue May 11, 2022 · 11 comments
Closed

Document test env setup with decorators? #155

atavistock opened this issue May 11, 2022 · 11 comments

Comments

@atavistock
Copy link

atavistock commented May 11, 2022

I'm able to easily switch adapters before the supervision tree starts, but I'm somewhat confused on the best approach to the decorator pattern and how it might work with different implementations.

In any given module I have @decorator cacheable(cache: MyApp.Cache.Local). The problem is that MyApp.Cache.Local is the real implementation, not an alias or a mock. I know I could do something messy checking the Mix.env and setting an appropriate alias in each module. But Is there a good way to define the mock class at a higher level like in application.ex or config.ex?

@atavistock atavistock changed the title Documentation test env? Document test env setup with decorators? May 11, 2022
@suzdalnitski
Copy link
Contributor

Having the exact same issue... Is there a way to use the Null Adapter with decorators?

@cabol
Copy link
Owner

cabol commented May 13, 2022

Hey 👋 !!

Well, the main limitation is decorations are evaluated in compilation time, so yes, the cache you pass there is the real implementation. I think there may be several ways to address this, but let me mention a workaround I've used myself in these cases. You can add a configuration parameter at the app level to define the adapter to use, for example:

adapter = Application.get_env(:my_app, :cache_adapter, Nebulex.Adapters.Local)

defmodule MyApp.Cache do
  otp_app: :my_app,
  adapter: adapter
end

And then you can use the different config files depending on the environment, for example in the test.exs you could:

config :my_app,
  cache_adapter: Nebulex.Adapters.Nil

It is a very naive/simple workaround but it has worked for me, maybe not the best, but it helps to address the need of having different adapters depending on the environment.

I will think if there is something else we can do about it, but in the meantime let me know if the workaround works for you. Thanks!

@suzdalnitski
Copy link
Contributor

@cabol

Thanks for a quick reply!

Do you think it may be possible to modify the decorator to accept a function returning the cache at runtime?

Something among the lines of:

defp cache, do: Application.fetch_env!(:my_app, :cache)

@decorate cacheable(cache: &cache/1)
def get_account(id) do
  ...
end

Would you be interested in a PR for this feature?

@cabol
Copy link
Owner

cabol commented May 16, 2022

Hey @suzdalnitski !! yeah, I think it is possible, in fact, the options :match and :key_generator accept a function and it is evaluated at runtime, so we can do the same with the cache for sure, and it would be a great feature!!

Would you be interested in a PR for this feature?

Absolutely !!!

@suzdalnitski
Copy link
Contributor

@cabol

Let me know if I should make any changes to my PR: #156

@atavistock
Copy link
Author

atavistock commented May 19, 2022

While I think this is a good direction, it just seems to move the problem a little.

So this does allow run-time evaluation in the decorator, but because the alternative implementation is not also evaluated in application.ex its not in the supervision tree.

I have to specifically evaluate the value at compile time, so I can also start the right process. My working implementation looks like this:

lib/my_app/cache.ex

defmodule MyApp.Cache do
  @cache_implementation Application.compile_env(:my_app, :cache_implementation, MyApp.Cache.Local)
  
  def dynamic_cache do
    @cache_implementation
  end

  defmodule Local do
    use Nebulex.Cache, otp_app: :my_app, adapter: Nebulex.Adapters.Local
  end

  defmodule Nil do
    use Nebulex.Cache, otp_app: :my_app, adapter: Nebulex.Adapters.Nil
  end
end

lib/my_app/application.ex

...
children = [
  MyApp.Cache.dynamic_cache(),
...

config/test.exs

...
  config :my_app, :cache_implementation,  MyApp.Cache.dynamic_cache()
...

Now the cache decorator can work with the runtime mfa tuple like @decorate cacheable(cache: {Carmagic.Cache, :dynamic_cache, []}), but because its evaluated at compile time it also works by calling the function directly like @decorate cacheable(cache: Carmagic.Cache.dynamic_cache())

@suzdalnitski
Copy link
Contributor

You still need to start the cache, here's how I'm doing this in tests (based on the DataCase):

defmodule MyApp.CacheCase do
  use ExUnit.CaseTemplate

  alias Ecto.Adapters.SQL.Sandbox

  using do
    quote do
      import Ecto
      import Ecto.Changeset
      import Ecto.Query
    end
  end

  setup _ do
    # Ecto setup
    :ok = Sandbox.checkout(MyApp.Repo)

    Sandbox.mode(MyApp.Repo, {:shared, self()})

    # Start the cache
    {:ok, cache_pid} = MyApp.Cache.LiveCache.start_link()

    Application.put_env(:my_app, :cache, MyApp.Cache.LiveCache)

    # Stop the cache on exit
    on_exit(fn ->
      if Process.alive?(cache_pid), do: Process.exit(cache_pid, :normal)

      Application.put_env(:my_app, :cache, MyApp.Cache.TestCache)
    end)

    :ok
  end
end

The caches are defined as:

defmodule MyApp.Cache.LiveCache do
  use Nebulex.Cache,
    otp_app: :my_app,
    adapter: Nebulex.Adapters.Local
end

defmodule MyApp.Cache.TestCache do
  use Nebulex.Cache,
    otp_app: :my_app,
    adapter: Nebulex.Adapters.Nil
end

Then make sure that you're running this as async within your tests:

use MyApp.CacheCase, async: false

@suzdalnitski
Copy link
Contributor

@cabol
Is there anything that can be improved in the PR? Thanks!

@cabol
Copy link
Owner

cabol commented Jun 1, 2022

@suzdalnitski sorry for the lateness, I've been pretty busy, I checked it already and I added a comment

@suzdalnitski the dialyzer is failing, I think I know why, but that will require some changes, so, since the PR looks overall good to me, I will merge it for now, and will push some improvements and the dialyzer fix later (very soon).

So, the PR is merged, but will push some fixes and/or improvements soon.

Also, I created another ticket documenting the feature a bit better #157

@suzdalnitski
Copy link
Contributor

@cabol That's great, thank you!

@cabol
Copy link
Owner

cabol commented Nov 4, 2022

Closing this issue since some workarounds have been shared and the ability to resolve the cache in decorators at runtime. However, for Nevbulex v3.0.0 I'm planning to address this better, not only with the decorators but in general, so suggestions or ideas are welcome, feel free to open an issue with the proposal.

@cabol cabol closed this as completed Nov 4, 2022
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

3 participants