Properly escape multi-segment path parameters

.codegen/impl.go.tmpl

@@ -9,6 +9,7 @@ import (
 	"io"
 	"net/http"
 	"github.com/databricks/databricks-sdk-go/client"
+	"github.com/databricks/databricks-sdk-go/httpclient"
 	{{range .ImportedPackages}}
 	"github.com/databricks/databricks-sdk-go/service/{{.}}"{{end}}
 )
@@ -21,16 +22,29 @@ type {{.CamelName}}Impl struct {
 {{range .Methods}}
 func (a *{{.Service.CamelName}}Impl) {{.PascalName}}(ctx context.Context{{if .Request}}, request {{.Request.PascalName}}{{end}}) {{ template "response-type" . }} {
 	{{- template "response-var" . }}
-	path := {{if .PathParts -}}
-		fmt.Sprintf("{{range  .PathParts}}{{.Prefix}}{{if or .Field .IsAccountId}}%v{{end}}{{ end }}"{{ range .PathParts }}{{if .Field}}, request.{{.Field.PascalName}}{{ else if .IsAccountId }}, a.client.ConfiguredAccountID(){{end}}{{ end }})
-	{{- else}}"{{.Path}}"{{end}}
+	path := {{ template "path" . }}
 	{{ template "make-header" . }}
 	err := a.client.Do(ctx, http.Method{{.TitleVerb}}, path, headers, {{ template "request-param" . }}, {{if .Response}}&{{.Response.CamelName}}{{else}}nil{{end}})
 	return {{ template "response" . }}
 }
 {{end -}}
 {{end}}
 
+{{- define "path" -}}
+{{- if .PathParts -}}
+	fmt.Sprintf("{{range  .PathParts -}}
+	    {{.Prefix}}{{if or .Field .IsAccountId}}%v{{end}}{{ end }}"
+	    {{- range .PathParts -}}
+	        {{- if and .Field .Field.IsPathMultiSegment}}, httpclient.EncodeMultiSegmentPathParameter(request.{{.Field.PascalName}})
+	        {{- else if .Field}}, request.{{.Field.PascalName}}
+			{{- else if .IsAccountId }}, a.client.ConfiguredAccountID()
+			{{- end -}}
+	    {{- end -}})
+{{- else -}}
+	"{{.Path}}"
+{{- end -}}
+{{- end -}}
+
 {{ define "request-param" -}}
   {{ if or (and .Request (or (eq .Verb "GET") (eq .Verb "DELETE") (eq .Verb "HEAD"))) (and .Operation .Operation.RequestBody) -}}
     request{{ if .RequestBodyField }}.{{.RequestBodyField.PascalName}}{{end}}

httpclient/request.go

@@ -109,6 +109,19 @@ func makeQueryString(data interface{}) (string, error) {
 	return "", fmt.Errorf("unsupported query string data: %#v", data)
 }
 
+func EncodeMultiSegmentPathParameter(p string) string {
+	segments := strings.Split(p, "/")
+	b := strings.Builder{}
+	b.Grow(len(p))
+	for i, s := range segments {
+		if i > 0 {
+			b.WriteString("/")
+		}
+		b.WriteString(url.PathEscape(s))
+	}
+	return b.String()
+}
+
 func makeRequestBody(method string, requestURL *string, data interface{}) (common.RequestBody, error) {
 	if data == nil {
 		return common.RequestBody{}, nil

httpclient/request_test.go

@@ -10,6 +10,7 @@ import (
 	"strings"
 	"testing"
 
+	"github.com/stretchr/testify/assert"
 	"github.com/stretchr/testify/require"
 	"golang.org/x/oauth2"
 )
@@ -118,3 +119,10 @@ func TestWithTokenSource(t *testing.T) {
 	require.NoError(t, err)
 	require.Equal(t, "awesome token", buf.String())
 }
+
+func TestEncodeMultiSegmentPathParameter(t *testing.T) {
+	// Slashes should not be encoded.
+	assert.Equal(t, "a/b/c", EncodeMultiSegmentPathParameter("a/b/c"))
+	// # and ? should be encoded.
+	assert.Equal(t, "a%23b%3Fc", EncodeMultiSegmentPathParameter("a#b?c"))
+}

internal/files_test.go

@@ -29,10 +29,10 @@ func (buf hashable) Hash() uint32 {
 	return h.Sum32()
 }
 
-func TestAccUCUploadAndDownloadFilesAPI(t *testing.T) {
+func TestUcAccFilesUploadAndDownload(t *testing.T) {
 	ctx, w, volume := setupUCVolume(t)
 
-	filePath := RandomName("/Volumes/" + volume.CatalogName + "/" + volume.SchemaName + "/" + volume.Name + "/files-")
+	filePath := RandomName("/Volumes/" + volume.CatalogName + "/" + volume.SchemaName + "/" + volume.Name + "/files-with-?-and-#-")
 	err := w.Files.Upload(ctx, files.UploadRequest{
 		FilePath: filePath,
 		Contents: io.NopCloser(strings.NewReader("abcd")),
@@ -57,7 +57,7 @@ func TestAccUCUploadAndDownloadFilesAPI(t *testing.T) {
 	assert.Equal(t, "abcd", string(contents))
 }
 
-func TestAccUCDeleteFile(t *testing.T) {
+func TestUcAccFilesDelete(t *testing.T) {
 	ctx, w, volume := setupUCVolume(t)
 
 	filePath := RandomName("/Volumes/" + volume.CatalogName + "/" + volume.SchemaName + "/" + volume.Name + "/file-")
@@ -67,7 +67,7 @@ func TestAccUCDeleteFile(t *testing.T) {
 	require.NoError(t, err)
 }
 
-func TestAccUCGetMetadata(t *testing.T) {
+func TestUcAccFilesGetMetadata(t *testing.T) {
 	ctx, w, volume := setupUCVolume(t)
 
 	filePath := RandomName("/Volumes/" + volume.CatalogName + "/" + volume.SchemaName + "/" + volume.Name + "/file-")
@@ -81,14 +81,14 @@ func TestAccUCGetMetadata(t *testing.T) {
 	assert.NotEmpty(t, metadata.LastModified)
 }
 
-func TestAccUCCreateDirectory(t *testing.T) {
+func TestUcAccFilesCreateDirectory(t *testing.T) {
 	ctx, w, volume := setupUCVolume(t)
 
 	directoryPath := RandomName("/Volumes/" + volume.CatalogName + "/" + volume.SchemaName + "/" + volume.Name + "/directory-")
 	require.NoError(t, createDirectory(t, ctx, w, directoryPath))
 }
 
-func TestAccUCListDirectoryContents(t *testing.T) {
+func TestUcAccFilesListDirectoryContents(t *testing.T) {
 	ctx, w, volume := setupUCVolume(t)
 
 	directoryPath := RandomName("/Volumes/" + volume.CatalogName + "/" + volume.SchemaName + "/" + volume.Name + "/directory-")
@@ -102,7 +102,7 @@ func TestAccUCListDirectoryContents(t *testing.T) {
 	assert.Len(t, response, 3)
 }
 
-func TestAccUCDeleteDirectory(t *testing.T) {
+func TestUcAccFilesDeleteDirectory(t *testing.T) {
 	ctx, w, volume := setupUCVolume(t)
 
 	directoryPath := RandomName("/Volumes/" + volume.CatalogName + "/" + volume.SchemaName + "/" + volume.Name + "/directory-")
@@ -112,7 +112,7 @@ func TestAccUCDeleteDirectory(t *testing.T) {
 	assert.NoError(t, err)
 }
 
-func TestAccUCGetDirectoryMetadata(t *testing.T) {
+func TestUcAccFilesGetDirectoryMetadata(t *testing.T) {
 	ctx, w, volume := setupUCVolume(t)
 
 	directoryPath := RandomName("/Volumes/" + volume.CatalogName + "/" + volume.SchemaName + "/" + volume.Name + "/directory-")

openapi/code/entity.go

@@ -10,14 +10,15 @@ import (
 // Field of a Type (Entity)
 type Field struct {
 	Named
-	Required bool
-	Entity   *Entity
-	Of       *Entity
-	IsJson   bool
-	IsPath   bool
-	IsQuery  bool
-	IsHeader bool
-	Schema   *openapi.Schema
+	Required           bool
+	Entity             *Entity
+	Of                 *Entity
+	IsJson             bool
+	IsPath             bool
+	IsPathMultiSegment bool
+	IsQuery            bool
+	IsHeader           bool
+	Schema             *openapi.Schema
 }
 
 func (f *Field) IsOptionalObject() bool {

openapi/code/service.go

@@ -149,11 +149,12 @@ func (svc *Service) getDescription(param openapi.Parameter) string {
 func (svc *Service) paramToField(op *openapi.Operation, param openapi.Parameter) *Field {
 	named := Named{param.Name, svc.getDescription(param)}
 	return &Field{
-		Named:    named,
-		Required: param.Required,
-		IsPath:   param.In == "path",
-		IsQuery:  param.In == "query",
-		IsHeader: param.In == "header",
+		Named:              named,
+		Required:           param.Required,
+		IsPath:             param.In == "path",
+		IsPathMultiSegment: param.MultiSegment,
+		IsQuery:            param.In == "query",
+		IsHeader:           param.In == "header",
 		Entity: svc.Package.schemaToEntity(param.Schema, []string{
 			op.Name(),
 			named.PascalName(),
@@ -254,6 +255,7 @@ func (svc *Service) addParams(request *Entity, op *openapi.Operation, params []o
 			field = param
 		}
 		field.IsPath = param.IsPath
+		field.IsPathMultiSegment = param.IsPathMultiSegment
 		field.IsQuery = param.IsQuery
 		field.IsHeader = param.IsHeader
 		request.fields[param.Name] = field