Refactoring (removing references to session in models.py and persistence.py)
authorNicolas DURAND <nicolas.durand@iri.centrepompidou.fr>
Wed, 31 Dec 2014 15:28:40 +0100
changeset 17 2db9202ad7cf
parent 16 8b7a8a28d5b9
child 18 37981f4fdb77
Refactoring (removing references to session in models.py and persistence.py)
src/catedit/api.py
src/catedit/models.py
src/catedit/persistence.py
--- a/src/catedit/api.py	Tue Dec 30 16:55:56 2014 +0100
+++ b/src/catedit/api.py	Wed Dec 31 15:28:40 2014 +0100
@@ -5,11 +5,13 @@
 serializations strings, it isn't bound to one specific view
 """
 
+from rdflib import Graph
 from flask.ext.restful import Resource, Api, reqparse
-from flask import request
+from flask import request, session
 from catedit import app, cache
 from catedit.models import Category, CategoryManager
 import catedit.persistence
+from io import StringIO
 
 api = Api(app)
 
@@ -40,11 +42,11 @@
             getattr(catedit.persistence, app.config["PERSISTENCE_METHOD"])()
         )
         if cat_id is not None:
-            cat = cat_manager_instance.load_cat(cat_id)
+            cat = cat_manager_instance.load_category(cat_id)
             return cat.cat_graph.serialize(format='turtle').decode("utf-8")
         else:
             response = []
-            for cat in cat_manager_instance.list_cat():
+            for cat in cat_manager_instance.list_categories():
                 response.append(cat.cat_graph.serialize(format='turtle')
                                              .decode("utf-8"))
             return response
@@ -61,9 +63,19 @@
             getattr(catedit.persistence, app.config["PERSISTENCE_METHOD"])()
         )
         args = cat_parser.parse_args()
-        if (cat_id is None and cat_manager_instance.persistence
-                                                   .session_compliant is True):
-            cat_manager_instance.save_cat(message=args["commit_message"])
+        if (cat_id is None):
+            if (cat_manager_instance.persistence.session_compliant is True):
+                logger.debug("Submitting - deleted categories are:"
+                             + str(session.get("deleted_categories", []))
+                             + " and modified categories are:"
+                             + str(session.get("modified_categories", [])))
+                cat_manager_instance.save_changes(
+                    deleted_cat_list = session.get("deleted_categories", []),
+                    modified_cat_list = session.get("modified_categories", []),
+                    message=args["commit_message"]
+                )
+            session["deleted_categories"] = []
+            session["modified_categories"] = []
         else:
             new_property_list = []
             logger.debug(args["property_predicate"])
@@ -86,11 +98,28 @@
                         new_property_list.append((property_predicate,
                                                   property_object_to_append))
             logger.debug(new_property_list)
-            cat = cat_manager_instance.load_cat(cat_id)
+            cat = cat_manager_instance.load_category(cat_id)
             cat.edit_category(new_description=args["description"],
                               new_label=args["label"],
                               new_other_properties=new_property_list)
-            cat_manager_instance.save_cat(cat, message=args["commit_message"])
+
+            session["modified_categories"][:] = [
+                elt for elt in session.get("modified_categories", [])
+                if elt["name"] != cat.cat_id
+            ]
+            session["modified_categories"].append(
+                {"name": cat.cat_id,
+                 "content": str(
+                    cat.cat_graph.serialize(format="turtle"), "utf-8"
+                 )}
+            )
+
+            # Now we must clean the deleted categories list in case the
+            # modified category was deleted before being edited
+            for element in session.get("deleted_categories", []):
+                if element["name"] == cat.cat_id:
+                    session["deleted_categories"].remove(element)
+
             logger.debug("put id: "+cat.cat_id)
         cache.clear()
         return 204
@@ -121,29 +150,136 @@
         cat = Category(label=args["label"],
                        description=args["description"],
                        other_properties=property_list)
-        cat_manager_instance = CategoryManager(
-            getattr(catedit.persistence, app.config["PERSISTENCE_METHOD"])()
+
+        session["modified_categories"][:] = [
+            elt for elt in session.get("modified_categories", [])
+            if elt["name"] != cat.cat_id
+        ]
+        session["modified_categories"].append(
+            {"name": cat.cat_id,
+             "content": str(
+                cat.cat_graph.serialize(format="turtle"), "utf-8"
+             )}
         )
-        cat_manager_instance.save_cat(cat, message=args["commit_message"])
+
         logger.debug("post id: "+cat.cat_id)
         cache.clear()
         return cat.cat_graph.serialize(format='turtle').decode("utf-8"), 201
 
     @classmethod
-    def delete(cls, cat_id):
+    def delete(cls, deleted_cat_id):
         """
-            API to delete the category of id cat_id
+            API to delete the category of id cat_id from the deletion list
         """
         args = cat_parser.parse_args()
-        cat_manager_instance = CategoryManager(
-            getattr(catedit.persistence, app.config["PERSISTENCE_METHOD"])()
-        )
-        if args["delete_message"] is None:
-            message = "Deleting category "+cat_id
+        if (deleted_cat_id in [
+            element["name"] for element in session.get("deleted_categories", [])
+        ]):
+            session["deleted_categories"].remove({"name": deleted_cat_id})
+            # warning, not safe if 2 files share the same name (or category id)
+            # but that shouldn't happen
         else:
-            message = args["delete_message"]
-        cat_manager_instance.delete_cat(cat_id, message=message)
-        logger.debug("delete id: "+cat_id)
+            session["deleted_categories"].append({"name": deleted_cat_id})
+            # now we must clean the modified categories list in case the
+            # deleted category was modified before
+            for element in session.get("modified_categories", []):
+                if element["name"] == deleted_cat_id:
+                    session["modified_categories"].remove(element)
+
+            # Now we also have to clean up categories that reference the
+            # deleted category
+            cat_manager_instance = CategoryManager(
+                getattr(
+                    catedit.persistence, app.config["PERSISTENCE_METHOD"]
+                )()
+            )
+            cat_list = cat_manager_instance.list_categories()
+            # first we edit what was already modified before the deletion
+            logger.debug(session["modified_categories"])
+            element_list = list(session.get("modified_categories", []))
+            if deleted_cat_id in [element["name"] for
+                                  element in session.get("deleted_categories", [])]:
+                for element in element_list:
+                    logger.debug(str(element))
+                    modified_cat_graph = Graph()
+                    modified_cat_graph.parse(source=StringIO(element["content"]),
+                                             format="turtle")
+                    modified_cat = Category(graph=modified_cat_graph)
+                    if (modified_cat.cat_id !=
+                            app.config["CATEGORY_NAMESPACE"] + deleted_cat_id):
+                        new_property_list = []
+                        for (predicate, obj) in modified_cat.properties:
+                            if not (
+                                (app.config["PROPERTY_LIST"]
+                                           [predicate]
+                                           ["object_type"] == "uriref-category")
+                                and
+                                (obj == (app.config["CATEGORY_NAMESPACE"] +
+                                         deleted_cat_id))
+                            ):
+                                new_property_list.append((predicate, obj))
+
+                        if new_property_list != modified_cat.properties:
+                            logger.debug("Modifying modified category")
+                            modified_cat.edit_category(
+                                new_other_properties=new_property_list
+                            )
+                            session["modified_categories"][:] = [
+                                elt for elt in session.get(
+                                    "modified_categories", []
+                                )
+                                if elt["name"] != modified_cat.cat_id
+                            ]
+                            session["modified_categories"].append(
+                                {"name": modified_cat.cat_id,
+                                 "content": str(
+                                    modified_cat.cat_graph
+                                                .serialize(format="turtle"),
+                                    "utf-8"
+                                 )}
+                            )
+                # now we check if an unmodified category reference the deleted
+                # category
+                for cat in cat_list:
+                    if cat.cat_id not in [
+                            element["name"] for element in
+                            session.get("modified_categories", [])
+                    ] and cat.cat_id not in [
+                            element["name"] for element in
+                            session.get("deleted_categories", [])
+                    ]:
+                        new_property_list = []
+                        for (predicate, obj) in cat.properties:
+                            if not (
+                                (app.config["PROPERTY_LIST"]
+                                           [predicate]
+                                           ["object_type"] == "uriref-category")
+                                and
+                                (obj == (app.config["CATEGORY_NAMESPACE"] +
+                                         deleted_cat_id))
+                            ):
+                                new_property_list.append((predicate, obj))
+
+                        if new_property_list != cat.properties:
+                            logger.debug("Modifying untouched category")
+                            cat.edit_category(
+                                new_other_properties=new_property_list
+                            )
+                            session["modified_categories"][:] = [
+                                elt for elt in session.get(
+                                    "modified_categories", []
+                                )
+                                if elt["name"] != cat.cat_id
+                            ]
+                            session["modified_categories"].append(
+                                {"name": cat.cat_id,
+                                 "content": str(
+                                    cat.cat_graph.serialize(format="turtle"),
+                                    "utf-8"
+                                 )}
+                            )
+
+        logger.debug("delete id: " + deleted_cat_id)
         cache.clear()
         return 204
 
--- a/src/catedit/models.py	Tue Dec 30 16:55:56 2014 +0100
+++ b/src/catedit/models.py	Wed Dec 31 15:28:40 2014 +0100
@@ -6,7 +6,6 @@
 """
 
 from rdflib import Graph, RDF, RDFS, Literal, URIRef
-from flask import session
 from uuid import uuid4
 from io import StringIO
 from slugify import slugify
@@ -171,6 +170,12 @@
     """
         CategoryManager class
         This class deals with creation and loading of Category objects
+
+        * load_category returns a category from its id
+        * list_categories returns all categories
+        * save_changes will take a list of modified categories and a list of
+        deleted categories and save the changes using the persistence method
+        * delete_category will delete a single category from its id
     """
     def __init__(self, persistence_method):
         """
@@ -186,7 +191,7 @@
         """
         self.persistence = persistence_method
 
-    def load_cat(self, cat_id):
+    def load_category(self, cat_id):
         """
             Loads a category from its id
         """
@@ -196,96 +201,45 @@
         cat = Category(graph=loaded_cat_graph)
         return cat
 
-    def save_cat(self, cat=None, message=None):
+    def save_changes(self,
+                     deleted_cat_list=[],
+                     modified_cat_list=[],
+                     message=None):
         """
-            Saves a category, message serves as commit message if github
-            is used as a persistence method
+            Saves all changes to categories
 
-            * If cat is None and the persistence method supports submitting
-            changes to multiple files in one go, then it will call
-            submit_changes method
+            * deleted_cat_list must be a list of dict describing deleted
+            categories, each dict of the format :
+            {"name": category_id}
+            * modified_cat_list must be a list of dict describing modified
+            categories, each dict of the format :
+            {"name": category_id, "content": category turtle serialization}
+            * message is used as commit message when applicable (github)
         """
-        if cat is None:
-            self.persistence.submit_changes(message=message)
-        else:
-            self.persistence.save(content=cat.cat_graph
-                                             .serialize(format='turtle'),
-                                  name=cat.cat_id,
-                                  message=message)
+        logger.debug("Calling persistance save method - deleted list is "
+                     + str(deleted_cat_list)
+                     + " and modified list is "
+                     + str(modified_cat_list))
+        self.persistence.save(deletion_list = deleted_cat_list,
+                              modification_list = modified_cat_list,
+                              message=message)
 
-    def delete_cat(self, deleted_cat_id, message=None):
+    def delete_category(self, deleted_cat_id, message=None):
         """
-            Deletes a category from its id, message serves as commit message if
-            github is used as a persistence method
+            Deletes a category from its id
+
+            * message serves as commit message if github is used as a
+            persistence method
+
+            NOTE: This will not auto-update existing categories to delete
+            references to deleted category. This is handled in the api as such
+            operations apply to the intermediary persistence (changeset)
         """
         self.persistence.delete(name=deleted_cat_id, message=message)
         # Now we must clean up the categories that reference the deleted cat
-        cat_list = self.list_cat()
-        # first we edit what was already modified before the deletion
-        logger.debug(session["modified_categories"])
-        element_list = list(session.get("modified_categories", []))
-        if deleted_cat_id in [element["name"] for
-                              element in session.get("deleted_categories", [])]:
-            for element in element_list:
-                logger.debug(str(element))
-                modified_cat_graph = Graph()
-                modified_cat_graph.parse(source=StringIO(element["content"]),
-                                         format="turtle")
-                modified_cat = Category(graph=modified_cat_graph)
-                if (modified_cat.cat_id !=
-                        app.config["CATEGORY_NAMESPACE"] + deleted_cat_id):
-                    new_property_list = []
-                    for (predicate, obj) in modified_cat.properties:
-                        if not (
-                            (app.config["PROPERTY_LIST"]
-                                       [predicate]
-                                       ["object_type"] == "uriref-category")
-                            and
-                            (obj == (app.config["CATEGORY_NAMESPACE"] +
-                                     deleted_cat_id))
-                        ):
-                            new_property_list.append((predicate, obj))
+
 
-                    if new_property_list != modified_cat.properties:
-                        logger.debug("Modifying modified category")
-                        modified_cat.edit_category(
-                            new_other_properties=new_property_list
-                        )
-                        self.save_cat(
-                            modified_cat,
-                            message=message+", cleaning up other properties"
-                        )
-            # now we check if an unmodified category reference the deleted
-            # category
-            for cat in cat_list:
-                if cat.cat_id not in [element["name"] for element in
-                                      session.get("modified_categories", [])] \
-                   and \
-                   cat.cat_id not in [element["name"] for element in
-                                      session.get("deleted_categories", [])]:
-                    new_property_list = []
-                    for (predicate, obj) in cat.properties:
-                        if not (
-                            (app.config["PROPERTY_LIST"]
-                                       [predicate]
-                                       ["object_type"] == "uriref-category")
-                            and
-                            (obj == (app.config["CATEGORY_NAMESPACE"] +
-                                     deleted_cat_id))
-                        ):
-                            new_property_list.append((predicate, obj))
-
-                    if new_property_list != cat.properties:
-                        logger.debug("Modifying untouched category")
-                        cat.edit_category(
-                            new_other_properties=new_property_list
-                        )
-                        self.save_cat(
-                            cat,
-                            message=message+", cleaning up other properties"
-                        )
-
-    def list_cat(self):
+    def list_categories(self):
         """
             Lists all categories available
         """
--- a/src/catedit/persistence.py	Tue Dec 30 16:55:56 2014 +0100
+++ b/src/catedit/persistence.py	Wed Dec 31 15:28:40 2014 +0100
@@ -151,13 +151,19 @@
         """
         return True
 
-    def submit_changes(self, **kwargs):
+    def save(self, **kwargs):
         """
             Saves all the recorded files in the session dict to a Github
             repository
 
             Expected kwargs is:
             * message: the commit message to document the changes
+            * deletion_list : the list of files to delete, list of dicts, each
+            one of the format:
+            {"name": name_of_the_file}
+            * modification_list: the list of the files to change, list of
+            dicts, each one of the format:
+            {"name": name_of_the_file, "content": content_of_the_file}
 
             IMPORTANT: To save to a file Github, we have to use Git
             internal mechanics:
@@ -185,31 +191,38 @@
             tree
         """
 
+        deletion_list = kwargs["deletion_list"]
+        modification_list = kwargs["modification_list"]
+
         # point 1
         ref_master = github.get("repos/"
                                 + app.config["REPOSITORY_OWNER"] + "/"
                                 + app.config["REPOSITORY_NAME"]
                                 + "/git/refs/heads/master")
         logger.debug(str(ref_master))
+
         # point 2
         last_commit_master = github.get("repos/"
                                         + app.config["REPOSITORY_OWNER"] + "/"
                                         + app.config["REPOSITORY_NAME"]
                                         + "/git/commits/"
                                         + ref_master["object"]["sha"])
+        logger.debug(str(last_commit_master))
 
-        logger.debug(str(last_commit_master))
-        # point 3
+        # Point 3
         last_commit_tree = github.get("repos/"
                                       + app.config["REPOSITORY_OWNER"] + "/"
                                       + app.config["REPOSITORY_NAME"]
                                       + "/git/trees/"
                                       + last_commit_master["tree"]["sha"]
                                       + "?recursive=1")
+        logger.debug(str(last_commit_tree))
 
-        logger.debug(str(last_commit_tree))
-        # point 4
+        # Point 4
         new_tree_data = {"tree": []}
+
+        # First we loop over the existing elements to spot which are modified
+        # and which are untouched and create new blobs when needed
         for element in last_commit_tree["tree"]:
             logger.debug(element)
             if element["type"] == "blob":
@@ -218,7 +231,7 @@
                 if not(
                     element["path"] in [
                         (app.config["CATEGORIES_PATH"] + category["name"])
-                        for category in session.get("deleted_categories", [])
+                        for category in deletion_list
                     ]
                 ):
 
@@ -230,16 +243,15 @@
                     if (
                         element["path"] in [
                             (app.config["CATEGORIES_PATH"] + category["name"])
-                            for category in
-                            session.get("modified_categories", [])
+                            for category in modification_list
                         ]
                     ):
                         # find element in modified categories
-                        for category in session["modified_categories"]:
+                        for category in modification_list:
                             if element["path"] == (
                                 app.config["CATEGORIES_PATH"] + category["name"]
                             ):
-                                # 4-1 for edited files
+                                # 4-1 for modified files, creating a new blob
                                 new_blob_data = {
                                     "content": category["content"],
                                     "encoding": "utf-8"
@@ -254,7 +266,8 @@
                                 blob_sha = new_blob["sha"]
                                 break
 
-                    # this means element is an untouched file
+                    # this means element is an untouched file so we just get
+                    # its sha
                     else:
                         blob_sha = element["sha"]
                     new_tree_data["tree"].append({"path": element["path"],
@@ -262,16 +275,17 @@
                                                   "type": "blob",
                                                   "sha": blob_sha})
         logger.debug(str(new_tree_data["tree"]))
-        # Now we'll add new files in the tree
-        for category in session.get("modified_categories", []):
+        # Now we loop over modified categories to find the ones that don't
+        # exist yet in the last commit tree in order to create blobs for them
+        for category in modification_list:
             logger.debug(app.config["CATEGORIES_PATH"]+category["name"]
                          + " should not be in "
                          + str([elt["path"] for
                                 elt in last_commit_tree["tree"]]))
-            if (app.config["CATEGORIES_PATH"]+category["name"] not in
+            if (app.config["CATEGORIES_PATH"] + category["name"] not in
                     [file["path"] for file in last_commit_tree["tree"]]):
 
-                # 4-1 for added files
+                # 4-1 for added files, creating a new blob
                 new_blob_data = {"content": category["content"],
                                  "encoding": "utf-8"}
                 new_blob = github.post("repos/"
@@ -294,7 +308,7 @@
                                         + "/git/trees",
                                         data=new_tree_data)
 
-        # point 5
+        # Point 5
         new_commit_data = {"message": kwargs["message"],
                            "parents": [last_commit_master["sha"]],
                            "tree": new_tree_response["sha"]}
@@ -306,7 +320,7 @@
                                  data=new_commit_data)
         logger.debug(str(new_commit))
 
-        # point 6
+        # Point 6
         new_head_data = {"sha": new_commit["sha"], "force": "true"}
         logger.debug(str(new_head_data))
         new_head = github.patch("repos/"
@@ -315,31 +329,6 @@
                                 + "/git/refs/heads/master",
                                 data=json.dumps(new_head_data))
         logger.debug(str(new_head))
-        session["deleted_categories"] = []
-        session["modified_categories"] = []
-
-    def save(self, **kwargs):
-        """
-            Saves given file to the session dict
-
-            Expected kwargs should be:
-            * name: the name of the file to save
-            * content: the content of the file to save
-        """
-        session["modified_categories"][:] = [
-            elt for elt in session.get("modified_categories", [])
-            if elt["name"] != kwargs["name"]
-        ]
-        session["modified_categories"].append(
-            {"name": kwargs["name"],
-             "content": str(kwargs["content"], "utf-8")}
-        )
-        # Now we must clean the deleted categories list in case the modified
-        # category was deleted before being edited
-
-        for element in session.get("deleted_categories", []):
-            if element["name"] == kwargs["name"]:
-                session["deleted_categories"].remove(element)
 
     def load(self, **kwargs):
         """
@@ -363,22 +352,13 @@
     def delete(self, **kwargs):
         """
             Deletes from a Github repository
-            Calling delete for a file already deleted will restore it
-            (by deleting it from the delete files list)
+
+            Expected kwargs are:
+            * name : the name of the file to delete
+            * message : the commit message for the deletion
+
         """
-        if (kwargs["name"] in [
-            element["name"] for element in session.get("deleted_categories", [])
-        ]):
-            session["deleted_categories"].remove({"name": kwargs["name"]})
-            # warning, not safe if 2 files share the same name (or category id)
-            # but that shouldn't happen
-        else:
-            session["deleted_categories"].append({"name": kwargs["name"]})
-            # now we must clean the modified categories list in case the
-            # deleted category was modified before
-            for element in session.get("modified_categories", []):
-                if element["name"] == kwargs["name"]:
-                    session["modified_categories"].remove(element)
+        pass
 
     def list(self, **kwargs):
         """