Repository: google/go-cloud
Pull request #819 (2018-11-27 19:42:03)

blob/gcsblob: add tests for OpenURL
The `gcsblob.go` is just moving the function from being defined inline to being defined at the top level. It's more obvious if you "hide whitespace" in the diff.

Updates #675.

vangent: PTAL.

Patch:
diff --git a/blob/gcsblob/gcsblob.go b/blob/gcsblob/gcsblob.go
index 2715637e..1c992c24 100644
--- a/blob/gcsblob/gcsblob.go
+++ b/blob/gcsblob/gcsblob.go
@@ -59,46 +59,48 @@ import (
 const defaultPageSize = 1000
 
 func init() {
-	blob.Register("gs", func(ctx context.Context, u *url.URL) (driver.Bucket, error) {
-		q := u.Query()
-		opts := &Options{}
+	blob.Register("gs", openURL)
+}
 
-		if accessID := q["access_id"]; len(accessID) > 0 {
-			opts.GoogleAccessID = accessID[0]
-		}
+func openURL(ctx context.Context, u *url.URL) (driver.Bucket, error) {
+	q := u.Query()
+	opts := &Options{}
 
-		if keyPath := q["private_key_path"]; len(keyPath) > 0 {
-			pk, err := ioutil.ReadFile(keyPath[0])
-			if err != nil {
-				return nil, err
-			}
-			opts.PrivateKey = pk
-		}
+	if accessID := q["access_id"]; len(accessID) > 0 {
+		opts.GoogleAccessID = accessID[0]
+	}
 
-		var creds *google.Credentials
-		if credPath := q["cred_path"]; len(credPath) == 0 {
-			var err error
-			creds, err = gcp.DefaultCredentials(ctx)
-			if err != nil {
-				return nil, err
-			}
-		} else {
-			jsonCreds, err := ioutil.ReadFile(credPath[0])
-			if err != nil {
-				return nil, err
-			}
-			creds, err = google.CredentialsFromJSON(ctx, jsonCreds)
-			if err != nil {
-				return nil, err
-			}
+	if keyPath := q["private_key_path"]; len(keyPath) > 0 {
+		pk, err := ioutil.ReadFile(keyPath[0])
+		if err != nil {
+			return nil, err
 		}
+		opts.PrivateKey = pk
+	}
 
-		client, err := gcp.NewHTTPClient(gcp.DefaultTransport(), gcp.CredentialsTokenSource(creds))
+	var creds *google.Credentials
+	if credPath := q["cred_path"]; len(credPath) == 0 {
+		var err error
+		creds, err = gcp.DefaultCredentials(ctx)
 		if err != nil {
 			return nil, err
 		}
-		return openBucket(ctx, u.Host, client, opts)
-	})
+	} else {
+		jsonCreds, err := ioutil.ReadFile(credPath[0])
+		if err != nil {
+			return nil, err
+		}
+		creds, err = google.CredentialsFromJSON(ctx, jsonCreds)
+		if err != nil {
+			return nil, err
+		}
+	}
+
+	client, err := gcp.NewHTTPClient(gcp.DefaultTransport(), gcp.CredentialsTokenSource(creds))
+	if err != nil {
+		return nil, err
+	}
+	return openBucket(ctx, u.Host, client, opts)
 }
 
 // Options sets options for constructing a *blob.Bucket backed by GCS.


Test patch:
diff --git a/blob/gcsblob/gcsblob_test.go b/blob/gcsblob/gcsblob_test.go
index 84540430..62726620 100644
--- a/blob/gcsblob/gcsblob_test.go
+++ b/blob/gcsblob/gcsblob_test.go
@@ -21,6 +21,8 @@ import (
 	"fmt"
 	"io/ioutil"
 	"net/http"
+	"net/url"
+	"os"
 	"testing"
 
 	"cloud.google.com/go/storage"
@@ -29,6 +31,7 @@ import (
 	"github.com/google/go-cloud/blob/drivertest"
 	"github.com/google/go-cloud/gcp"
 	"github.com/google/go-cloud/internal/testing/setup"
+	"github.com/google/go-cmp/cmp"
 	"google.golang.org/api/googleapi"
 	"google.golang.org/api/option"
 )
@@ -200,3 +203,99 @@ func TestBufferSize(t *testing.T) {
 		}
 	}
 }
+
+func TestOpenURL(t *testing.T) {
+
+	ctx := context.Background()
+
+	// Create a file for use as a dummy private key file.
+	privateKey := []byte("some content")
+	pkFile, err := ioutil.TempFile("", "my-private-key")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(pkFile.Name())
+	if _, err := pkFile.Write(privateKey); err != nil {
+		t.Fatal(err)
+	}
+	if err := pkFile.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	jsonCred := []byte(`{"client_id": "foo.apps.googleusercontent.com", "client_secret": "bar", "refresh_token": "baz", "type": "authorized_user"}`)
+	credFile, err := ioutil.TempFile("", "my-creds")
+	if err != nil {
+		t.Fatal(err)
+	}
+	defer os.Remove(credFile.Name())
+	if _, err := credFile.Write(jsonCred); err != nil {
+		t.Fatal(err)
+	}
+	if err := credFile.Close(); err != nil {
+		t.Fatal(err)
+	}
+
+	tests := []struct {
+		url      string
+		wantName string
+		wantOpts Options
+		wantErr  bool
+	}{
+		{
+			url:      "gs://mybucket?cred_path=" + credFile.Name(),
+			wantName: "mybucket",
+		},
+		{
+			url:      "gs://mybucket2?cred_path=" + credFile.Name(),
+			wantName: "mybucket2",
+		},
+		{
+			url:      "gs://foo?access_id=bar&cred_path=" + credFile.Name(),
+			wantName: "foo",
+			wantOpts: Options{GoogleAccessID: "bar"},
+		},
+		{
+			url:     "gs://foo?private_key_path=/path/does/not/exist",
+			wantErr: true,
+		},
+		{
+			url:      "gs://foo?cred_path=" + credFile.Name() + "&private_key_path=" + pkFile.Name(),
+			wantName: "foo",
+			wantOpts: Options{PrivateKey: privateKey},
+		},
+		{
+			url:     "gs://foo?cred_path=/path/does/not/exist",
+			wantErr: true,
+		},
+		{
+			url:     "gs://foo?cred_path=" + pkFile.Name(),
+			wantErr: true,
+		},
+	}
+
+	for _, test := range tests {
+		t.Run(test.url, func(t *testing.T) {
+			u, err := url.Parse(test.url)
+			if err != nil {
+				t.Fatal(err)
+			}
+			got, err := openURL(ctx, u)
+			if (err != nil) != test.wantErr {
+				t.Errorf("got err %v want error %v", err, test.wantErr)
+			}
+			if err != nil {
+				return
+			}
+			gotB, ok := got.(*bucket)
+			if !ok {
+				t.Fatalf("got %T want *bucket", got)
+			}
+			if gotB.name != test.wantName {
+				t.Errorf("got bucket name %q want %q", gotB.name, test.wantName)
+			}
+			if diff := cmp.Diff(*gotB.opts, test.wantOpts); diff != "" {
+				t.Errorf("got\n%v\nwant\n%v\ndiff\n%s", *gotB.opts, test.wantOpts, diff)
+			}
+		})
+	}
+}