Properly escape multi-segment path parameters (#596)

## Changes
Ports databricks/databricks-sdk-go#869 to Python
SDK. Subsumes #592.

Currently, path parameters are directly interpolated into the request
URL without escaping. This means that characters like /, ? and # will
not be percent-encoded and will affect the semantics of the URL,
starting a new path segment, query parameters, or fragment,
respectively. This means that it is impossible for users of the Files
API to upload/download objects that contain ? or # in their name. / is
allowed in the path of the Files API, so it does not need to be escaped.

The Files API is currently marked with x-databricks-multi-segment,
indicating that it should be permitted to have / characters but other
characters need to be percent-encoded. This PR implements this.

## Tests

- [x] Unit test for multi-segment path escaping behavior.
- [x] Updated integration test to use # and ? symbols in the file name.
This failed on main but succeeded on this PR.

Author: mgyucht

.codegen/service.py.tmpl

@@ -9,7 +9,7 @@ import time
 import random
 import logging
 from ..errors import OperationTimeout, OperationFailed
-from ._internal import _enum, _from_dict, _repeated_dict, _repeated_enum, Wait
+from ._internal import _enum, _from_dict, _repeated_dict, _repeated_enum, Wait, _escape_multi_segment_path_parameter
 
 _LOG = logging.getLogger('databricks.sdk')
 
@@ -325,9 +325,7 @@ class {{.PascalName}}API:{{if .Description}}
 
 {{define "method-do" -}}
 self._api.do('{{.Verb}}',
-  {{if .PathParts -}}
-      f'{{range  .PathParts}}{{.Prefix}}{{if .Field}}{{"{"}}{{template "safe-snake-name" .Field}}{{with .Field.Entity.Enum}}.value{{end}}{{"}"}}{{else if .IsAccountId}}{{"{self._api.account_id}"}}{{end}}{{ end }}'
-  {{- else}}'{{.Path}}'{{end}}
+  {{ template "path" . }}
   {{if .Request}}
       {{- if .Request.HasQueryField}}, query=query{{end}}
       {{- if .Request.MapValue}}, body=contents
@@ -341,6 +339,31 @@ self._api.do('{{.Verb}}',
   {{- if .IsResponseByteStream }}, raw=True{{ end }})
 {{- end}}
 
+{{- define "path" -}}
+{{- if .PathParts -}}
+  f'{{range  .PathParts -}}
+    {{- .Prefix -}}
+    {{- if .Field -}}
+      {{- "{" -}}
+      {{- if .Field.IsPathMultiSegment -}}_escape_multi_segment_path_parameter({{ template "path-parameter" . }})
+      {{- else -}}{{ template "path-parameter" . }}
+      {{- end -}}
+      {{- "}" -}}
+    {{- else if .IsAccountId}}
+      {{- "{" -}}
+      self._api.account_id
+      {{- "}" -}}
+    {{- end -}}
+  {{- end }}'
+{{- else -}}
+  '{{.Path}}'
+{{- end -}}
+{{- end -}}
+
+{{- define "path-parameter" -}}
+  {{template "safe-snake-name" .Field}}{{with .Field.Entity.Enum}}.value{{end}}
+{{- end -}}
+
 {{define "method-return-type" -}}
   {{if and .Wait (and (not .IsCrudRead) (not (eq .SnakeName "get_run"))) }} -> Wait[{{.Wait.Poll.Response.PascalName}}]
   {{- else if not .Response.IsEmpty }} -> {{if .Response.ArrayValue -}}

databricks/sdk/service/_internal.py

@@ -1,4 +1,5 @@
 import datetime
+import urllib.parse
 from typing import Callable, Dict, Generic, Optional, Type, TypeVar
 
 
@@ -38,6 +39,10 @@ def _repeated_enum(d: Dict[str, any], field: str, cls: Type) -> any:
     return res
 
 
+def _escape_multi_segment_path_parameter(param: str) -> str:
+    return urllib.parse.quote(param)
+
+
 ReturnType = TypeVar('ReturnType')
 
 

databricks/sdk/service/files.py

@@ -219,7 +219,7 @@ def test_files_api_upload_download(ucws, random):
     with ResourceWithCleanup.create_schema(w, 'main', schema):
         with ResourceWithCleanup.create_volume(w, 'main', schema, volume):
             f = io.BytesIO(b"some text data")
-            target_file = f'/Volumes/main/{schema}/{volume}/filesit-{random()}.txt'
+            target_file = f'/Volumes/main/{schema}/{volume}/filesit-with-?-and-#-{random()}.txt'
             w.files.upload(target_file, f)
             with w.files.download(target_file).contents as f:
                 assert f.read() == b"some text data"

tests/integration/test_files.py

@@ -1,8 +1,9 @@
 from dataclasses import dataclass
 from enum import Enum
 
-from databricks.sdk.service._internal import (_enum, _from_dict,
-                                              _repeated_dict, _repeated_enum)
+from databricks.sdk.service._internal import (
+    _enum, _escape_multi_segment_path_parameter, _from_dict, _repeated_dict,
+    _repeated_enum)
 
 
 class A(Enum):
@@ -41,3 +42,10 @@ def test_from_dict():
 
 def test_repeated_dict():
     assert _repeated_dict({'x': [{'field': 'a'}, {'field': 'c'}]}, 'x', B) == [B('a'), B('c')]
+
+
+def test_escape_multi_segment_path_parameter():
+    assert _escape_multi_segment_path_parameter('a b') == 'a%20b'
+    assert _escape_multi_segment_path_parameter('a/b') == 'a/b'
+    assert _escape_multi_segment_path_parameter('a?b') == 'a%3Fb'
+    assert _escape_multi_segment_path_parameter('a#b') == 'a%23b'