internal/postgres: add ability to read and write readme and license name

- GetVersion now reads from the 'license' and 'readme' columns.
- InsertVersion now also tests to make sure that none of the required fields
are nil or empty strings before insertion and returns an error indicating
what is wrong with the input.
- Also added status codes to the InsertVersion errors to make
testing easier.

Fixes b/124339314
Fixes b/124340579

Change-Id: I37c969bf4618a00cf220f99ecb2ff088bd5af009
Reviewed-on: https://team-review.git.corp.google.com/c/423000
Reviewed-by: Katie Hockman <katiehockman@google.com>

go.mod

@@ -4,4 +4,5 @@ require (
 	github.com/lib/pq v1.0.0
 	gocloud.dev v0.10.0
 	golang.org/x/tools v0.0.0-20181221154417-3ad2d988d5e2
+	google.golang.org/grpc v1.18.0
 )

internal/postgres/postgres.go

@@ -6,10 +6,12 @@ package postgres
 
 import (
 	"database/sql"
-	"errors"
+	"fmt"
 	"time"
 
 	"golang.org/x/discovery/internal"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 type DB struct {
@@ -93,7 +95,6 @@ func (db *DB) InsertVersionLogs(logs []*internal.VersionLog) error {
 func (db *DB) GetVersion(name string, version string) (*internal.Version, error) {
 	var commitTime, createdAt, updatedAt time.Time
 	var synopsis, license, readme string
-
 	query := `
 		SELECT
 			created_at,
@@ -105,9 +106,9 @@ func (db *DB) GetVersion(name string, version string) (*internal.Version, error)
 		FROM versions
 		WHERE name = $1 and version = $2;`
 	row := db.QueryRow(query, name, version)
-
 	if err := row.Scan(&createdAt, &updatedAt, &synopsis, &commitTime, &license, &readme); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("row.Scan(%q, %q, %q, %q, %q, %q): %v",
+			createdAt, updatedAt, synopsis, commitTime, license, readme, err)
 	}
 	return &internal.Version{
 		CreatedAt: createdAt,
@@ -121,7 +122,6 @@ func (db *DB) GetVersion(name string, version string) (*internal.Version, error)
 		License:    license,
 		ReadMe:     readme,
 	}, nil
-
 }
 
 // InsertVersion inserts a Version into the database along with any
@@ -131,8 +131,8 @@ func (db *DB) GetVersion(name string, version string) (*internal.Version, error)
 // exists in the database will not update the existing series or
 // module.
 func (db *DB) InsertVersion(version *internal.Version) error {
-	if version == nil {
-		return errors.New("postgres: cannot insert nil version")
+	if version == nil || version.Module == nil || version.Module.Name == "" || version.Version == "" || version.License == "" || version.CommitTime.IsZero() {
+		return status.Errorf(codes.InvalidArgument, "postgres: InsertVersion: must specify a module name, version, license, and non-zero commit time")
 	}
 
 	return db.Transact(func(tx *sql.Tx) error {
@@ -152,9 +152,6 @@ func (db *DB) InsertVersion(version *internal.Version) error {
 			return err
 		}
 
-		// TODO(ckimblebrown, julieqiu): Update Versions schema and insert readmes,
-		// licenses, dependencies, and packages (the rest of the fields in the
-		// internal.Version struct)
 		if _, err := tx.Exec(
 			`INSERT INTO versions(name, version, synopsis, commit_time, license, readme)
 			VALUES($1,$2,$3,$4,$5,$6)`,

internal/postgres/postgres_test.go

@@ -10,43 +10,44 @@ import (
 	"time"
 
 	"golang.org/x/discovery/internal"
+	"google.golang.org/grpc/codes"
+	"google.golang.org/grpc/status"
 )
 
 func TestPostgres_ReadAndWriteVersion(t *testing.T) {
-	var series = &internal.Series{
-		Name:    "myseries",
-		Modules: []*internal.Module{},
-	}
-
-	var module = &internal.Module{
-		Name:     "valid_module_name",
-		Series:   series,
-		Versions: []*internal.Version{},
-	}
-
-	var testVersion = &internal.Version{
-		Module:       module,
-		Version:      "v1.0.0",
-		Synopsis:     "This is a synopsis",
-		License:      "licensename",
-		ReadMe:       "readme",
-		CommitTime:   time.Now(),
-		Packages:     []*internal.Package{},
-		Dependencies: []*internal.Version{},
-		Dependents:   []*internal.Version{},
-	}
+	var (
+		now    = time.Now()
+		series = &internal.Series{
+			Name:    "myseries",
+			Modules: []*internal.Module{},
+		}
+		module = &internal.Module{
+			Name:     "valid_module_name",
+			Series:   series,
+			Versions: []*internal.Version{},
+		}
+		testVersion = &internal.Version{
+			Module:     module,
+			Version:    "v1.0.0",
+			Synopsis:   "This is a synopsis",
+			License:    "licensename",
+			ReadMe:     "readme",
+			CommitTime: now,
+		}
+	)
 
 	testCases := []struct {
 		name, moduleName, version string
 		versionData               *internal.Version
-		wantReadErr, wantWriteErr bool
+		wantWriteErrCode          codes.Code
+		wantReadErr               bool
 	}{
 		{
-			name:         "nil_version_write_error",
-			moduleName:   "valid_module_name",
-			version:      "v1.0.0",
-			wantReadErr:  true,
-			wantWriteErr: true,
+			name:             "nil_version_write_error",
+			moduleName:       "valid_module_name",
+			version:          "v1.0.0",
+			wantWriteErrCode: codes.InvalidArgument,
+			wantReadErr:      true,
 		},
 		{
 			name:        "valid_test",
@@ -68,6 +69,61 @@ func TestPostgres_ReadAndWriteVersion(t *testing.T) {
 			versionData: testVersion,
 			wantReadErr: true,
 		},
+		{
+			name: "missing_module",
+			versionData: &internal.Version{
+				Version:    "v1.0.0",
+				Synopsis:   "This is a synopsis",
+				License:    "licensename",
+				CommitTime: now,
+			},
+			wantWriteErrCode: codes.InvalidArgument,
+			wantReadErr:      true,
+		},
+		{
+			name: "missing_module_name",
+			versionData: &internal.Version{
+				Module:     &internal.Module{},
+				Version:    "v1.0.0",
+				Synopsis:   "This is a synopsis",
+				License:    "licensename",
+				CommitTime: now,
+			},
+			wantWriteErrCode: codes.InvalidArgument,
+			wantReadErr:      true,
+		},
+		{
+			name: "missing_version",
+			versionData: &internal.Version{
+				Module:     module,
+				Synopsis:   "This is a synopsis",
+				License:    "licensename",
+				CommitTime: now,
+			},
+			wantWriteErrCode: codes.InvalidArgument,
+			wantReadErr:      true,
+		},
+		{
+			name: "missing_license_name",
+			versionData: &internal.Version{
+				Module:     module,
+				Version:    "v1.0.0",
+				Synopsis:   "This is a synopsis",
+				CommitTime: now,
+			},
+			wantWriteErrCode: codes.InvalidArgument,
+			wantReadErr:      true,
+		},
+		{
+			name: "empty_commit_time",
+			versionData: &internal.Version{
+				Module:   module,
+				Version:  "v1.0.0",
+				Synopsis: "This is a synopsis",
+			},
+			wantWriteErrCode: codes.InvalidArgument,
+			wantReadErr:      true,
+		},
 	}
 
 	for _, tc := range testCases {
@@ -75,8 +131,8 @@ func TestPostgres_ReadAndWriteVersion(t *testing.T) {
 			teardownTestCase, db := SetupCleanDB(t)
 			defer teardownTestCase(t)
 
-			if err := db.InsertVersion(tc.versionData); tc.wantWriteErr != (err != nil) {
-				t.Errorf("db.InsertVersion(%+v) error: %v, want write error: %t", tc.versionData, err, tc.wantWriteErr)
+			if err := db.InsertVersion(tc.versionData); status.Code(err) != tc.wantWriteErrCode {
+				t.Errorf("db.InsertVersion(%+v) error: %v, want write error: %v", tc.versionData, err, tc.wantWriteErrCode)
 			}
 
 			// Test that insertion of duplicate primary key fails when the first insert worked