Update spec and fix minor problems in implementation

maubot/management/api/client.py

@@ -13,7 +13,9 @@
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
+from typing import Optional
 from json import JSONDecodeError
+from http import HTTPStatus
 
 from aiohttp import web
 
@@ -43,7 +45,7 @@ async def get_client(request: web.Request) -> web.Response:
     return web.json_response(client.to_dict())
 
 
-async def create_client(user_id: UserID, data: dict) -> web.Response:
+async def _create_client(user_id: Optional[UserID], data: dict) -> web.Response:
     homeserver = data.get("homeserver", None)
     access_token = data.get("access_token", None)
     new_client = MatrixClient(base_url=homeserver, token=access_token, loop=Client.loop,
@@ -54,7 +56,7 @@ async def create_client(user_id: UserID, data: dict) -> web.Response:
         return ErrBadClientAccessToken
     except MatrixRequestError:
         return ErrBadClientAccessDetails
-    if user_id == "new":
+    if user_id is None:
         existing_client = Client.get(mxid, None)
         if existing_client is not None:
             return ErrUserExists
@@ -73,7 +75,7 @@ async def create_client(user_id: UserID, data: dict) -> web.Response:
     return web.json_response(client.to_dict())
 
 
-async def update_client(client: Client, data: dict) -> web.Response:
+async def _update_client(client: Client, data: dict) -> web.Response:
     try:
         await client.update_access_details(data.get("access_token", None),
                                            data.get("homeserver", None))
@@ -89,22 +91,30 @@ async def update_client(client: Client, data: dict) -> web.Response:
     client.enabled = data.get("enabled", client.enabled)
     client.autojoin = data.get("autojoin", client.autojoin)
     client.sync = data.get("sync", client.sync)
-    return web.json_response(client.to_dict())
+    return web.json_response(client.to_dict(), status=HTTPStatus.CREATED)
+
+
+@routes.post("/client/new")
+async def create_client(request: web.Request) -> web.Response:
+    try:
+        data = await request.json()
+    except JSONDecodeError:
+        return ErrBodyNotJSON
+    return await _create_client(None, data)
 
 
 @routes.put("/client/{id}")
 async def update_client(request: web.Request) -> web.Response:
     user_id = request.match_info.get("id", None)
-    # /client/new always creates a new client
+    client = Client.get(user_id, None)
-    client = Client.get(user_id, None) if user_id != "new" else None
     try:
         data = await request.json()
     except JSONDecodeError:
         return ErrBodyNotJSON
     if not client:
-        return await create_client(user_id, data)
+        return await _create_client(user_id, data)
     else:
-        return await update_client(client, data)
+        return await _update_client(client, data)
 
 
 @routes.delete("/client/{id}")

maubot/management/api/instance.py

@@ -14,6 +14,7 @@
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
 from json import JSONDecodeError
+from http import HTTPStatus
 
 from aiohttp import web
 
@@ -40,7 +41,7 @@ async def get_instance(request: web.Request) -> web.Response:
     return web.json_response(instance.to_dict())
 
 
-async def create_instance(instance_id: str, data: dict) -> web.Response:
+async def _create_instance(instance_id: str, data: dict) -> web.Response:
     plugin_type = data.get("type", None)
     primary_user = data.get("primary_user", None)
     if not plugin_type:
@@ -60,10 +61,10 @@ async def create_instance(instance_id: str, data: dict) -> web.Response:
     PluginInstance.db.add(db_instance)
     PluginInstance.db.commit()
     await instance.start()
-    return web.json_response(instance.to_dict())
+    return web.json_response(instance.to_dict(), status=HTTPStatus.CREATED)
 
 
-async def update_instance(instance: PluginInstance, data: dict) -> web.Response:
+async def _update_instance(instance: PluginInstance, data: dict) -> web.Response:
     if not await instance.update_primary_user(data.get("primary_user")):
         return ErrPrimaryUserNotFound
     instance.update_id(data.get("id", None))
@@ -83,9 +84,9 @@ async def update_instance(request: web.Request) -> web.Response:
     except JSONDecodeError:
         return ErrBodyNotJSON
     if not instance:
-        return await create_instance(instance_id, data)
+        return await _create_instance(instance_id, data)
     else:
-        return await update_instance(instance, data)
+        return await _update_instance(instance, data)
 
 
 @routes.delete("/instance/{id}")

maubot/management/api/plugin.py

@@ -13,13 +13,15 @@
 #
 # You should have received a copy of the GNU Affero General Public License
 # along with this program.  If not, see <https://www.gnu.org/licenses/>.
-from aiohttp import web
+from http import HTTPStatus
 from io import BytesIO
 from time import time
 import traceback
 import os.path
 import re
 
+from aiohttp import web
+
 from ...loader import PluginLoader, ZippedPluginLoader, MaubotZipImportError
 from .responses import (ErrPluginNotFound, ErrPluginInUse, plugin_import_error,
                         plugin_reload_error, RespDeleted, RespOK, ErrUnsupportedPluginLoader)
@@ -77,7 +79,7 @@ async def upload_new_plugin(content: bytes, pid: str, version: str) -> web.Respo
     except MaubotZipImportError as e:
         ZippedPluginLoader.trash(path)
         return plugin_import_error(str(e), traceback.format_exc())
-    return web.json_response(plugin.to_dict())
+    return web.json_response(plugin.to_dict(), status=HTTPStatus.CREATED)
 
 
 async def upload_replacement_plugin(plugin: ZippedPluginLoader, content: bytes, new_version: str

maubot/management/api/spec.yaml

@@ -81,12 +81,10 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/Plugin'
+        400:
+          $ref: '#/components/responses/BadRequest'
         401:
           $ref: '#/components/responses/Unauthorized'
-        412:
-          description: >-
-            Instances of the uploaded plugin type are currently active,
-            therefore the plugin can't be updated
       requestBody:
         content:
           application/zip:
@@ -131,6 +129,10 @@ paths:
           $ref: '#/components/responses/PluginNotFound'
         412:
           description: One or more plugin instances of this type exist
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
   /plugin/{id}/reload:
     parameters:
     - name: id
@@ -215,10 +217,16 @@ paths:
           description: Plugin instance edited
         201:
           description: Plugin instance created
+        400:
+          $ref: '#/components/responses/BadRequest'
         401:
           $ref: '#/components/responses/Unauthorized'
         404:
-          $ref: '#/components/responses/InstanceNotFound'
+          description: The referenced client or plugin type could not be found
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
 
   '/clients':
     get:
@@ -236,6 +244,35 @@ paths:
                   $ref: '#/components/schemas/MatrixClient'
         401:
           $ref: '#/components/responses/Unauthorized'
+  /client/new:
+    post:
+      operationId: create_client
+      summary: Create a Matrix client
+      tags: [Clients]
+      requestBody:
+        content:
+          application/json:
+            schema:
+              $ref: '#/components/schemas/MatrixClient'
+      responses:
+        201:
+          description: Client created
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/MatrixClient'
+        400:
+          $ref: '#/components/responses/BadRequest'
+        401:
+          $ref: '#/components/responses/Unauthorized'
+        404:
+          $ref: '#/components/responses/ClientNotFound'
+        409:
+          description: There is already a client with the user ID of that token.
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
   '/client/{id}':
     parameters:
     - name: id
@@ -281,10 +318,10 @@ paths:
             application/json:
               schema:
                 $ref: '#/components/schemas/MatrixClient'
+        400:
+          $ref: '#/components/responses/BadRequest'
         401:
           $ref: '#/components/responses/Unauthorized'
-        404:
-          $ref: '#/components/responses/ClientNotFound'
     delete:
       operationId: delete_client
       summary: Delete a Matrix client
@@ -298,23 +335,58 @@ paths:
           $ref: '#/components/responses/ClientNotFound'
         412:
           description: One or more plugin instances with this as their primary client exist
+          content:
+            application/json:
+              schema:
+                $ref: '#/components/schemas/Error'
 
 components:
   responses:
     Unauthorized:
       description: Invalid or missing access token
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/Error'
     PluginNotFound:
       description: Plugin not found
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/Error'
     ClientNotFound:
       description: Client not found
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/Error'
     InstanceNotFound:
       description: Plugin instance not found
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/Error'
+    BadRequest:
+      description: Bad request (e.g. bad request body)
+      content:
+        application/json:
+          schema:
+            $ref: '#/components/schemas/Error'
   securitySchemes:
     bearer:
       type: http
       scheme: bearer
       description: Required authentication for all endpoints
   schemas:
+    Error:
+      type: object
+      properties:
+        error:
+          type: string
+          description: A human-readable error message
+        errcode:
+          type: string
+          description: A simple error code
     Plugin:
       type: object
       properties: