fix(couchdb): use url.PathEscape in DBExists for consistency with other DB* methods

[dominicbarnes]

Currently, url.PathEncode is applied for CreateDB and DestroyDB, but not for DBExists, which behave inconsistently.

I discovered this when I created a database that included special characters, but DBExists would not correctly detect a database even after being created.

For a reproducible example, I could not find a good test case to update, as the request path is never part of the test assertion. You can reproduce the issue with the following setup:

docker-compose.yml

services:
  couchdb:
    image: couchdb:3
    ports:
      - 5984:5984
    environment:
      COUCHDB_USER: admin
      COUCHDB_PASSWORD: password

go.mod

module kivik-testing

go 1.23.0

require github.com/go-kivik/kivik/v4 v4.3.0

require (
    github.com/google/uuid v1.6.0 // indirect
    golang.org/x/net v0.17.0 // indirect
    golang.org/x/sync v0.4.0 // indirect
)

main.go

package main

import (
    "context"
    "log"
    "net/http"
    "net/url"

    "github.com/go-kivik/kivik/v4"
    _ "github.com/go-kivik/kivik/v4/couchdb"
)

func main() {
    ctx := context.Background()

    couchdb, err := kivik.New("couch", "http://admin:password@localhost:5984")
    if err != nil {
        log.Fatal(err)
    }

    dbName := "foo/bar"

    // ensure database is created
    if err := couchdb.CreateDB(ctx, dbName); err != nil {
        switch kivik.HTTPStatus(err) {
        case http.StatusPreconditionFailed:
            // do nothing, database already exists
        default:
            log.Fatalf("failed to create database %s: %s", dbName, err)
        }
    }

    // check existence with existing kivik
    exists, err := couchdb.DBExists(ctx, dbName)
    if err != nil {
        log.Fatalf("failed to check database %s existence", dbName)
    }
    log.Printf("exists (got %t, expected true)", exists)

    // check existince with hack
    exists, err = couchdb.DBExists(ctx, url.PathEscape(dbName))
    if err != nil {
        log.Fatalf("failed to check database %s existence", dbName)
    }
    log.Printf("exists with hack (got %t, expected true)", exists)
}

Upon running my "test" script, I get the following output:

$ go run .
exists (got false, expected true)
exists with hack (got true, expected true)

This PR includes a fix, which is to use url.PathEscape within DBExists which mirrors CreateDB and DestroyDB and will produce consistent/correct behavior.

It would be nice to validate this fix with a test case, but I don't think the current test suite really has this kind of assertion baked in (it kinda assumes the right URL path is used, since it just returns the configured *http.Response no matter what the request).

[flimzy]

Thank you for catching this and submitting a patch! It would be good to add a test, as well, which would fail prior to the patch, but pass after. Is that something you'd like to add?

[dominicbarnes]

@flimzy yeah, let me throw something together

[dominicbarnes]

@flimzy I just pushed bb18c2a6216ab298554db5c4b8867e77b51e05d1 which adds a test that fails before the change and passes after.

To avoid altering any other tests, this creates a new test helper which creates a client which inspects req.URL.RawPath as part of the HTTP mock, returning an error if there is a mismatch.

This probably isn't the most elegant approach, but it certainly gets the job done short of pulling in a full HTTP mocking library. Let me know what you think, I'm happy to iterate on it further if you have any suggestions.

[dominicbarnes]

@flimzy after looking over some other tests, I found some other examples of inspecting req.URL via newCustomClient, so I've ditched the one-time helper in 8c9b10a99ae18f95efd5f7b17f5f857aceba3ea2

I think this way is more consistent with the rest of the existing tests, so let me know what you think.

[flimzy]

Looks great. Thanks for the contribution! I'll roll a patch release shortly.