Fixing lots of issues

bananas_api/helpers/api_schema.py

@@ -52,6 +52,17 @@ def normalize_message(exception):
     return errors
 
 
+class ValidateURL(validate.URL):
+    def __call__(self, value):
+        if not value:
+            return value
+
+        if len(value) > 95:
+            raise ValidationError("Longer than maximum length 95.")
+
+        return super().__call__(value)
+
+
 class OrderedSchema(Schema):
     class Meta:
         ordered = True
@@ -69,7 +80,7 @@ class Global(OrderedSchema):
     archived = fields.Boolean()
     replaced_by = fields.Nested(ReplacedBy(), data_key="replaced-by", allow_none=True)
     description = fields.String(validate=validate.Length(max=511))
-    url = fields.String(validate=validate.Length(max=95))
+    url = fields.String(validate=ValidateURL())
     tags = fields.List(fields.String(validate=validate.Length(max=31)))
 
 

bananas_api/helpers/content_save.py

@@ -9,14 +9,15 @@
     import_module,
 )
 
+from ..helpers.content_storage import get_indexed_package
 from ..index.local import click_index_local
 from ..index.github import click_index_github
 
 log = logging.getLogger(__name__)
 
 TIMER_TIMEOUT = 60 * 5
 
-_pending_changes = defaultdict(list)
+_pending_changes = defaultdict(set)
 _timer = defaultdict(lambda: None)
 _index_instance = None
 
@@ -33,10 +34,12 @@ def store_on_disk(user, package=None):
         _store_on_disk_safe(package, user.display_name)
 
     while _pending_changes[user.full_id]:
-        package = _pending_changes[user.full_id].pop()
+        content_type, unique_id = _pending_changes[user.full_id].pop()
+        package = get_indexed_package(content_type, unique_id)
+
         _store_on_disk_safe(package, user.display_name)
 
-    _index_instance.commit()
+    _index_instance.push_changes()
 
 
 async def _timer_handler(user):
@@ -47,7 +50,7 @@ async def _timer_handler(user):
 
 
 def queue_store_on_disk(user, package):
-    _pending_changes[user.full_id].append(package)
+    _pending_changes[user.full_id].add((package["content_type"], package["unique_id"]))
 
     # Per user, start a timer. If it expires, we push the update. This
     # allows a user to take a bit of time to get its edits right, before we

bananas_api/index/common_disk.py

@@ -75,7 +75,7 @@ class Index:
     def __init__(self, folder):
         self.folder = folder
         self.files = []
-        self.changes = []
+        self.change = None
 
     def _read_content_entry(self, content_type, folder_name, unique_id):
         folder_name = f"{folder_name}/{unique_id}"
@@ -118,6 +118,12 @@ def reload(self):
         clear_indexed_packages()
         self.load_all()
 
+    def commit(self):
+        pass
+
+    def push_changes(self):
+        pass
+
     def load_all(self):
         # Because we are loaded the content, there is no way to already do
         # dependency validation. So for now, disable it. After we loaded
@@ -169,7 +175,8 @@ def load_all(self):
                 raise Exception("Failed to load content entries: %r" % errors)
 
     def store_package(self, package, display_name):
-        self.changes.append(f"{package['content_type'].value}/{package['unique_id']} (by {display_name})")
+        self.change = f"{package['content_type'].value}/{package['unique_id']} (by {display_name})"
+
         path = f"{package['content_type'].value}/{package['unique_id']}"
 
         os.makedirs(f"{self.folder}/{path}/versions", exist_ok=True)
@@ -203,3 +210,5 @@ def store_package(self, package, display_name):
                 if data_overwrite:
                     fp.write("\n")
                     fp.write(yaml_dump(data_overwrite))
+
+        self.commit()

bananas_api/index/github.py

@@ -57,8 +57,8 @@ def reload(self):
         self._fetch_latest()
         super().reload()
 
-    def commit(self):
-        super().commit()
+    def push_changes(self):
+        super().push_changes()
 
         if not self._ssh_command:
             log.error("No GitHub private key supplied; cannot push to GitHub.")

bananas_api/index/local.py

@@ -20,25 +20,38 @@ def _prepare_git(self):
         try:
             self._git = git.Repo(self.folder)
         except git.exc.NoSuchPathError:
-            self._git = git.Repo.init(self.folder)
+            self._init_repository()
         except git.exc.InvalidGitRepositoryError:
-            self._git = git.Repo.init(self.folder)
+            self._init_repository()
+
+    def _init_repository(self):
+        self._git = git.Repo.init(self.folder)
+        # Always make sure there is a commit in the working tree, otherwise
+        # HEAD is invalid, which results in other nasty problems.
+        self._git.index.commit(
+            "Add: initial empty commit", author=self._git_author, committer=self._git_author,
+        )
 
     def commit(self):
         files = self.files[:]
-        changes = self.changes[:]
         self.files = []
-        self.changes = []
+
+        change = self.change
+        self.change = None
 
         for filename in files:
             self._git.index.add(filename)
 
-        commit_message = "".join([f"\n - {change}" for change in changes])
+        # Check if there was anything to commit; possibly someone changed an
+        # edit back to the original, meaning we are about to commit an empty
+        # commit. That would be silly of course.
+        if not self._git.index.diff("HEAD"):
+            return
+
+        commit_message = f"Update: {change}"
 
         self._git.index.commit(
-            f"Update: changes made via content-api\n{commit_message}",
-            author=self._git_author,
-            committer=self._git_author,
+            commit_message, author=self._git_author, committer=self._git_author,
         )
 
 

bananas_api/new_upload/session.py

@@ -68,7 +68,7 @@ def reset_session_timer(session, first_time=False):
 
         _timer[session["user"].full_id].cancel()
 
-    # Per user, start a timer. If it expires, we remove the cnontent. This
+    # Per user, start a timer. If it expires, we remove the content. This
     # means that if a user failed to publish within a reasonable amount of
     # time, we reclaim the diskspace.
     loop = asyncio.get_event_loop()
@@ -79,7 +79,7 @@ def reset_session_timer(session, first_time=False):
 @click.option(
     "--cleanup-graceperiod",
     help="Graceperiod between cleanup of new uploads.",
-    default=60 * 15,
+    default=60 * 60 * 14,
     show_default=True,
     metavar="SECONDS",
 )
@@ -216,7 +216,14 @@ def update_session(session, data):
 
     for key, value in data.items():
         if isinstance(value, str):
-            session[key] = value.strip()
+            value = value.strip()
+            session[key] = value
+
+            # Setting an empty string means: use the one from global. In case
+            # it is for a setting that should not be empty, validation will
+            # fail.
+            if value == "":
+                del session[key]
         else:
             session[key] = value
 

bananas_api/web_routes/discover.py

@@ -59,9 +59,8 @@ async def package_by_unique_id(request):
     content_type = in_path_content_type(request.match_info["content_type"])
     unique_id = in_path_unique_id(request.match_info["unique_id"])
 
-    try:
-        package = get_indexed_package(content_type, unique_id)
-    except KeyError:
+    package = get_indexed_package(content_type, unique_id)
+    if not package:
         return web.HTTPNotFound()
 
     package = Package().dump(package)
@@ -74,9 +73,8 @@ async def package_by_upload_date(request):
     unique_id = in_path_unique_id(request.match_info["unique_id"])
     upload_date = in_path_upload_date(request.match_info["upload_date"])
 
-    try:
-        version = get_indexed_version(content_type, unique_id, upload_date)
-    except KeyError:
+    version = get_indexed_version(content_type, unique_id, upload_date)
+    if not version:
         return web.HTTPNotFound()
 
     # Copy and add two fields to convert VersionMinimized to Version

bananas_api/web_routes/update.py

@@ -27,9 +27,8 @@ async def package_update(request):
     unique_id = in_path_unique_id(request.match_info["unique_id"])
     user = in_header_authorization(request.headers)
 
-    try:
-        package = get_indexed_package(content_type, unique_id)
-    except KeyError:
+    package = get_indexed_package(content_type, unique_id)
+    if not package:
         return web.HTTPNotFound()
 
     for author in package["authors"]:
@@ -45,9 +44,25 @@ async def package_update(request):
             {"message": "request body failed validation", "errors": normalize_message(e)}, status=400
         )
 
+    # The only field you are not allowed to make empty (and which you can
+    # change), is "version", so do some extra validation there.
+    if "name" in data and not len(data["name"].strip()):
+        return web.json_response(
+            {"message": "request body failed validation", "errors": {"name": ["Cannot be empty"]}}, status=400
+        )
+
     # Update the record with the changed fields and schedule for commit
     for key, value in data.items():
-        package[key] = value
+        if isinstance(value, str):
+            value = value.strip()
+            package[key] = value
+
+            # Setting an empty string means: use the one from global.
+            if value == "":
+                del package[key]
+        else:
+            package[key] = value
+
     queue_store_on_disk(user, package)
 
     return web.HTTPNoContent()
@@ -60,9 +75,8 @@ async def version_update(request):
     upload_date = in_path_upload_date(request.match_info["upload_date"])
     user = in_header_authorization(request.headers)
 
-    try:
-        package = get_indexed_package(content_type, unique_id)
-    except KeyError:
+    package = get_indexed_package(content_type, unique_id)
+    if not package:
         return web.HTTPNotFound()
 
     for author in package["authors"]:
@@ -71,9 +85,8 @@ async def version_update(request):
     else:
         return web.HTTPNotFound()
 
-    try:
-        version = get_indexed_version(content_type, unique_id, upload_date)
-    except KeyError:
+    version = get_indexed_version(content_type, unique_id, upload_date)
+    if not version:
         return web.HTTPNotFound()
 
     try:
@@ -83,10 +96,22 @@ async def version_update(request):
             {"message": "request body failed validation", "errors": normalize_message(e)}, status=400
         )
 
+    # The only field you are not allowed to make empty (and which you can
+    # change), is "version", so do some extra validation there.
+    if "version" in data and not len(data["version"].strip()):
+        return web.json_response(
+            {"message": "request body failed validation", "errors": {"version": ["Cannot be empty"]}}, status=400
+        )
+
     # Update the record with the changed fields and schedule for commit
     for key, value in data.items():
         if isinstance(value, str):
-            version[key] = value.strip()
+            value = value.strip()
+            version[key] = value
+
+            # Setting an empty string means: use the one from global.
+            if value == "":
+                del version[key]
         else:
             version[key] = value
 

regression/913_oversized_url.yaml

@@ -2,8 +2,8 @@ steps:
 - api: user/login
 - api: new-package/start
 - api: new-package/update
-  url: "123456789012345678901234567890123456789012345678901234567890123456789012345678901234567890123456"
+  url: "http://89.com/5678901234567890123456789012345678901234567890123456789012345678901234567890123456"
   error:
     url: "Longer than maximum length 95."
 - api: new-package/update
-  url: "12345678901234567890123456789012345678901234567890123456789012345678901234567890123456789012345"
+  url: "http://89.com/567890123456789012345678901234567890123456789012345678901234567890123456789012345"