From 9514f1de9c8b1e58c443000cc75e17515ec4948e Mon Sep 17 00:00:00 2001 From: Jocelyn Fiat Date: Tue, 12 May 2015 22:02:23 +0200 Subject: [PATCH] Updated SQLITE builder using GLOBAL_SETTINGS to map 0 to 0, by default 0 -> NULL Updated CMS_NODE_API, with status, not_published, published and trashed. Updated Form response to use permission scopes. Updated sqlquery to retrieve user author. Added logger info in cms_response Updated CMS_NODE with a new status attribute. Updated table nodes to support trashing (or soft deletes) of node using the new status field Updated Sqlite builder to test different scenarios for users and roles. Updated NODE_FORM_RESPONSE.edit_form feature to add a delete operation if there is a node ie node id >0 and the current user has delete permission on it. Updated NODE_HANDLER.do_post to handle the operation "DELETE". Updated queries to retrieve nodes filter by no logical deleted rows (ie. status is trashed). Signed-off-by: jvelilla --- examples/demo/site/scripts/node.sql | 3 +- .../sqlite/src/cms_storage_sqlite_builder.e | 62 +++++++++++++++---- modules/node/cms_node_api.e | 55 +++++++++------- modules/node/content/cms_node.e | 51 ++++++++++++--- modules/node/handler/node_form_response.e | 13 +++- modules/node/handler/node_handler.e | 15 ++++- .../node/persistence/cms_node_storage_sql.e | 34 ++++++---- 7 files changed, 171 insertions(+), 62 deletions(-) diff --git a/examples/demo/site/scripts/node.sql b/examples/demo/site/scripts/node.sql index 85aa51b..d30a854 100644 --- a/examples/demo/site/scripts/node.sql +++ b/examples/demo/site/scripts/node.sql @@ -11,7 +11,8 @@ CREATE TABLE "nodes"( "author" INTEGER, "publish" DATETIME, "created" DATETIME NOT NULL, - "changed" DATETIME NOT NULL + "changed" DATETIME NOT NULL, + "status" INTEGER ); CREATE TABLE page_nodes( diff --git a/library/persistence/sqlite/src/cms_storage_sqlite_builder.e b/library/persistence/sqlite/src/cms_storage_sqlite_builder.e index 0e94aaa..bb4dcf3 100644 --- a/library/persistence/sqlite/src/cms_storage_sqlite_builder.e +++ b/library/persistence/sqlite/src/cms_storage_sqlite_builder.e @@ -12,6 +12,8 @@ class inherit CMS_STORAGE_SQL_BUILDER + GLOBAL_SETTINGS + create make @@ -35,6 +37,8 @@ feature -- Factory end s.append (";LongNames=0;Timeout=1000;NoTXN=0;SyncPragma=NORMAL;StepAPI=0;") create Result.make (create {DATABASE_CONNECTION_ODBC}.login_with_connection_string (s)) + set_map_zero_null_value (False) + -- This way we map 0 to 0, instead of Null as default. --create Result.make (create {DATABASE_CONNECTION_ODBC}.login_with_connection_string (l_database_config.connection_string)) if Result.is_available then if not Result.is_initialized then @@ -47,30 +51,62 @@ feature -- Factory initialize (a_setup: CMS_SETUP; a_storage: CMS_STORAGE_STORE_SQL) local u: CMS_USER - r: CMS_USER_ROLE + l_anonymous_role, l_authenticated_role, r: CMS_USER_ROLE + l_roles: LIST [CMS_USER_ROLE] do - -- Schema + --| Schema a_storage.sql_execute_file_script (a_setup.environment.path.extended ("scripts").extended ("core.sql")) - -- Data - -- Users + --| Roles + create l_anonymous_role.make ("anonymous") + a_storage.save_user_role (l_anonymous_role) + + create l_authenticated_role.make ("authenticated") + a_storage.save_user_role (l_authenticated_role) + + --| Users create u.make ("admin") u.set_password ("istrator#") u.set_email (a_setup.site_email) a_storage.new_user (u) - -- Roles - create r.make ("anonymous") - a_storage.save_user_role (r) - create r.make ("authenticated") - r.add_permission ("create page") - r.add_permission ("edit page") + --| Node + -- FIXME: move that initialization to node module + l_anonymous_role.add_permission ("view any page") + a_storage.save_user_role (l_anonymous_role) + + l_authenticated_role.add_permission ("create page") + l_authenticated_role.add_permission ("view any page") + l_authenticated_role.add_permission ("edit own page") + l_authenticated_role.add_permission ("delete own page") + a_storage.save_user_role (l_authenticated_role) + + + --| For testing purpose, to be removed later. + + -- Roles, view role for testing. + create r.make ("view") + r.add_permission ("view page") a_storage.save_user_role (r) - -- Test custom value + create {ARRAYED_LIST [CMS_USER_ROLE]} l_roles.make (1) + l_roles.force (r) - a_storage.set_custom_value ("abc", "123", "test") - a_storage.set_custom_value ("abc", "OK", "test") + create u.make ("auth") + u.set_password ("enticated#") + u.set_email (a_setup.site_email) + a_storage.new_user (u) + + create u.make ("test") + u.set_password ("test#") + u.set_email (a_setup.site_email) + a_storage.new_user (u) + + create u.make ("view") + u.set_password ("only#") + u.set_email (a_setup.site_email) + u.set_roles (l_roles) + a_storage.new_user (u) end end diff --git a/modules/node/cms_node_api.e b/modules/node/cms_node_api.e index 9fb2370..9922722 100644 --- a/modules/node/cms_node_api.e +++ b/modules/node/cms_node_api.e @@ -249,6 +249,29 @@ feature -- Access: Node end end + is_author_of_node (u: CMS_USER; a_node: CMS_NODE): BOOLEAN + -- Is the user `u' owner of the node `n'. + do + if attached node_storage.node_author (a_node.id) as l_author then + Result := u.same_as (l_author) + end + end + +feature -- Permission Scope: Node + + permission_scope (u: detachable CMS_USER; a_node: CMS_NODE): STRING + -- Result 'own' if the user `u' is the owner of the node `a_node', in other case + -- `any'. + do + -- FIXME: check if this is ok, since a role may have "any" permission enabled, and "own" disabled, + -- in this case, we should check both permissions + -- obviously such case should be rare, and look like bad configured permissions, but this may occurs. + Result := "any" + if u /= Void and then is_author_of_node (u, a_node) then + Result := "own" + end + end + feature -- Change: Node save_node (a_node: CMS_NODE) @@ -279,32 +302,16 @@ feature -- Change: Node node_storage.update_node (a_node) end --- update_node_title (a_user_id: like {CMS_USER}.id; a_node_id: like {CMS_NODE}.id; a_title: READABLE_STRING_32) --- -- Update node title, with user identified by `a_id', with node id `a_node_id' and a new title `a_title'. --- do --- debug ("refactor_fixme") --- fixme ("Check preconditions") --- end --- node_storage.update_node_title (a_user_id, a_node_id, a_title) --- end --- update_node_summary (a_user_id: like {CMS_USER}.id; a_node_id: like {CMS_NODE}.id; a_summary: READABLE_STRING_32) --- -- Update node summary, with user identified by `a_user_id', with node id `a_node_id' and a new summary `a_summary'. --- do --- debug ("refactor_fixme") --- fixme ("Check preconditions") --- end --- node_storage.update_node_summary (a_user_id, a_node_id, a_summary) --- end +feature -- Node status --- update_node_content (a_user_id: like {CMS_USER}.id; a_node_id: like {CMS_NODE}.id; a_content: READABLE_STRING_32) --- -- Update node content, with user identified by `a_user_id', with node id `a_node_id' and a new content `a_content'. --- do --- debug ("refactor_fixme") --- fixme ("Check preconditions") --- end --- node_storage.update_node_content (a_user_id, a_node_id, a_content) --- end + Not_published: INTEGER = 0 + -- The node is not published. + Published: INTEGER = 1 + -- The node is published. + + Trashed: INTEGER = -1 + -- The node is trashed (soft delete), ready to be deleted/destroyed from storage. end diff --git a/modules/node/content/cms_node.e b/modules/node/content/cms_node.e index fcff4cc..51808b2 100644 --- a/modules/node/content/cms_node.e +++ b/modules/node/content/cms_node.e @@ -13,10 +13,6 @@ inherit REFACTORING_HELPER ---create --- make, --- make_empty - feature{NONE} -- Initialization make_empty @@ -35,10 +31,7 @@ feature{NONE} -- Initialization set_creation_date (l_time) set_modification_date (l_time) set_publication_date (l_time) - - debug ("refactor_fixme") - fixme ("Remove default harcoded format") - end + mark_not_published ensure title_set: title = a_title end @@ -60,6 +53,7 @@ feature -- Conversion a_node.summary, a_node.format ) + set_status (a_node.status) end feature -- Access @@ -78,6 +72,12 @@ feature -- Access deferred end + status: INTEGER + -- Associated status for the current node. + -- default: {CMS_NODE_API}.Not_Published} + -- {CMS_NODE_API}.Published + -- {CMS_NODE_API}.Trashed + feature -- Access title: READABLE_STRING_32 @@ -211,6 +211,41 @@ feature -- Element change auther_set: author = u end + mark_not_published + -- Set status to not_published. + do + set_status ({CMS_NODE_API}.not_published) + ensure + status_not_published: status = {CMS_NODE_API}.not_published + end + + mark_published + -- Set status to published. + do + set_status ({CMS_NODE_API}.published) + ensure + status_published: status = {CMS_NODE_API}.published + end + + mark_trashed + -- Set status to trashed. + do + set_status ({CMS_NODE_API}.trashed) + ensure + status_trash: status = {CMS_NODE_API}.trashed + end + + +feature {CMS_NODE_STORAGE_I} -- Access: status change. + + set_status (a_status: like status) + -- Assign `status' with `a_status'. + do + status := a_status + ensure + status_set: status = a_status + end + note copyright: "2011-2015, Javier Velilla, Jocelyn Fiat, Eiffel Software and others" license: "Eiffel Forum License v2 (see http://www.eiffel.com/licensing/forum.txt)" diff --git a/modules/node/handler/node_form_response.e b/modules/node/handler/node_form_response.e index 169205b..8ade0b6 100644 --- a/modules/node/handler/node_form_response.e +++ b/modules/node/handler/node_form_response.e @@ -48,7 +48,7 @@ feature -- Execution attached node_api.node (nid) as l_node then if attached node_api.node_type_for (l_node) as l_type then - if has_permission ("edit " + l_type.name) then + if has_permission ("edit " + node_api.permission_scope (current_user (request), l_node) + " " + l_type.name) then f := edit_form (l_node, url (request.path_info, Void), "edit-" + l_type.name, l_type) if request.is_post_request_method then f.validation_actions.extend (agent edit_form_validate (?, b)) @@ -82,7 +82,7 @@ feature -- Execution attached {WSF_STRING} request.path_parameter ("type") as p_type and then attached node_api.node_type (p_type.value) as l_type then - if has_permission ("create " + l_type.name) then + if has_permission ("create " + l_type.name) then if attached l_type.new_node (Void) as l_node then f := edit_form (l_node, url (request.path_info, Void), "edit-" + l_type.name, l_type) if request.is_post_request_method then @@ -228,6 +228,15 @@ feature -- Form ts.set_default_value ("Preview") f.extend (ts) + if a_node /= Void and then a_node.id > 0 and then has_permission ("delete " + a_name) then + create ts.make ("op") + ts.set_default_value ("Delete") + fixme ("[ + ts.set_default_value (i18n ("Delete"))i18n or other name such as "translated" or "translation + ]") + f.extend (ts) + end + Result := f end diff --git a/modules/node/handler/node_handler.e b/modules/node/handler/node_handler.e index 8894a02..0e6d8b9 100644 --- a/modules/node/handler/node_handler.e +++ b/modules/node/handler/node_handler.e @@ -114,9 +114,17 @@ feature -- HTTP Methods local edit_response: NODE_FORM_RESPONSE do + fixme ("Refactor code: extract methods: edit_node and add_node") if req.path_info.ends_with_general ("/edit") then - create edit_response.make (req, res, api, node_api) - edit_response.execute + if + attached {WSF_STRING} req.form_parameter ("op") as l_op and then + l_op.value.same_string ("Delete") + then + do_delete (req, res) + else + create edit_response.make (req, res, api, node_api) + edit_response.execute + end elseif req.path_info.starts_with_general ("/node/add/") then create edit_response.make (req, res, api, node_api) edit_response.execute @@ -142,11 +150,12 @@ feature -- HTTP Methods l_id.is_integer and then attached node_api.node (l_id.integer_value) as l_node then - if api.user_has_permission (l_user, "delete " + l_node.content_type) then + if api.user_has_permission (l_user, "delete " + node_api.permission_scope (l_user, l_node) + " " + l_node.content_type) then node_api.delete_node (l_node) res.send (create {CMS_REDIRECTION_RESPONSE_MESSAGE}.make (req.absolute_script_url (""))) else send_access_denied (req, res) + -- send_not_authorized ? end else do_error (req, res, l_id) diff --git a/modules/node/persistence/cms_node_storage_sql.e b/modules/node/persistence/cms_node_storage_sql.e index 9d569be..ebbf4a8 100644 --- a/modules/node/persistence/cms_node_storage_sql.e +++ b/modules/node/persistence/cms_node_storage_sql.e @@ -108,8 +108,8 @@ feature -- Access error_handler.reset write_information_log (generator + ".node_author") create l_parameters.make (1) - l_parameters.put (a_id, "node_id") - sql_query (select_node_author, l_parameters) + l_parameters.put (a_id, "nid") + sql_query (Select_user_author, l_parameters) if sql_rows_count >= 1 then Result := fetch_author end @@ -144,11 +144,15 @@ feature -- Change: Node -- Remove node by id `a_id'. local l_parameters: STRING_TABLE [ANY] + l_time: DATE_TIME do + create l_time.make_now_utc write_information_log (generator + ".delete_node") error_handler.reset create l_parameters.make (1) + l_parameters.put (l_time, "changed") + l_parameters.put ({CMS_NODE_API}.trashed, "status") l_parameters.put (a_id, "nid") sql_change (sql_delete_node, l_parameters) end @@ -209,7 +213,7 @@ feature {NONE} -- Implementation error_handler.reset write_information_log (generator + ".store_node") - create l_parameters.make (8) + create l_parameters.make (9) l_parameters.put (a_node.content_type, "type") l_parameters.put (a_node.title, "title") l_parameters.put (a_node.summary, "summary") @@ -217,6 +221,7 @@ feature {NONE} -- Implementation l_parameters.put (a_node.format, "format") l_parameters.put (a_node.publication_date, "publish") l_parameters.put (now, "changed") + l_parameters.put (a_node.status, "status") if attached a_node.author as l_author then check valid_author: l_author.has_id end l_parameters.put (l_author.id, "author") @@ -260,24 +265,28 @@ feature -- Helpers feature {NONE} -- Queries - sql_select_nodes_count: STRING = "SELECT count(*) from Nodes;" + sql_select_nodes_count: STRING = "SELECT count(*) FROM Nodes WHERE status != -1 ;" + -- Nodes count (Published and not Published) + --| note: {CMS_NODE_API}.trashed = -1 - sql_select_nodes: STRING = "SELECT * from Nodes;" + sql_select_nodes: STRING = "SELECT * FROM Nodes WHERE status != -1 ;" -- SQL Query to retrieve all nodes. + --| note: {CMS_NODE_API}.trashed = -1 - sql_select_node_by_id: STRING = "SELECT nid, revision, type, title, summary, content, format, author, publish, created, changed FROM Nodes WHERE nid =:nid ORDER BY revision desc, publish desc LIMIT 1;" + sql_select_node_by_id: STRING = "SELECT nid, revision, type, title, summary, content, format, author, publish, created, changed, status FROM Nodes WHERE nid =:nid ORDER BY revision desc, publish desc LIMIT 1;" - sql_select_recent_nodes: STRING = "SELECT nid, revision, type, title, summary, content, format, author, publish, created, changed FROM Nodes ORDER BY nid desc, publish desc LIMIT :rows OFFSET :offset ;" + sql_select_recent_nodes: STRING = "SELECT nid, revision, type, title, summary, content, format, author, publish, created, changed, status FROM Nodes ORDER BY nid desc, publish desc LIMIT :rows OFFSET :offset ;" - sql_insert_node: STRING = "INSERT INTO nodes (revision, type, title, summary, content, format, publish, created, changed, author) VALUES (1, :type, :title, :summary, :content, :format, :publish, :created, :changed, :author);" + sql_insert_node: STRING = "INSERT INTO nodes (revision, type, title, summary, content, format, publish, created, changed, status, author) VALUES (1, :type, :title, :summary, :content, :format, :publish, :created, :changed, :status, :author);" -- SQL Insert to add a new node. - sql_update_node : STRING = "UPDATE nodes SET revision = revision, type=:type, title=:title, summary=:summary, content=:content, format=:format, publish=:publish, changed=:changed, author=:author WHERE nid=:nid;" + sql_update_node : STRING = "UPDATE nodes SET revision = revision, type=:type, title=:title, summary=:summary, content=:content, format=:format, publish=:publish, changed=:changed, status=:status, author=:author WHERE nid=:nid;" -- FIXME: for now no revision inc.! -- sql_update_node : STRING = "UPDATE nodes SET revision = revision + 1, type=:type, title=:title, summary=:summary, content=:content, format=:format, publish=:publish, changed=:changed, revision = revision + 1, author=:author WHERE nid=:nid;" -- SQL node. - sql_delete_node: STRING = "DELETE FROM nodes WHERE nid=:nid;" + sql_delete_node: STRING = "UPDATE nodes SET changed=:changed, status =:status WHERE nid=:nid" + -- Soft deletion with free metadata. -- sql_update_node_author: STRING = "UPDATE nodes SET author=:author WHERE nid=:nid;" @@ -294,7 +303,7 @@ feature {NONE} -- Queries feature {NONE} -- Sql Queries: USER_ROLES collaborators, author - Select_user_author: STRING = "SELECT uid, name, password, salt, email, status, created, signed FROM Nodes INNER JOIN users ON nodes.author=users.uid AND users.uid = :uid;" + Select_user_author: STRING = "SELECT uid, name, password, salt, email, users.status, users.created, signed FROM Nodes INNER JOIN users ON nodes.author=users.uid AND nodes.nid = :nid;" Select_node_author: STRING = "SELECT nid, revision, type, title, summary, content, format, author, publish, created, changed FROM users INNER JOIN nodes ON nodes.author=users.uid AND nodes.nid =:nid;" @@ -335,6 +344,9 @@ feature {NONE} -- Implementation if attached sql_read_date_time (11) as l_modif_date then Result.set_modification_date (l_modif_date) end + if attached sql_read_integer_32 (12) as l_status then + Result.set_status (l_status) + end end end