Permalink
Browse files

core: add guards to m_rsc_gone functions.

This gives a better error message.

Fix for controller_page:previously_existed/2 to return 404 for unknown resource id's.

Fixes #459.

Also an additional fix for controller_page:resource_exists/2,
as discussed in commit 45bca64 on github.
  • Loading branch information...
1 parent 474374e commit ae823f4a82e5529c73593f8a7c80ba96771c72fc @kaos kaos committed Nov 14, 2012
Showing with 16 additions and 11 deletions.
  1. +12 โˆ’7 modules/mod_base/controllers/controller_page.erl
  2. +4 โˆ’4 src/models/m_rsc_gone.erl
@@ -34,12 +34,13 @@ resource_exists(ReqData, Context) ->
ContextQs = z_context:ensure_qs(Context1),
try
Id = get_id(ContextQs),
- true = m_rsc:exists(Id, ContextQs),
- case z_context:get(cat, ContextQs) of
- undefined ->
- ?WM_REPLY(true, ContextQs);
- Cat ->
- ?WM_REPLY(m_rsc:is_a(Id, Cat, ContextQs), ContextQs)
+ case {m_rsc:exists(Id, ContextQs), z_context:get(cat, ContextQs)} of
+ {Exists, undefined} ->
+ ?WM_REPLY(Exists, ContextQs);
+ {true, Cat} ->
+ ?WM_REPLY(m_rsc:is_a(Id, Cat, ContextQs), ContextQs);
+ {false, _} ->
+ ?WM_REPLY(false, ContextQs)
end
catch
_:_ -> ?WM_REPLY(false, ContextQs)
@@ -48,7 +49,11 @@ resource_exists(ReqData, Context) ->
%% @doc Check if the resource used to exist
previously_existed(ReqData, Context) ->
Context1 = ?WM_REQ(ReqData, Context),
- IsGone = m_rsc_gone:is_gone(get_id(Context1), Context1),
+ IsGone = case get_id(Context1) of
+ Id when is_integer(Id) ->
+ m_rsc_gone:is_gone(Id, Context1);
+ _ -> false
+ end,
?WM_REPLY(IsGone, Context1).
@@ -54,7 +54,7 @@ m_value(#m{value=undefined}, _Context) ->
%% @doc Get the possible 'rsc_gone' resource for the id.
-get(Id, Context) ->
+get(Id, Context) when is_integer(Id) ->
F = fun() ->
z_db:assoc_row("select * from rsc_gone where id = $1", [Id], Context)
end,
@@ -63,20 +63,20 @@ get(Id, Context) ->
%% @doc Check if the resource used to exist.
-spec is_gone(integer(), #context{}) -> boolean().
-is_gone(Id, Context) ->
+is_gone(Id, Context) when is_integer(Id) ->
F = fun() ->
z_db:q1("select count(*) from rsc_gone where id = $1", [Id], Context) =:= 1
end,
z_depcache:memo(F, {rsc_is_gone, Id}, Context).
%% @doc Copy a resource to the 'gone' table, use the current user as the modifier (deleter).
-spec gone(integer(), #context{}) -> {ok, integer()}.
-gone(Id, Context) ->
+gone(Id, Context) when is_integer(Id) ->
gone(Id, undefined, Context).
%% @doc Copy a resource to the 'gone' table, use the current user as the modifier (deleter).
%% Also sets the 'new id', which is the id that replaces the deleted id.
-gone(Id, NewId, Context) ->
+gone(Id, NewId, Context) when is_integer(Id), is_integer(NewId) ->
Props = z_db:assoc_row("
select id, name, version, page_path, uri, is_authoritative, creator_id, created
from rsc

7 comments on commit ae823f4

@arjan
Member
arjan commented on ae823f4 Nov 14, 2012

nice ๐Ÿ‘

@mmzeeman
Member

FYI Working on sqlite3 support. These kind of type mistakes will not trigger any errors in sqlite3. When you define that a column should be an integer you can happily put character data in it. Works just fine.

@arjan
Member
arjan commented on ae823f4 Nov 14, 2012

Yeah! Can't wait till it's done.... :-D

@mmzeeman
Member

Basic queries work, transactions work, the connection pool with poolboy are ready. Wanted to start with moving things around in z_db too see what else is needed. Have made a branch on my clone of zotonic :-)

It is very interesting to see the design tradeoffs of both db's. Postgres wants its types right, but doesn't care to much about transactions. This is exactly the opposite for sqlite.

@arjan
Member
arjan commented on ae823f4 Nov 14, 2012

And it's made so that db support is more modular, I guess? So if somebody would want to create a mysql adapter... :-)

Anyhow, even more motivation for me to get 0.9 out of the door so we can merge your stuff in right after!
Most important thing missing is having nice defaults for the base site template and polising the blog template.

@mmzeeman
Member

The idea is that you provide a driver module for your database. If there already is an interface module for mysql that is not difficult. Usually a matter of finding out how to get meta-information out of the database and return that to the database layer. I'm working on it when I have time, currently very busy with something else.

@arjan
Member
arjan commented on ae823f4 Nov 14, 2012

Sure, but that is what you'd expect. https://github.com/Eonblast/Emysql is pretty good I hear.

Please sign in to comment.