Add CLI compatibility for TechDocs spec; fix failing integration tests

.gitignore

@@ -13,3 +13,4 @@ test/.env
 .tmp*
 MANIFEST
 venv
+openapi*.yaml

linodecli/__init__.py

@@ -51,7 +51,11 @@
     or TEST_MODE
 )
 
-cli = CLI(VERSION, handle_url_overrides(BASE_URL), skip_config=skip_config)
+cli = CLI(
+    VERSION,
+    handle_url_overrides(BASE_URL, override_path=True),
+    skip_config=skip_config,
+)
 
 
 def main():  # pylint: disable=too-many-branches,too-many-statements

linodecli/api_request.py

@@ -13,7 +13,7 @@
 from requests import Response
 
 from linodecli.exit_codes import ExitCodes
-from linodecli.helpers import API_CA_PATH
+from linodecli.helpers import API_CA_PATH, API_VERSION_OVERRIDE
 
 from .baked.operation import (
     ExplicitEmptyListValue,
@@ -185,14 +185,22 @@ def _build_filter_header(
 
 
 def _build_request_url(ctx, operation, parsed_args) -> str:
-    target_server = handle_url_overrides(
+    url_base = handle_url_overrides(
         operation.url_base,
         host=ctx.config.get_value("api_host"),
-        version=ctx.config.get_value("api_version"),
         scheme=ctx.config.get_value("api_scheme"),
     )
 
-    result = f"{target_server}{operation.url_path}".format(**vars(parsed_args))
+    result = f"{url_base}{operation.url_path}".format(
+        # {apiVersion} is defined in the endpoint paths for
+        # the TechDocs API specs
+        apiVersion=(
+            API_VERSION_OVERRIDE
+            or ctx.config.get_value("api_version")
+            or operation.default_api_version
+        ),
+        **vars(parsed_args),
+    )
 
     if operation.method == "get":
         result += f"?page={ctx.page}&page_size={ctx.page_size}"

linodecli/baked/operation.py

@@ -11,10 +11,11 @@
 from collections import defaultdict
 from getpass import getpass
 from os import environ, path
-from typing import Any, Dict, List, Tuple
+from typing import Any, Dict, List, Optional, Tuple
+from urllib.parse import urlparse
 
 import openapi3.paths
-from openapi3.paths import Operation
+from openapi3.paths import Operation, Parameter
 
 from linodecli.baked.request import OpenAPIFilteringRequest, OpenAPIRequest
 from linodecli.baked.response import OpenAPIResponse
@@ -295,7 +296,9 @@ class OpenAPIOperation:
     This is the class that should be pickled when building the CLI.
     """
 
-    def __init__(self, command, operation: Operation, method, params):
+    def __init__(
+        self, command, operation: Operation, method, params
+    ):  # pylint: disable=too-many-locals,too-many-branches,too-many-statements
         """
         Wraps an openapi3.Operation object and handles pulling out values relevant
         to the Linode CLI.
@@ -355,18 +358,17 @@ def __init__(self, command, operation: Operation, method, params):
 
         self.summary = operation.summary
         self.description = operation.description.split(".")[0]
-        self.params = [OpenAPIOperationParameter(c) for c in params]
 
-        # These fields must be stored separately
-        # to allow them to be easily modified
-        # at runtime.
-        self.url_base = (
-            operation.servers[0].url
-            if operation.servers
-            else operation._root.servers[0].url
-        )
+        # The apiVersion attribute should not be specified as a positional argument
+        self.params = [
+            OpenAPIOperationParameter(param)
+            for param in params
+            if param.name not in {"apiVersion"}
+        ]
 
-        self.url_path = operation.path[-2]
+        self.url_base, self.url_path, self.default_api_version = (
+            self._get_api_url_components(operation, params)
+        )
 
         self.url = self.url_base + self.url_path
 
@@ -410,6 +412,85 @@ def _flatten_url_path(tag: str) -> str:
         new_tag = re.sub(r"[^a-z ]", "", new_tag).replace(" ", "-")
         return new_tag
 
+    @staticmethod
+    def _resolve_api_version(
+        params: List[Parameter], server_url: str
+    ) -> Optional[str]:
+        """
+        Returns the API version for a given list of params and target URL.
+
+        :param params: The params for this operation's endpoint path.
+        :type params: List[Parameter]
+        :param server_url: The URL of server for this operation.
+        :type server_url: str
+
+        :returns: The default API version if the URL has a version, else None.
+        :rtype: Optional[str]
+        """
+
+        # Remove empty segments from the URL path, stripping the first,
+        # last and any duplicate slashes if necessary.
+        # There shouldn't be a case where this is needed, but it's
+        # always good to make things more resilient :)
+        url_path_segments = [
+            seg for seg in urlparse(server_url).path.split("/") if len(seg) > 0
+        ]
+        if len(url_path_segments) > 0:
+            return "/".join(url_path_segments)
+
+        version_param = next(
+            (
+                param
+                for param in params
+                if param.name == "apiVersion" and param.in_ == "path"
+            ),
+            None,
+        )
+        if version_param is not None:
+            return version_param.schema.default
+
+        return None
+
+    @staticmethod
+    def _get_api_url_components(
+        operation: Operation, params: List[Parameter]
+    ) -> Tuple[str, str, str]:
+        """
+        Returns the URL components for a given operation.
+
+        :param operation: The operation to get the URL components for.
+        :type operation: Operation
+        :param params: The parameters for this operation's route.
+        :type params: List[Parameter]
+
+        :returns: The base URL, path, and default API version of the operation.
+        :rtype: Tuple[str, str, str]
+        """
+
+        url_server = (
+            operation.servers[0].url
+            if operation.servers
+            # pylint: disable-next=protected-access
+            else operation._root.servers[0].url
+        )
+
+        url_base = urlparse(url_server)._replace(path="").geturl()
+        url_path = operation.path[-2]
+
+        api_version = OpenAPIOperation._resolve_api_version(params, url_server)
+        if api_version is None:
+            raise ValueError(
+                f"Failed to resolve API version for operation {operation}"
+            )
+
+        # The apiVersion is only specified in the new-style OpenAPI spec,
+        # so we need to manually insert it into the path to maintain
+        # backwards compatibility
+        if "{apiVersion}" not in url_path:
+            url_path = "/{apiVersion}" + url_path
+
+        return url_base, url_path, api_version
+
     def process_response_json(
         self, json: Dict[str, Any], handler: OutputHandler
     ):  # pylint: disable=redefined-outer-name

linodecli/helpers.py

@@ -19,17 +19,23 @@
 
 
 def handle_url_overrides(
-    url: str, host: str = None, version: str = None, scheme: str = None
+    url: str,
+    host: str = None,
+    version: str = None,
+    scheme: str = None,
+    override_path=False,
 ):
     """
     Returns the URL with the API URL environment overrides applied.
+    If override_path is True and the API version env var is specified,
+    the URL path will be updated accordingly.
     """
 
     parsed_url = urlparse(url)
 
     overrides = {
         "netloc": API_HOST_OVERRIDE or host,
-        "path": API_VERSION_OVERRIDE or version,
+        "path": (API_VERSION_OVERRIDE or version) if override_path else None,
         "scheme": API_SCHEME_OVERRIDE or scheme,
     }
 

linodecli/output/output_handler.py

@@ -4,6 +4,7 @@
 
 import copy
 import json
+import sys
 from argparse import Namespace
 from enum import Enum, auto
 from sys import stdout
@@ -243,8 +244,8 @@ def _get_columns(self, attrs, max_depth=1):
             for col in self.columns.split(","):
                 for attr in attrs:
                     # Display this column if the format string
-                    # matches the column_name or path of this column
-                    if col in (attr.column_name, attr.name):
+                    # matches the path of this column
+                    if col == attr.name:
                         attrs.remove(attr)
                         columns.append(attr)
 
@@ -444,7 +445,8 @@ def configure(
                 print(
                     "WARNING: '--all' is a deprecated flag, "
                     "and will be removed in a future version. "
-                    "Please consider use '--all-columns' instead."
+                    "Please consider use '--all-columns' instead.",
+                    file=sys.stderr,
                 )
             self.columns = "*"
         elif parsed.format:

tests/fixtures/api_request_test_foobar_get.yaml

@@ -4,7 +4,6 @@ info:
   version: 1.0.0
 servers:
   - url: http://localhost/v4
-
 paths:
   /foo/bar:
     get:

tests/fixtures/api_request_test_foobar_post.yaml

@@ -3,7 +3,7 @@ info:
   title: API Specification
   version: 1.0.0
 servers:
-  - url: http://localhost
+  - url: http://localhost/v4
 
 paths:
   /foo/bar:

tests/fixtures/api_url_components_test.yaml

@@ -0,0 +1,48 @@
+openapi: 3.0.1
+info:
+  title: API Specification
+  version: 1.0.0
+servers:
+  - url: http://localhost/v19
+paths:
+  /foo/bar:
+    get:
+      operationId: fooBarGet
+      responses:
+        '200':
+          description: foobar
+          content:
+            application/json: {}
+    delete:
+      operationId: fooBarDelete
+      servers:
+        - url: http://localhost/v12beta
+      responses:
+        '200':
+          description: foobar
+          content:
+            application/json: {}
+  /{apiVersion}/bar/foo:
+    parameters:
+    - name: apiVersion
+      in: path
+      required: true
+      schema:
+        type: string
+        default: v9canary
+    get:
+      operationId: barFooGet
+      responses:
+        '200':
+          description: foobar
+          content:
+            application/json: {}
+    post:
+      operationId: barFooPost
+      servers:
+        - url: http://localhost/v100beta
+      responses:
+        '200':
+          description: foobar
+          content:
+            application/json: {}

tests/fixtures/output_test_get.yaml

@@ -3,7 +3,7 @@ info:
   title: API Specification
   version: 1.0.0
 servers:
-  - url: http://localhost
+  - url: http://localhost/v4
 
 paths:
   /foo/bar:

tests/fixtures/overrides_test_get.yaml

@@ -3,7 +3,7 @@ info:
   title: API Specification
   version: 1.0.0
 servers:
-  - url: http://localhost
+  - url: http://localhost/v4
 
 paths:
   /foo/bar:

tests/fixtures/response_test_get.yaml

@@ -3,7 +3,7 @@ info:
   title: API Specification
   version: 1.0.0
 servers:
-  - url: http://localhost
+  - url: http://localhost/v4
 
 paths:
   /foo/bar:

tests/fixtures/subtable_test_get.yaml

@@ -3,7 +3,7 @@ info:
   title: API Specification
   version: 1.0.0
 servers:
-  - url: http://localhost
+  - url: http://localhost/v4
 
 paths:
   /foo/bar:

tests/integration/account/test_account.py

@@ -244,7 +244,7 @@ def get_user_id():
                 "--delimiter",
                 ",",
                 "--format",
-                "id",
+                "username",
             ]
         )
         .stdout.decode()