[Backend][Multi-user] Adjust/implement run api for multiuser support

backend/src/apiserver/common/util.go

@@ -28,3 +28,14 @@ func GetNamespaceFromAPIResourceReferences(resourceRefs []*api.ResourceReference
 	}
 	return namespace
 }
+
+func GetExperimentIDFromAPIResourceReferences(resourceRefs []*api.ResourceReference) string {
+	experimentID := ""
+	for _, resourceRef := range resourceRefs {
+		if resourceRef.Key.Type == api.ResourceType_EXPERIMENT {
+			experimentID = resourceRef.Key.Id
+			break
+		}
+	}
+	return experimentID
+}

backend/src/apiserver/common/util_test.go

@@ -22,28 +22,83 @@ import (
 )
 
 func TestGetNamespaceFromResourceReferences(t *testing.T) {
-	references := []*api.ResourceReference{
+	tests := []struct {
+		name              string
+		references        []*api.ResourceReference
+		expectedNamespace string
+	}{
 		{
-			Key: &api.ResourceKey{
-				Type: api.ResourceType_EXPERIMENT, Id: "123"},
-			Relationship: api.Relationship_CREATOR,
+			"resource reference with namespace and experiment",
+			[]*api.ResourceReference{
+				{
+					Key: &api.ResourceKey{
+						Type: api.ResourceType_EXPERIMENT, Id: "123"},
+					Relationship: api.Relationship_CREATOR,
+				},
+				{
+					Key: &api.ResourceKey{
+						Type: api.ResourceType_NAMESPACE, Id: "ns"},
+					Relationship: api.Relationship_OWNER,
+				},
+			},
+			"ns",
 		},
 		{
-			Key: &api.ResourceKey{
-				Type: api.ResourceType_NAMESPACE, Id: "ns"},
-			Relationship: api.Relationship_OWNER,
+			"resource reference with experiment only",
+			[]*api.ResourceReference{
+				{
+					Key: &api.ResourceKey{
+						Type: api.ResourceType_EXPERIMENT, Id: "123"},
+					Relationship: api.Relationship_CREATOR,
+				},
+			},
+			"",
 		},
 	}
-	namespace := GetNamespaceFromAPIResourceReferences(references)
-	assert.Equal(t, "ns", namespace)
+	for _, tc := range tests {
+		namespace := GetNamespaceFromAPIResourceReferences(tc.references)
+		assert.Equal(t, tc.expectedNamespace, namespace,
+			"TestGetNamespaceFromResourceReferences(%v) has unexpected result.", tc.name)
+	}
+}
 
-	references = []*api.ResourceReference{
+func TestGetExperimentIDFromResourceReferences(t *testing.T) {
+	tests := []struct {
+		name                 string
+		references           []*api.ResourceReference
+		expectedExperimentID string
+	}{
 		{
-			Key: &api.ResourceKey{
-				Type: api.ResourceType_EXPERIMENT, Id: "123"},
-			Relationship: api.Relationship_CREATOR,
+			"resource reference with namespace and experiment",
+			[]*api.ResourceReference{
+				{
+					Key: &api.ResourceKey{
+						Type: api.ResourceType_EXPERIMENT, Id: "123"},
+					Relationship: api.Relationship_CREATOR,
+				},
+				{
+					Key: &api.ResourceKey{
+						Type: api.ResourceType_NAMESPACE, Id: "ns"},
+					Relationship: api.Relationship_OWNER,
+				},
+			},
+			"123",
 		},
+		{
+			"resource reference with namespace only",
+			[]*api.ResourceReference{
+				{
+					Key: &api.ResourceKey{
+						Type: api.ResourceType_NAMESPACE, Id: "ns"},
+					Relationship: api.Relationship_OWNER,
+				},
+			},
+			"",
+		},
+	}
+	for _, tc := range tests {
+		experimentID := GetExperimentIDFromAPIResourceReferences(tc.references)
+		assert.Equal(t, tc.expectedExperimentID, experimentID,
+			"TestGetExperimentIDFromResourceReferences(%v) has unexpected result.", tc.name)
 	}
-	namespace = GetNamespaceFromAPIResourceReferences(references)
-	assert.Equal(t, "", namespace)
 }

backend/src/apiserver/list/list.go

@@ -250,20 +250,39 @@ func FilterOnResourceReference(tableName string, columns []string, resourceType
 
 // FilterOnExperiment filters the given table by rows based on provided experiment ID,
 // and returns the rebuilt SelectBuilder
-func FilterRunOnExperiment(
+func FilterOnExperiment(
 	tableName string,
 	columns []string,
 	selectCount bool,
 	experimentID string,
 ) (sq.SelectBuilder, error) {
+	return filterByColumnValue(tableName, columns, selectCount, "ExperimentUUID", experimentID), nil
+}
+
+func FilterOnNamespace(
+	tableName string,
+	columns []string,
+	selectCount bool,
+	namespace string,
+) (sq.SelectBuilder, error) {
+	return filterByColumnValue(tableName, columns, selectCount, "Namespace", namespace), nil
+}
+
+func filterByColumnValue(
+	tableName string,
+	columns []string,
+	selectCount bool,
+	columnName string,
+	filterValue interface{},
+) sq.SelectBuilder {
 	selectBuilder := sq.Select(columns...)
 	if selectCount {
 		selectBuilder = sq.Select("count(*)")
 	}
 	selectBuilder = selectBuilder.From(tableName).Where(
-		sq.Eq{"ExperimentUUID": experimentID},
+		sq.Eq{columnName: filterValue},
 	)
-	return selectBuilder, nil
+	return selectBuilder
 }
 
 // Scans the one given row into a number, and returns the number

backend/src/apiserver/list/list_test.go

@@ -736,3 +736,91 @@ func TestFilterOnResourceReference(t *testing.T) {
 		}
 	}
 }
+
+func TestFilterOnExperiment(t *testing.T) {
+
+	type testIn struct {
+		table  string
+		count  bool
+		filter *common.FilterContext
+	}
+	tests := []struct {
+		in      *testIn
+		wantSql string
+		wantErr error
+	}{
+		{
+			in: &testIn{
+				table:  "testTable",
+				count:  false,
+				filter: &common.FilterContext{},
+			},
+			wantSql: "SELECT * FROM testTable WHERE ExperimentUUID = ?",
+			wantErr: nil,
+		},
+		{
+			in: &testIn{
+				table:  "testTable",
+				count:  true,
+				filter: &common.FilterContext{},
+			},
+			wantSql: "SELECT count(*) FROM testTable WHERE ExperimentUUID = ?",
+			wantErr: nil,
+		},
+	}
+
+	for _, test := range tests {
+		sqlBuilder, gotErr := FilterOnExperiment(test.in.table, []string{"*"}, test.in.count, "123")
+		gotSql, _, err := sqlBuilder.ToSql()
+		assert.Nil(t, err)
+
+		if gotSql != test.wantSql || gotErr != test.wantErr {
+			t.Errorf("FilterOnExperiment(%+v) =\nGot: %q, %v\nWant: %q, %v",
+				test.in, gotSql, gotErr, test.wantSql, test.wantErr)
+		}
+	}
+}
+
+func TestFilterOnNamesapce(t *testing.T) {
+
+	type testIn struct {
+		table  string
+		count  bool
+		filter *common.FilterContext
+	}
+	tests := []struct {
+		in      *testIn
+		wantSql string
+		wantErr error
+	}{
+		{
+			in: &testIn{
+				table:  "testTable",
+				count:  false,
+				filter: &common.FilterContext{},
+			},
+			wantSql: "SELECT * FROM testTable WHERE Namespace = ?",
+			wantErr: nil,
+		},
+		{
+			in: &testIn{
+				table:  "testTable",
+				count:  true,
+				filter: &common.FilterContext{},
+			},
+			wantSql: "SELECT count(*) FROM testTable WHERE Namespace = ?",
+			wantErr: nil,
+		},
+	}
+
+	for _, test := range tests {
+		sqlBuilder, gotErr := FilterOnNamespace(test.in.table, []string{"*"}, test.in.count, "ns")
+		gotSql, _, err := sqlBuilder.ToSql()
+		assert.Nil(t, err)
+
+		if gotSql != test.wantSql || gotErr != test.wantErr {
+			t.Errorf("FilterOnNamespace(%+v) =\nGot: %q, %v\nWant: %q, %v",
+				test.in, gotSql, gotErr, test.wantSql, test.wantErr)
+		}
+	}
+}

backend/src/apiserver/resource/resource_manager.go

@@ -316,10 +316,30 @@ func (r *ResourceManager) CreateRun(apiRun *api.Run) (*model.RunDetail, error) {
 		}
 	}
 
-	namespace := common.GetNamespaceFromAPIResourceReferences(apiRun.ResourceReferences)
+	resourceReferences := apiRun.GetResourceReferences()
+	experimentID := common.GetExperimentIDFromAPIResourceReferences(resourceReferences)
+	if len(experimentID) == 0 {
+		if multiuserMode {
+			return nil, util.NewInternalServerError(errors.New("Missing experiment"), "Experiment is required for CreateRun/CreateJob.")
+		} else {
+			// Add a reference to the default experiment
+			ref, err := r.getDefaultExperimentResourceReference(resourceReferences)
+			if err != nil {
+				return nil, util.Wrap(err, "Failed to create run.")
+			}
+			apiRun.ResourceReferences = append(apiRun.ResourceReferences, ref)
+			experimentID = ref.GetKey().GetId()
+		}
+	}
+	experiment, err := r.GetExperiment(experimentID)
+	if err != nil {
+		return nil, util.NewInternalServerError(err, "Failed to get experiment.")
+	}
+
+	namespace := experiment.Namespace
 	if len(namespace) == 0 {
 		if multiuserMode {
-			return nil, util.NewInvalidInputError("Run should specify namespace")
+			return nil, util.NewInternalServerError(errors.New("Missing namespace"), "Experiment %v doesn't have a namespace.", experiment.Name)
 		} else {
 			namespace = common.GetPodNamespace()
 		}
@@ -330,15 +350,6 @@ func (r *ResourceManager) CreateRun(apiRun *api.Run) (*model.RunDetail, error) {
 		return nil, util.NewInternalServerError(err, "Failed to create a workflow for (%s)", workflow.Name)
 	}
 
-	// Add a reference to the default experiment if run does not already have a containing experiment
-	ref, err := r.getDefaultExperimentIfNoExperiment(apiRun.GetResourceReferences())
-	if err != nil {
-		return nil, err
-	}
-	if ref != nil {
-		apiRun.ResourceReferences = append(apiRun.ResourceReferences, ref)
-	}
-
 	// Store run metadata into database
 	runDetail, err := r.ToModelRunDetail(apiRun, runId, util.NewWorkflow(newWorkflow), string(workflowSpecManifestBytes))
 	if err != nil {
@@ -780,7 +791,6 @@ func (r *ResourceManager) getWorkflowSpecBytes(spec *api.PipelineSpec) ([]byte,
 		if err != nil {
 			return nil, util.Wrap(err, "Get pipeline YAML failed.")
 		}
-
 		return []byte(workflow.ToStringForStore()), nil
 	} else if spec.GetWorkflowManifest() != "" {
 		return []byte(spec.GetWorkflowManifest()), nil
@@ -849,7 +859,10 @@ func (r *ResourceManager) getDefaultExperimentIfNoExperiment(references []*api.R
 			return nil, nil
 		}
 	}
+	return r.getDefaultExperimentResourceReference(references)
+}
 
+func (r *ResourceManager) getDefaultExperimentResourceReference(references []*api.ResourceReference) (*api.ResourceReference, error) {
 	// Create reference to the default experiment
 	defaultExperimentId, err := r.GetDefaultExperimentId()
 	if err != nil {
@@ -1034,11 +1047,11 @@ func (r *ResourceManager) GetNamespaceFromRunID(runId string) (string, error) {
 	if err != nil {
 		return "", util.Wrap(err, "Failed to get namespace from run id.")
 	}
-	namespace := model.GetNamespaceFromModelResourceReferences(runDetail.ResourceReferences)
+	namespace := runDetail.Namespace
 	if len(namespace) == 0 {
 		if common.IsMultiUserMode() {
 			// All runs should have namespace in multi user mode.
-			return "", errors.New("Invalid db data: run doesn't have a namespace resource reference")
+			return "", errors.New("Invalid db data: run_details doesn't have a namespace")
 		} else {
 			// When db model doesn't have namespace stored (e.g. legacy runs), use
 			// pod namespace as default.