• add comments per golang conventions so the Exported methods within Bucket.go appear in Godoc

Running golint on gocb ‘master’ branch produces 186 lint errors. While these don’t affect execution of the gocb package, they represent undocumented methods the package purposefully exports, and they do not show up in gocb's godoc. The couchbase Go SDK docs event point to godoc as the reference developers should follow, but there is a gap in that these exported methods documented. It also makes the library appear unfinished, or not adhering to some of the Golang conventions prescribed by golint.

This pull request document these exported methods on Bucket (for starters) for the benefit of the community, specifically:

[mholton@mh:~/Go/src/github.com/holtonma/gocb → fix_golint_exported_methods]$ golint
bucket.go:10:1: comment on exported type Bucket should be of the form "Bucket ..." (with optional leading article)
bucket.go:54:1: exported method Bucket.OperationTimeout should have comment or be unexported
bucket.go:57:1: exported method Bucket.SetOperationTimeout should have comment or be unexported
bucket.go:60:1: exported method Bucket.DurabilityTimeout should have comment or be unexported
bucket.go:63:1: exported method Bucket.SetDurabilityTimeout should have comment or be unexported
bucket.go:66:1: exported method Bucket.DurabilityPollTimeout should have comment or be unexported
bucket.go:69:1: exported method Bucket.SetDurabilityPollTimeout should have comment or be unexported
bucket.go:73:1: exported method Bucket.SetTranscoder should have comment or be unexported
bucket.go:77:1: exported method Bucket.InvalidateQueryCache should have comment or be unexported
bucket.go:83:6: exported type Cas should have comment or be unexported
bucket.go:111:1: exported method Bucket.Close should have comment or be unexported
bucket.go:115:1: exported method Bucket.IoRouter should have comment or be unexported
bucket.go:119:1: comment on exported method Bucket.Internal should be of the form "Internal ..."
bucket.go:121:29: exported method Internal returns unexported type *gocb.bucketInternal, which can be annoying to use
bucket.go:125:1: exported method Bucket.Manager should have comment or be unexported

If this PR is accepted, I'll take the time to clean up the rest of the golint messages on bucket_crud, bucket_dura, bucket_http, bucket_multi, bucket_subdoc, etc. The full list of golint messages is in this gist: https://gist.github.com/holtonma/9aea53185f887496983a

Need to revisit variable names, and error text. The purpose of these changes are so that one could do this:

if cas, err := bucket.Upsert(key, value, 0); err != nil {
    if err == gocbcore.ErrTmpFail || err == gocbcore.ErrQueueOverflow {
        // try again later
    } else {
        // handle other errors
    }
}

The way current master code is implemented prevents me from doing that.

Hey guys,

Wondering if you would be interested in accepting this patch. We are using 1.8.1, so our nodes do not have the CouchAPIBase attribute.

As far as I can tell this solves the issue, and I am able to write to and read from our production couchbase cluster.

Curious about your feedback, or if there's anything else I need to do as part of this patch to take advantage of the go client.

More details:

The server info JSON for our production couchbase 1.8.1 cluster includes node information, formatted like this:

$ curl http://localhost:21214/pools/default/bucketsStreaming/default
{
  "name":"default",
  "bucketType":"membase",
  ...snip
  "nodes":[{
    "replication":1.0,
    "clusterMembership":"active",
    "status":"healthy",
    "hostname":"127.0.0.1:8091",
    "clusterCompatibility":1,
    "version":"1.8.1...",
    "os":"...",
    "ports":{"proxy":11211,"direct":11210}
  },{

What is missing is the Node.CouchAPIBase property (according to the link, added in 2.0.1.0).

If that property is not set, then in routeconfig.go we skip over this if statement:

// routeconfig.go#buildRouteConfig
            if node.CouchAPIBase != "" {
                // Slice off the UUID as Go's HTTP client cannot handle being passed URL-Encoded path values.
                capiEp := strings.SplitN(node.CouchAPIBase, "%2B", 2)[0]

                capiEpList = append(capiEpList, capiEp)
            }

This results in an invalid configuration, as the capiEpList is then empty.

// from routeconfig.go
func (config *routeConfig) IsValid() bool {
    if len(config.kvServerList) == 0 || len(config.mgmtEpList) == 0 {
        return false
    }
    switch config.bktType {
    case BktTypeCouchbase:
        return len(config.vbMap) > 0 && len(config.vbMap[0]) > 0 && len(config.capiEpList) > 0
    case BktTypeMemcached:
        return len(config.ketamaMap) > 0
    default:
        return false
    }
}

The case BktTypeCouchbase: statement in particular is where our issue crops up.

The property is not present, so we do not append any servers to config.capiEpList.

The workaround is to simply append servers to the capiEpList if they are present (accomplished by the patch). This code will append the servers to the capiEpList only if the length of the capiEpList is 0 (meaning it has not been populated).

This has been tested against our production 1.8.1 cluster and results in a valid configuration.

@StabbyCutyou @csmith0651

There was an error in line 225. It appended value instead of element from setContents. My fix is looking for id of item to remove and removing it from a slice.

Also I'm curious why this feature is implemented in client SDK as far as I understand the API is provided by couchbase server. These data structures may be manipulated with basic operations without retrieving and storing the entire document.

The current code always appends brackets [] regardless of whether the role you want to add requires them or not. For example using the Role

user := gocb.User{
    Username: username,
    DisplayName: usernameConfig.DisplayName,
    Password: password,
    Roles: []gocb.Role{
        {
            Name: "ro_admin",
        },
    },
}

produces this error from the couchbase server...

couchbase_test.go:140: err: {"errors":{"roles":"Cannot assign roles to user because the following roles are unknown, malformed or role parameters are undefined: [ro_admin[]]"}} | {"unique_id":"157521e8-de25-4a23-b76b-fb7164a6b006","endpoint":"http://localhost:8091"}

The gocb client generates this request to the server...

PUT /settings/rbac/users/local/v-test-test-1YY4qZyJH6FzMmITe5YV-1589064428 HTTP/1.1 Host: localhost:8091 User-Agent: {"a":"gocbcore/v9.0.0 gocb/v2.1.0","i":"c00126f6-12fb-4259-b175-75a6d8af5dbf"} Transfer-Encoding: chunked Authorization: Basic QWRtaW5pc3RyYXRvcjpBZG1pbjEyMw== Content-Type: application/x-www-form-urlencoded Accept-Encoding: gzip

name=test&password=A1a-fgSrWXPJygyJMWkL&roles=ro_admin%5B%5

where the cli client correctly produces this request...

PUT /settings/rbac/users/local/jdoe HTTP/1.1 Host: 127.0.0.1:8091 User-Agent: couchbase-cli 6.5.1-6299 Accept-Encoding: gzip, deflate Accept: / Connection: keep-alive Content-Length: 52 Content-Type: application/x-www-form-urlencoded Authorization: Basic QWRtaW5pc3RyYXRvcjpBZG1pbjEyMw==

name=John+Doe&password=cbpass&roles=ro_admin&groups=

==================
WARNING: DATA RACE
Read at 0x00c4206987f8 by goroutine 30:
  gopkg.in/couchbase/gocb%2ev1.(*thresholdLogGroup).logRecordedRecords()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/thresholdlogtracer.go:98 +0x5f
  gopkg.in/couchbase/gocb%2ev1.(*ThresholdLoggingTracer).logRecordedRecords()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/thresholdlogtracer.go:198 +0x6c
  gopkg.in/couchbase/gocb%2ev1.(*ThresholdLoggingTracer).loggerRoutine()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/thresholdlogtracer.go:250 +0x4a

Previous write at 0x00c4206987f8 by goroutine 59:
  gopkg.in/couchbase/gocb%2ev1.(*thresholdLogGroup).recordOp()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/thresholdlogtracer.go:60 +0x499
  gopkg.in/couchbase/gocb%2ev1.(*ThresholdLoggingTracer).recordOp()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/thresholdlogtracer.go:261 +0xc2
  gopkg.in/couchbase/gocb%2ev1.(*thresholdLogSpan).Finish()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/thresholdlogtracer.go:408 +0x266
  gopkg.in/couchbase/gocb%2ev1.(*Bucket).Get()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/bucket_crud.go:13 +0x11b
  ... (private code)

Goroutine 30 (running) created at:
  gopkg.in/couchbase/gocb%2ev1.(*ThresholdLoggingTracer).startLoggerRoutine()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/thresholdlogtracer.go:242 +0x918
  gopkg.in/couchbase/gocb%2ev1.(*ThresholdLoggingTracer).AddRef()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/thresholdlogtracer.go:181 +0x78
  gopkg.in/couchbase/gocb%2ev1.tracerAddRef()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/tracing.go:14 +0x83
  gopkg.in/couchbase/gocb%2ev1.Connect()
      /Users/lloiser/go/src/gopkg.in/couchbase/gocb.v1/cluster.go:83 +0x439
  ... (private code)

Goroutine 59 (running) created at:
  testing.(*T).Run()
      /usr/local/go/src/testing/testing.go:824 +0x564
  testing.runTests.func1()
      /usr/local/go/src/testing/testing.go:1063 +0xa4
  testing.tRunner()
      /usr/local/go/src/testing/testing.go:777 +0x16d
  testing.runTests()
      /usr/local/go/src/testing/testing.go:1061 +0x4e1
  testing.(*M).Run()
      /usr/local/go/src/testing/testing.go:978 +0x2cd
  main.main()
      _testmain.go:42 +0x22a
==================

Partial patch for subdoc set-nil issue https://issues.couchbase.com/browse/GOCBC-994

Opened as answer to https://issues.couchbase.com/browse/GOCBC-994?focusedCommentId=438876&page=com.atlassian.jira.plugin.system.issuetabpanels:comment-tabpanel#comment-438876

As I mentioned there:

Opened now. I did not do it before because this is what I consider a temporary patch, and as you may notice I have basically zero tests around and I'm not sure that all those cases are the correct ones. So, please, take it with a pinch of salt!

Hi,

currently the query MatchAll and MatchNone doesn't work because when the json marshalling of the query occurred, it returns an empty json instead of { "match_all": {} } (for match all query).

So the only way I find to produce the correct json is to add an attribute and initialize an empty map.

Maybe there is a more elegant way to do it :/

RP is recreated to give Gerrit right email.

You cannot use methods of the underlying type, and you have to use type aliases for that. I suppose it is just a typo.

Please merge this fix ASAP because the package is broken after the previous merged PR.

When using durability, the number of goroutines is always increasing. The function observeOne is blocking, trying to send a message to either replicaCh or persistCh, but the function durability is not reading from these channels anymore once it reached the target durability settings. Thus, numServers goroutines are blocked on writes and never return.

There is a special case, If we ask for maximum replica and persist durability, the function will enter a endless loop, polling the server every 100ms.

By using buffered channels we ensure the writes are not blocking anymore, and once replicaCh and persistCh have been written, we can exit the goroutine.

Add type to const declarations to improve docs and use iota where possible to initialize constants.

The way constants were previously defined did not cause their documentation to be included with the type definition documentation. This is corrected by explicitly adding the type to the const declaration. Like so:

        type X int
        const (
                X0 X = iota
                X1
                X2
        )

as opposed to

        type X int
        const (
                X0 = X(1)
                X1 = X(2)
                X2 = X(3)
        )

This causes the constants to appear under their type definition when using go doc.

Before this patch:

$ go doc QueryStatus
package gocb // import "."

type QueryStatus string
    QueryStatus provides information about the current status of a query.

After this patch:

$ go doc QueryStatus
package gocb // import "."

type QueryStatus string
    QueryStatus provides information about the current status of a query.

const QueryStatusRunning QueryStatus = "running" ...

This also uses iota to initialize numeric constants. This is a cleaner way to assign the type and value on a set of constants and is more idiomatic. https://yourbasic.org/golang/iota/

No constant values or types are altered by this change.