Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

refactor UpdateUser to a Handler Function #173

Open
wants to merge 3 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
7 changes: 7 additions & 0 deletions db/db.go
Original file line number Diff line number Diff line change
Expand Up @@ -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.
Expand Down
26 changes: 26 additions & 0 deletions db/mock.go
Original file line number Diff line number Diff line change
Expand Up @@ -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 {
Expand Down Expand Up @@ -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()
Expand Down
7 changes: 7 additions & 0 deletions db/mock_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -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) {
Expand Down
2 changes: 2 additions & 0 deletions db/tladb.go
Original file line number Diff line number Diff line change
Expand Up @@ -3,6 +3,7 @@ package db
import (
"context"

"cloud.google.com/go/firestore"
"github.com/labstack/echo/v4"
)

Expand Down Expand Up @@ -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)
Expand Down
48 changes: 0 additions & 48 deletions db/user.go
Original file line number Diff line number Diff line change
@@ -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.
Expand Down Expand Up @@ -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")
}
76 changes: 0 additions & 76 deletions db/user_test.go
Original file line number Diff line number Diff line change
@@ -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"
)

Expand Down Expand Up @@ -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) {})
})
}
90 changes: 82 additions & 8 deletions handler/user.go
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand All @@ -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 {
Expand Down Expand Up @@ -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 {
Expand Down Expand Up @@ -118,9 +152,10 @@ func DeleteUser(cc echo.Context) error {
// with the default data.
//
// Request Body:
// {
// "uid": string <optional>
// }
//
// {
// "uid": string <optional>
// }
//
// Returns: Status 200 with a marshalled User struct on success.
func CreateUser(cc echo.Context) error {
Expand Down Expand Up @@ -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")
}
Loading
Loading