From fab093e73444fab534b57b6a86debbff89bc98ca Mon Sep 17 00:00:00 2001 From: travis Date: Wed, 7 Feb 2024 02:29:27 -0800 Subject: [PATCH 1/3] refactor UpdateUser --- db/mock.go | 26 ++++++++++++++ db/user.go | 48 -------------------------- handler/user.go | 90 ++++++++++++++++++++++++++++++++++++++++++++----- 3 files changed, 108 insertions(+), 56 deletions(-) diff --git a/db/mock.go b/db/mock.go index a18314a..9b6b097 100644 --- a/db/mock.go +++ b/db/mock.go @@ -13,6 +13,18 @@ type MockDB struct { db map[string]map[string]interface{} } +// An Update describes an update to a value referred to by a path. +// An Update should have either a non-empty Path or a non-empty FieldPath, +// but not both. +// +// See DocumentRef.Create for acceptable values. +// To delete a field, specify firestore.Delete as the value. +type Update struct { + Path string // Will be split on dots, and must not contain any of "˜*/[]". + FieldPath []string + Value interface{} +} + func (d *MockDB) LoadProgram(_ context.Context, pid string) (Program, error) { p, ok := d.db[programsPath][pid].(Program) if !ok { @@ -82,6 +94,20 @@ func (d *MockDB) CreateUser(_ context.Context, u User) (User, error) { return u, nil } +func (d *MockDB) UpdateUser(_ context.Context, uid string, updates []Update) (err error) { + raw, ok := d.db[usersPath][uid] + if !ok { + return errors.New("invalid user ID") + } + + u, ok := raw.(map[string]interface{}) + for _, update := range updates { + u[update.Path] = update.Value + } + + return nil +} + func (d *MockDB) CreateProgram(_ context.Context, p Program) (Program, error) { // Give the program a UID p.UID = uuid.New().String() diff --git a/db/user.go b/db/user.go index 76622e4..3979bcc 100755 --- a/db/user.go +++ b/db/user.go @@ -1,16 +1,7 @@ package db import ( - "context" - "net/http" - - "github.com/pkg/errors" - "github.com/uclaacm/teach-la-go-backend/httpext" - "google.golang.org/grpc/codes" - "google.golang.org/grpc/status" - "cloud.google.com/go/firestore" - "github.com/labstack/echo/v4" ) // User is a struct representation of a user document. @@ -44,42 +35,3 @@ func (u *User) ToFirestoreUpdate() []firestore.Update { return f } - -// UpdateUser updates the doc with specified UID's fields -// to match those of the request body. -// -// Request Body: -// { -// "uid": [REQUIRED] -// [User object fields] -// } -// -// Returns: Status 200 on success. -func (d *DB) UpdateUser(c echo.Context) error { - // unmarshal request body into an User struct. - requestObj := User{} - if err := httpext.RequestBodyTo(c.Request(), &requestObj); err != nil { - return err - } - - uid := requestObj.UID - if uid == "" { - return c.String(http.StatusBadRequest, "a uid is required") - } - if len(requestObj.Programs) != 0 { - return c.String(http.StatusBadRequest, "program list cannot be updated via /program/update") - } - - err := d.RunTransaction(c.Request().Context(), func(ctx context.Context, tx *firestore.Transaction) error { - ref := d.Collection(usersPath).Doc(uid) - return tx.Update(ref, requestObj.ToFirestoreUpdate()) - }) - if err != nil { - if status.Code(err) == codes.NotFound { - return c.String(http.StatusNotFound, "user could not be found") - } - return c.String(http.StatusInternalServerError, errors.Wrap(err, "failed to update user data").Error()) - } - - return c.String(http.StatusOK, "user updated successfully") -} diff --git a/handler/user.go b/handler/user.go index 22777b7..3a00a20 100644 --- a/handler/user.go +++ b/handler/user.go @@ -4,6 +4,7 @@ import ( "net/http" "strings" + "cloud.google.com/go/firestore" "github.com/labstack/echo/v4" "github.com/pkg/errors" "github.com/uclaacm/teach-la-go-backend/db" @@ -12,12 +13,44 @@ import ( "google.golang.org/grpc/status" ) +// User is a struct representation of a user document. +// It provides functions for converting the struct +// to firebase-digestible types. +type User struct { + Classes []string `firestore:"classes" json:"classes"` + DisplayName string `firestore:"displayName" json:"displayName"` + MostRecentProgram string `firestore:"mostRecentProgram" json:"mostRecentProgram"` + PhotoName string `firestore:"photoName" json:"photoName"` + Programs []string `firestore:"programs" json:"programs"` + UID string `json:"uid"` + DeveloperAcc bool `firestore:"developerAcc" json:"developerAcc"` +} + +// ToFirestoreUpdate returns the database update +// representation of its UserData struct. +func (u *User) ToFirestoreUpdate() []firestore.Update { + f := []firestore.Update{ + {Path: "mostRecentProgram", Value: u.MostRecentProgram}, + } + + switch { + case u.DisplayName != "": + f = append(f, firestore.Update{Path: "displayName", Value: u.DisplayName}) + case u.PhotoName != "": + f = append(f, firestore.Update{Path: "photoName", Value: u.PhotoName}) + case len(u.Programs) != 0: + f = append(f, firestore.Update{Path: "programs", Value: firestore.ArrayUnion(u.Programs)}) + } + + return f +} + // GetUser acquires the user document with the given uid. The // provided context must be a *db.DBContext. // // Query Parameters: -// - uid string: UID of user to GET -// - programs string: Whether to acquire programs. +// - uid string: UID of user to GET +// - programs string: Whether to acquire programs. // // Returns: Status 200 with marshalled User and programs. func GetUser(cc echo.Context) error { @@ -68,9 +101,10 @@ func GetUser(cc echo.Context) error { // from the database. // // Request Body: -// { -// "uid": REQUIRED -// } +// +// { +// "uid": REQUIRED +// } // // Returns: status 200 on deletion. func DeleteUser(cc echo.Context) error { @@ -118,9 +152,10 @@ func DeleteUser(cc echo.Context) error { // with the default data. // // Request Body: -// { -// "uid": string -// } +// +// { +// "uid": string +// } // // Returns: Status 200 with a marshalled User struct on success. func CreateUser(cc echo.Context) error { @@ -163,3 +198,42 @@ func CreateUser(cc echo.Context) error { return c.JSON(http.StatusCreated, &user) } + +// UpdateUser updates the doc with specified UID's fields +// to match those of the request body. +// +// Request Body: +// +// { +// "uid": [REQUIRED] +// [User object fields] +// } +// +// Returns: Status 200 on success. +func UpdateUser(cc echo.Context) error { + // unmarshal request body into an User struct. + requestObj := User{} + c := cc.(*db.DBContext) + if err := httpext.RequestBodyTo(c.Request(), &requestObj); err != nil { + return err + } + + uid := requestObj.UID + + if uid == "" { + return c.String(http.StatusBadRequest, "a uid is required") + } + if len(requestObj.Programs) != 0 { + return c.String(http.StatusBadRequest, "program list cannot be updated via /program/update") + } + + ref := c.QueryParam("uid") + if err := c.UpdateUser(c.Request().Context(), ref, requestObj.ToFirestoreUpdate()); err != nil { + if status.Code(err) == codes.NotFound { + return c.String(http.StatusNotFound, "could not find user") + } + return c.String(http.StatusInternalServerError, "failed to update user.") + } + + return c.String(http.StatusOK, "user updated successfully") +} From 1d4fc74730cec88a7644a76560168d559609ccbe Mon Sep 17 00:00:00 2001 From: travis Date: Wed, 7 Feb 2024 02:30:13 -0800 Subject: [PATCH 2/3] add new crud function --- db/db.go | 7 +++++++ db/tladb.go | 2 ++ 2 files changed, 9 insertions(+) diff --git a/db/db.go b/db/db.go index e94d7eb..b87b817 100755 --- a/db/db.go +++ b/db/db.go @@ -130,6 +130,13 @@ func (d *DB) DeleteUser(ctx context.Context, uid string) error { return nil } +func (d *DB) UpdateUser(ctx context.Context, uid string, updates []firestore.Update) error { + if _, err := d.Collection(usersPath).Doc(uid).Update(ctx, updates); err != nil { + return err + } + return nil +} + // Open returns a pointer to a new database client based on // JSON credentials given by the environment variable. // Returns an error if it fails at any point. diff --git a/db/tladb.go b/db/tladb.go index 72f9f0a..8b2c777 100644 --- a/db/tladb.go +++ b/db/tladb.go @@ -3,6 +3,7 @@ package db import ( "context" + "cloud.google.com/go/firestore" "github.com/labstack/echo/v4" ) @@ -30,6 +31,7 @@ type TLADB interface { LoadUser(context.Context, string) (User, error) StoreUser(context.Context, User) error DeleteUser(context.Context, string) error + UpdateUser(context.Context, string, []firestore.Update) error CreateUser(context.Context, User) (User, error) CreateProgram(context.Context, Program) (Program, error) From 385250a8cb4eff7a5280e55dd0a96a4cb5393420 Mon Sep 17 00:00:00 2001 From: travis Date: Wed, 7 Feb 2024 02:30:55 -0800 Subject: [PATCH 3/3] refactor tests --- db/mock_test.go | 7 ++++ db/user_test.go | 76 -------------------------------------------- handler/user_test.go | 66 ++++++++++++++++++++++++++++++++++++++ 3 files changed, 73 insertions(+), 76 deletions(-) diff --git a/db/mock_test.go b/db/mock_test.go index 26d0e37..bbda60f 100644 --- a/db/mock_test.go +++ b/db/mock_test.go @@ -42,6 +42,13 @@ func TestMockUser(t *testing.T) { })) assert.NoError(t, d.DeleteUser(context.Background(), "test")) }) + t.Run("update", func(t *testing.T) { + d := db.OpenMock() + err := d.UpdateUser(context.Background(), "test", []db.Update{{ + Path: "photoName", Value: "test_photo_name", + }}) + assert.NoError(t, err) + }) } func TestMockProgram(t *testing.T) { diff --git a/db/user_test.go b/db/user_test.go index 19134c5..43b60fc 100644 --- a/db/user_test.go +++ b/db/user_test.go @@ -1,15 +1,8 @@ package db import ( - "context" - "encoding/json" - "net/http" - "net/http/httptest" - "os" - "strings" "testing" - "github.com/labstack/echo/v4" "github.com/stretchr/testify/assert" ) @@ -41,72 +34,3 @@ func TestUserToFirestoreUpdate(t *testing.T) { // TODO: value cannot be easily verified. }) } - -func TestUpdateUser(t *testing.T) { - d, err := Open(context.Background(), os.Getenv("TLACFG")) - if !assert.NoError(t, err) { - return - } - - t.Run("MissingUID", func(t *testing.T) { - req := httptest.NewRequest(http.MethodPut, "/", strings.NewReader("{}")) - rec := httptest.NewRecorder() - assert.NotNil(t, req, rec) - c := echo.New().NewContext(req, rec) - - if assert.NoError(t, d.UpdateUser(c)) { - assert.Equal(t, http.StatusBadRequest, rec.Code) - } - }) - t.Run("BadUID", func(t *testing.T) { - req := httptest.NewRequest(http.MethodPut, "/", strings.NewReader("{\"uid\":\"fakeUID\"}")) - rec := httptest.NewRecorder() - assert.NotNil(t, req, rec) - c := echo.New().NewContext(req, rec) - - if assert.NoError(t, d.UpdateUser(c)) { - assert.Equal(t, http.StatusNotFound, rec.Code) - } - }) - - dbConsistencyWarning(t) - - t.Run("TypicalRequests", func(t *testing.T) { - iter := d.Collection(usersPath).Documents(context.Background()) - defer iter.Stop() - randomDoc, err := iter.Next() - if !assert.NoError(t, err) { - return - } - t.Logf("using doc ID (%s)", randomDoc.Ref.ID) - - u := User{} - if err := randomDoc.DataTo(&u); !assert.NoError(t, err) { - t.Fatalf("encountered a fatal error when converting random user doc to object: %s", err) - } - u.UID = randomDoc.Ref.ID - u.Programs = []string{} - - t.Run("DisplayName", func(t *testing.T) { - uCopy := u - uCopy.DisplayName = "test" - - bytes, err := json.Marshal(&uCopy) - assert.NoError(t, err) - - req := httptest.NewRequest(http.MethodPut, "/", strings.NewReader(string(bytes))) - rec := httptest.NewRecorder() - assert.NotNil(t, req, rec) - c := echo.New().NewContext(req, rec) - - if assert.NoError(t, d.UpdateUser(c)) { - assert.Equal(t, http.StatusOK, rec.Code) - // TODO: more tests required. - } - - }) - t.Run("MostRecentProgram", func(t *testing.T) {}) // TODO - t.Run("PhotoName", func(t *testing.T) {}) - t.Run("Programs", func(t *testing.T) {}) - }) -} diff --git a/handler/user_test.go b/handler/user_test.go index b4929db..71a0ba9 100644 --- a/handler/user_test.go +++ b/handler/user_test.go @@ -290,3 +290,69 @@ func TestCreateUser(t *testing.T) { }) } + +func TestUpdateUser(t *testing.T) { + t.Run("MissingUID", func(t *testing.T) { + d := db.OpenMock() + req := httptest.NewRequest(http.MethodPut, "/", strings.NewReader("{}")) + rec := httptest.NewRecorder() + assert.NotNil(t, req, rec) + c := echo.New().NewContext(req, rec) + + if assert.NoError(t, handler.UpdateUser(c)) { + assert.Equal(t, http.StatusBadRequest, rec.Code) + } + }) + t.Run("BadUID", func(t *testing.T) { + d := db.OpenMock() + req := httptest.NewRequest(http.MethodPut, "/", strings.NewReader("{\"uid\":\"fakeUID\"}")) + rec := httptest.NewRecorder() + assert.NotNil(t, req, rec) + c := echo.New().NewContext(req, rec) + + if assert.NoError(t, handler.UpdateUser(c)) { + assert.Equal(t, http.StatusNotFound, rec.Code) + } + }) + + // dbConsistencyWarning(t) + + t.Run("TypicalRequests", func(t *testing.T) { + iter := d.Collection(usersPath).Documents(context.Background()) + defer iter.Stop() + randomDoc, err := iter.Next() + if !assert.NoError(t, err) { + return + } + t.Logf("using doc ID (%s)", randomDoc.Ref.ID) + + u := User{} + if err := randomDoc.DataTo(&u); !assert.NoError(t, err) { + t.Fatalf("encountered a fatal error when converting random user doc to object: %s", err) + } + u.UID = randomDoc.Ref.ID + u.Programs = []string{} + + t.Run("DisplayName", func(t *testing.T) { + uCopy := u + uCopy.DisplayName = "test" + + bytes, err := json.Marshal(&uCopy) + assert.NoError(t, err) + + req := httptest.NewRequest(http.MethodPut, "/", strings.NewReader(string(bytes))) + rec := httptest.NewRecorder() + assert.NotNil(t, req, rec) + c := echo.New().NewContext(req, rec) + + if assert.NoError(t, handler.UpdateUser(c)) { + assert.Equal(t, http.StatusOK, rec.Code) + // TODO: more tests required. + } + + }) + t.Run("MostRecentProgram", func(t *testing.T) {}) // TODO + t.Run("PhotoName", func(t *testing.T) {}) + t.Run("Programs", func(t *testing.T) {}) + }) +}