[Tour Of Beam] verify that unit exists when saving progress (apache#2…

…4118)

* AIO

* Update learning/tour-of-beam/backend/integration_tests/auth_test.go

Co-authored-by: Danny McCormick <[email protected]>

* nit

Co-authored-by: Danny McCormick <[email protected]>

learning/tour-of-beam/backend/function.go

@@ -197,6 +197,10 @@ func postUnitComplete(w http.ResponseWriter, r *http.Request, sdk tob.Sdk, uid s
 	unitId := r.URL.Query().Get("id")
 
 	err := svc.SetUnitComplete(r.Context(), sdk, unitId, uid)
+	if errors.Is(err, tob.ErrNoUnit) {
+		finalizeErrResponse(w, http.StatusNotFound, NOT_FOUND, "unit not found")
+		return
+	}
 	if err != nil {
 		log.Println("Set unit complete error:", err)
 		finalizeErrResponse(w, http.StatusInternalServerError, INTERNAL_ERROR, "storage error")
@@ -219,6 +223,10 @@ func postUserCode(w http.ResponseWriter, r *http.Request, sdk tob.Sdk, uid strin
 	}
 
 	err = svc.SaveUserCode(r.Context(), sdk, unitId, uid, userCodeRequest)
+	if errors.Is(err, tob.ErrNoUnit) {
+		finalizeErrResponse(w, http.StatusNotFound, NOT_FOUND, "unit not found")
+		return
+	}
 	if err != nil {
 		log.Println("Save user code error:", err)
 		message := "storage error"

learning/tour-of-beam/backend/integration_tests/auth_test.go

@@ -17,6 +17,7 @@ package main
 
 import (
 	"flag"
+	"net/http"
 	"os"
 	"path/filepath"
 	"testing"
@@ -36,76 +37,100 @@ func TestMain(m *testing.M) {
 	os.Exit(m.Run())
 }
 
+func makeUserCodeRequest() UserCodeRequest {
+	return UserCodeRequest{
+		Files: []UserCodeFile{
+			{Name: "main.py", Content: "import sys; sys.exit(0)", IsMain: true},
+		},
+		PipelineOptions: "some opts",
+	}
+}
+
+func checkBadHttpCode(t *testing.T, err error, code int) {
+	if err == nil {
+		t.Fatal("error expected")
+	}
+	if err, ok := err.(*ErrBadResponse); ok {
+		if err.Code == code {
+			return
+		}
+	}
+	t.Fatalf("Expected ErrBadResponse with code %v, got %v", code, err)
+}
+
 func TestSaveGetProgress(t *testing.T) {
 	idToken := emulator.getIDToken()
 
+	// postUnitCompleteURL
+	port := os.Getenv(PORT_POST_UNIT_COMPLETE)
+	if port == "" {
+		t.Fatal(PORT_POST_UNIT_COMPLETE, "env not set")
+	}
+	postUnitCompleteURL := "http://localhost:" + port
+
+	// postUserCodeURL
+	port = os.Getenv(PORT_POST_USER_CODE)
+	if port == "" {
+		t.Fatal(PORT_POST_USER_CODE, "env not set")
+	}
+	postUserCodeURL := "http://localhost:" + port
+
+	// getUserProgressURL
+	port = os.Getenv(PORT_GET_USER_PROGRESS)
+	if port == "" {
+		t.Fatal(PORT_GET_USER_PROGRESS, "env not set")
+	}
+	getUserProgressURL := "http://localhost:" + port
+
+	t.Run("save_complete_no_unit", func(t *testing.T) {
+		resp, err := PostUnitComplete(postUnitCompleteURL, "python", "unknown_unit_id_1", idToken)
+		checkBadHttpCode(t, err, http.StatusNotFound)
+		assert.Equal(t, "NOT_FOUND", resp.Code)
+		assert.Equal(t, "unit not found", resp.Message)
+	})
 	t.Run("save_complete", func(t *testing.T) {
-		port := os.Getenv(PORT_POST_UNIT_COMPLETE)
-		if port == "" {
-			t.Fatal(PORT_POST_UNIT_COMPLETE, "env not set")
-		}
-		url := "http://localhost:" + port
-
-		err := PostUnitComplete(url, "python", "unit_id_1", idToken)
+		_, err := PostUnitComplete(postUnitCompleteURL, "python", "challenge1", idToken)
 		if err != nil {
 			t.Fatal(err)
 		}
 	})
 	t.Run("save_code", func(t *testing.T) {
-		port := os.Getenv(PORT_POST_USER_CODE)
-		if port == "" {
-			t.Fatal(PORT_POST_USER_CODE, "env not set")
-		}
-		url := "http://localhost:" + port
-		req := UserCodeRequest{
-			Files: []UserCodeFile{
-				{Name: "main.py", Content: "import sys; sys.exit(0)", IsMain: true},
-			},
-			PipelineOptions: "some opts",
-		}
-
-		_, err := PostUserCode(url, "python", "unit_id_2", idToken, req)
+		req := makeUserCodeRequest()
+		_, err := PostUserCode(postUserCodeURL, "python", "example1", idToken, req)
 		if err != nil {
 			t.Fatal(err)
 		}
 	})
-	t.Run("save_code_fail", func(t *testing.T) {
-		port := os.Getenv(PORT_POST_USER_CODE)
-		if port == "" {
-			t.Fatal(PORT_POST_USER_CODE, "env not set")
-		}
-		url := "http://localhost:" + port
-		req := UserCodeRequest{
-			Files: []UserCodeFile{
-				// empty content doesn't pass validation
-				{Name: "main.py", Content: "", IsMain: true},
-			},
-			PipelineOptions: "some opts",
-		}
+	t.Run("save_code_playground_fail", func(t *testing.T) {
+		req := makeUserCodeRequest()
 
-		resp, err := PostUserCode(url, "python", "unit_id_1", idToken, req)
-		if err != nil {
-			t.Fatal(err)
-		}
+		// empty content doesn't pass validation
+		req.Files[0].Content = ""
+
+		resp, err := PostUserCode(postUserCodeURL, "python", "example1", idToken, req)
+		checkBadHttpCode(t, err, http.StatusInternalServerError)
 		assert.Equal(t, "INTERNAL_ERROR", resp.Code)
 		msg := "playground api error"
 		assert.Equal(t, msg, resp.Message[:len(msg)])
 
+	})
+	t.Run("save_code_no_unit", func(t *testing.T) {
+		req := makeUserCodeRequest()
+		resp, err := PostUserCode(postUserCodeURL, "python", "unknown_unit_id_1", idToken, req)
+		checkBadHttpCode(t, err, http.StatusNotFound)
+		assert.Equal(t, "NOT_FOUND", resp.Code)
+		assert.Equal(t, "unit not found", resp.Message)
+
 	})
 	t.Run("get", func(t *testing.T) {
-		port := os.Getenv(PORT_GET_USER_PROGRESS)
-		if port == "" {
-			t.Fatal(PORT_GET_USER_PROGRESS, "env not set")
-		}
-		url := "http://localhost:" + port
 
 		mock_path := filepath.Join("..", "samples", "api", "get_user_progress.json")
 		var exp SdkProgress
 		if err := loadJson(mock_path, &exp); err != nil {
 			t.Fatal(err)
 		}
 
-		resp, err := GetUserProgress(url, "python", idToken)
+		resp, err := GetUserProgress(getUserProgressURL, "python", idToken)
 		if err != nil {
 			t.Fatal(err)
 		}

learning/tour-of-beam/backend/integration_tests/client.go

@@ -21,6 +21,14 @@ import (
 	"os"
 )
 
+type ErrBadResponse struct {
+	Code int
+}
+
+func (e *ErrBadResponse) Error() string {
+	return fmt.Sprintf("http code %d", e.Code)
+}
+
 var (
 	ExpectedHeaders = map[string]string{
 		"Access-Control-Allow-Origin": "*",
@@ -81,11 +89,11 @@ func GetUserProgress(url, sdk, token string) (SdkProgress, error) {
 	return result, err
 }
 
-func PostUnitComplete(url, sdk, unitId, token string) error {
-	var result interface{}
+func PostUnitComplete(url, sdk, unitId, token string) (ErrorResponse, error) {
+	var result ErrorResponse
 	err := Do(&result, http.MethodPost, url, map[string]string{"sdk": sdk, "id": unitId},
 		map[string]string{"Authorization": "Bearer " + token}, nil)
-	return err
+	return result, err
 }
 
 func PostUserCode(url, sdk, unitId, token string, body UserCodeRequest) (ErrorResponse, error) {
@@ -166,5 +174,12 @@ func Do(dst interface{}, method, url string, queryParams, headers map[string]str
 
 	tee := io.TeeReader(resp.Body, os.Stdout)
 	defer os.Stdout.WriteString("\n")
-	return json.NewDecoder(tee).Decode(dst)
+	if err := json.NewDecoder(tee).Decode(dst); err != nil {
+		return fmt.Errorf("response decode err: %w", err)
+	}
+
+	if resp.StatusCode != http.StatusOK {
+		return &ErrBadResponse{resp.StatusCode}
+	}
+	return nil
 }

learning/tour-of-beam/backend/integration_tests/function_test.go

@@ -17,6 +17,7 @@ package main
 
 import (
 	"encoding/json"
+	"net/http"
 	"os"
 	"path/filepath"
 	"testing"
@@ -113,23 +114,27 @@ func TestGetUnitContent(t *testing.T) {
 
 func TestNegative(t *testing.T) {
 	for i, params := range []struct {
-		portEnvName string
-		queryParams map[string]string
-		headers     map[string]string
-		expected    ErrorResponse
+		portEnvName  string
+		queryParams  map[string]string
+		headers      map[string]string
+		expectedCode int
+		expected     ErrorResponse
 	}{
 		{PORT_GET_CONTENT_TREE, nil, nil,
+			http.StatusBadRequest,
 			ErrorResponse{
 				Code:    "BAD_FORMAT",
 				Message: "unknown sdk",
 			},
 		},
 		{PORT_GET_CONTENT_TREE, map[string]string{"sdk": "scio"}, nil,
 			// TODO: actually here should be a NOT_FOUND error
+			http.StatusInternalServerError,
 			ErrorResponse{Code: "INTERNAL_ERROR", Message: "storage error"},
 		},
 		{PORT_GET_UNIT_CONTENT, map[string]string{"sdk": "python", "id": "unknown_unitId"},
 			nil,
+			http.StatusNotFound,
 			ErrorResponse{
 				Code:    "NOT_FOUND",
 				Message: "unit not found",
@@ -140,6 +145,7 @@ func TestNegative(t *testing.T) {
 		{PORT_GET_USER_PROGRESS,
 			map[string]string{"sdk": "python"},
 			map[string]string{"authorization": "bad_header"},
+			http.StatusUnauthorized,
 			ErrorResponse{
 				Code:    "UNAUTHORIZED",
 				Message: "bad auth header",
@@ -155,9 +161,7 @@ func TestNegative(t *testing.T) {
 
 		var resp ErrorResponse
 		err := Get(&resp, url, params.queryParams, params.headers)
-		if err != nil {
-			t.Fatal(err)
-		}
+		checkBadHttpCode(t, err, params.expectedCode)
 		assert.Equal(t, params.expected, resp)
 	}
 }

learning/tour-of-beam/backend/internal/service/content.go

@@ -70,10 +70,18 @@ func (s *Svc) GetUserProgress(ctx context.Context, sdk tob.Sdk, userId string) (
 }
 
 func (s *Svc) SetUnitComplete(ctx context.Context, sdk tob.Sdk, unitId, uid string) error {
+	if err := s.Repo.CheckUnitExists(ctx, sdk, unitId); err != nil {
+		return err
+	}
+
 	return s.Repo.SetUnitComplete(ctx, sdk, unitId, uid)
 }
 
 func (s *Svc) SaveUserCode(ctx context.Context, sdk tob.Sdk, unitId, uid string, userRequest tob.UserCodeRequest) error {
+	if err := s.Repo.CheckUnitExists(ctx, sdk, unitId); err != nil {
+		return err
+	}
+
 	req := MakePgSaveRequest(userRequest, sdk)
 	resp, err := s.PgClient.SaveSnippet(ctx, &req)
 	if err != nil {

learning/tour-of-beam/backend/internal/storage/datastore.go

@@ -229,15 +229,17 @@ func (d *DatastoreDb) SaveContentTrees(ctx context.Context, trees []tob.ContentT
 	return nil
 }
 
-// Get learning unit content by unitId
-func (d *DatastoreDb) GetUnitContent(ctx context.Context, sdk tob.Sdk, unitId string) (unit *tob.Unit, err error) {
+// get a custom projection of a learning unit
+func (d *DatastoreDb) getUnit(ctx context.Context, sdk tob.Sdk, unitId string,
+	projectionFunc func(*datastore.Query) *datastore.Query) (unit *tob.Unit, err error) {
 	var tbNodes []TbLearningNode
 	rootKey := pgNameKey(TbLearningPathKind, sdk.StorageID(), nil)
 
 	query := datastore.NewQuery(TbLearningNodeKind).
 		Namespace(PgNamespace).
 		Ancestor(rootKey).
 		FilterField("id", "=", unitId)
+	query = projectionFunc(query)
 
 	_, err = d.Client.GetAll(ctx, query, &tbNodes)
 	if err != nil {
@@ -258,6 +260,27 @@ func (d *DatastoreDb) GetUnitContent(ctx context.Context, sdk tob.Sdk, unitId st
 	return node.Unit, nil
 }
 
+// Get learning unit content by unitId
+func (d *DatastoreDb) GetUnitContent(ctx context.Context, sdk tob.Sdk, unitId string) (unit *tob.Unit, err error) {
+	return d.getUnit(ctx, sdk, unitId, func(q *datastore.Query) *datastore.Query {
+		return q
+	})
+}
+
+// Check if the unit exists, returns ErrNoUnit if not
+func (d *DatastoreDb) CheckUnitExists(ctx context.Context, sdk tob.Sdk, unitId string) (err error) {
+	unit, err := d.getUnit(ctx, sdk, unitId, func(q *datastore.Query) *datastore.Query {
+		return q.Project("__key__", "type")
+	})
+	if err != nil {
+		return err
+	}
+	if unit == nil {
+		return tob.ErrNoUnit
+	}
+	return nil
+}
+
 func (d *DatastoreDb) SaveUser(ctx context.Context, uid string) error {
 	userKey := pgNameKey(TbUserKind, uid, nil)
 

learning/tour-of-beam/backend/internal/storage/iface.go

@@ -26,6 +26,8 @@ type Iface interface {
 	SaveContentTrees(ctx context.Context, trees []tob.ContentTree) error
 
 	GetUnitContent(ctx context.Context, sdk tob.Sdk, unitId string) (*tob.Unit, error)
+	// Check if the unit exists, returns ErrNoUnit if not
+	CheckUnitExists(ctx context.Context, sdk tob.Sdk, unitId string) error
 
 	SaveUser(ctx context.Context, uid string) error
 	GetUserProgress(ctx context.Context, sdk tob.Sdk, uid string) (*tob.SdkProgress, error)

learning/tour-of-beam/backend/internal/storage/index.yaml

@@ -6,6 +6,11 @@ indexes:
 # index.yaml file manually, remove the "# AUTOGENERATED" marker line above.
 # If you want to manage some indexes manually, move them above the marker line.
 
+- kind: "tb_learning_node"
+  ancestor: yes
+  properties:
+  - name: "id"
+  - name: "type"
 - kind: "tb_learning_module"
   ancestor: yes
   properties:

learning/tour-of-beam/backend/internal/storage/mock.go

@@ -60,6 +60,15 @@ func (d *Mock) GetUnitContent(_ context.Context, sdk tob.Sdk, unitId string) (u
 	return u, err
 }
 
+func (d *Mock) CheckUnitExists(ctx context.Context, sdk tob.Sdk, unitId string) error {
+	for _, existingId := range []string{"intro-unit", "example1", "challenge1"} {
+		if unitId == existingId {
+			return nil
+		}
+	}
+	return tob.ErrNoUnit
+}
+
 func (d *Mock) SaveUser(ctx context.Context, uid string) error {
 	return nil
 }

learning/tour-of-beam/backend/samples/api/get_user_progress.json

@@ -1,11 +1,11 @@
 {
     "units": [
         {
-            "id": "unit_id_1",
+            "id": "challenge1",
             "isCompleted": true
         },
         {
-            "id": "unit_id_2",
+            "id": "example1",
             "userSnippetId": "QY2dskTNu_w"
         }
     ]