-
Notifications
You must be signed in to change notification settings - Fork 62
Cache store #132
Cache store #132
Changes from 6 commits
ccbdaa4
90f3c9d
550693c
8606af8
d87edab
8b54271
bc32d62
a7fc6ee
f4201cf
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -26,7 +26,8 @@ config :logger, :console, | |
config :thesis, | ||
store: Thesis.EctoStore, | ||
authorization: Example.ThesisAuth, | ||
uploader: Thesis.RepoUploader | ||
uploader: Thesis.RepoUploader, | ||
enable_cache: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. I'd like to allow them to specify their own cache store if they want, rather than just a boolean true/false. To do that, let's change this to There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Yeah, I like your solution a lot better :) The one thing I'm not crazy about though is the config :thesis, cache: {Thesis.CacheStore, [other: :options, can: "go here"]} And then our store behavior functions could (optionally with a [] default) accept those options as a second argument like so: def page(slug, [cache_name: name]) do
ConCache.get_or_store(name, slug, fn() ->
store().page(slug)
end)
end
def page_contents(slug, opts) when is_binary(slug) do
page_contents(page(slug, opts))
end Users will be able to pass in configuration values from the thesis config to their own custom stores or caches without having to either hardcode it in their own modules or set up their own additional config. Thoughts? There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. You've got me thinking, @anthonylebrun ... I think to be consistent, we'd use this strategy instead (note the separate # Configure thesis content editor
config :thesis,
store: Thesis.EctoStore,
cache: Thesis.CacheStore,
authorization: Example.ThesisAuth,
uploader: Thesis.RepoUploader
config :thesis, Thesis.EctoStore, repo: Example.Repo
config :thesis, Thesis.CacheStore,
cache_name: "whut_whut",
other_option: "heyo" There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Nice! This seems more idiomatic than what I was suggesting :) I'm still learning those Elixir conventions. |
||
|
||
# config :thesis, Thesis.OspryUploader, | ||
# ospry_public_key: System.get_env("OSPRY_PUBLIC_KEY") | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -30,9 +30,10 @@ import_config "#{Mix.env}.exs" | |
config :thesis, | ||
store: Thesis.EctoStore, | ||
authorization: ExamplePhx.ThesisAuth, | ||
uploader: Thesis.RepoUploader | ||
uploader: Thesis.RepoUploader, | ||
# uploader: <MyApp>.<CustomUploaderModule> | ||
# uploader: Thesis.OspryUploader | ||
enable_cache: true | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will change to: cache: Thesis.CacheStore |
||
config :thesis, Thesis.EctoStore, repo: ExamplePhx.Repo | ||
# config :thesis, Thesis.OspryUploader, | ||
# ospry_public_key: "pk-prod-asdfasdfasdfasdf" | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -9,6 +9,18 @@ defmodule Thesis.Config do | |
Application.get_env(:thesis, :store) | ||
end | ||
|
||
def enable_cache do | ||
Application.get_env(:thesis, :enable_cache, false) | ||
end | ||
|
||
def cache_name do | ||
Application.get_env(:thesis, :cache_name, :thesis_cache) | ||
end | ||
|
||
def cache do | ||
if enable_cache(), do: Thesis.CacheStore, else: store() | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. This will change to: def cache do
Application.get_env(:thesis, :cache, store())
end |
||
end | ||
|
||
def dynamic_pages do | ||
Application.get_env(:thesis, :dynamic_pages) | ||
end | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
@@ -0,0 +1,63 @@ | ||
defmodule Thesis.CacheStore do | ||
@moduledoc """ | ||
Thesis.CacheStore is a proxy store that acts in conjunction with a proper | ||
store like Thesis.EctoStore to provide caching and faster page load times. | ||
It can be enabled by setting `Thesis.Config.enable_cache` is set to `true`. | ||
""" | ||
|
||
@behaviour Thesis.Store | ||
|
||
import Thesis.Config | ||
alias Thesis.{Page, PageContent} | ||
|
||
def page(slug) do | ||
ConCache.get_or_store(cache_name(), slug, fn() -> | ||
store().page(slug) | ||
end) | ||
end | ||
|
||
def page_contents(slug) when is_binary(slug) do | ||
page_contents(page(slug)) | ||
end | ||
|
||
def page_contents(nil) do | ||
cache_get(:global, fn() -> | ||
store().page_contents(nil) | ||
end) | ||
end | ||
|
||
def page_contents(page = %Page{id: page_id}) do | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's decouple this from def page_contents(page = %{id: page_id}) do
|
||
cache_get(page_id, fn() -> | ||
store().page_contents(page) | ||
end) | ||
end | ||
|
||
def update(page_params = %{"slug" => slug}, contents_params) do | ||
cache_delete(slug) | ||
store().page_contents(slug) |> Enum.each(fn(%PageContent{page_id: page_id}) -> | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Let's decouple this from store().page_contents(slug) |> Enum.each(fn(%{page_id: page_id}) -> We should probably review the rest of the code base and make sure we're doing that elsewhere too. :) |
||
cache_delete(page_id) | ||
end) | ||
store().update(page_params, contents_params) | ||
end | ||
|
||
def delete(params = %{"slug" => slug}) do | ||
ConCache.delete(cache_name(), slug) | ||
store().delete(params) | ||
end | ||
|
||
def restore(id) do | ||
store().restore(id) | ||
end | ||
|
||
def backups(page_slug_or_id) do | ||
store().backups(page_slug_or_id) | ||
end | ||
|
||
defp cache_get(key, fun) do | ||
ConCache.get_or_store(cache_name(), key, fun) | ||
end | ||
|
||
defp cache_delete(key) do | ||
ConCache.delete(cache_name(), key) | ||
end | ||
end |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Maybe flesh out the paragraph here, give a little more background regarding why this is desirable.