Merge pull request #184 from pixlise/bug/notification-fixes

Bug/notification fixes
pixlise · Mar 19, 2024 · ccf1f49 · ccf1f49
2 parents 75e380a + 31cb390
commit ccf1f49
Show file tree

Hide file tree

Showing 14 changed files with 222 additions and 122 deletions.
diff --git a/.github/workflows/main.yml b/.github/workflows/main.yml
@@ -53,6 +53,21 @@ jobs:
 
   test:
     runs-on: ubuntu-latest
+    needs: [version]
+    # services:
+    #   mongodb:
+    #     image: mongo:4.0.28
+    #     #env:
+    #       #MONGO_INITDB_ROOT_USERNAME: admin
+    #       #MONGO_INITDB_ROOT_PASSWORD: admin
+    #       #MONGO_INITDB_DATABASE: APP-DATABASE
+    #     ports:
+    #       - 27017:27017
+    #     options: >-
+    #       --health-cmd mongo
+    #       --health-interval 10s
+    #       --health-timeout 5s
+    #       --health-retries 5
     steps:
       - uses: actions/checkout@v3
         with:
@@ -62,22 +77,41 @@ jobs:
         uses: actions/setup-go@v3
         with:
           go-version: "1.21.0"
-      - name: Test
-        run: echo $PIXLISE_API_TEST_ZENODO_URI && echo "Tests Off" # make test
+      - name: Start MongoDB
+        uses: supercharge/[email protected]
+        with:
+          mongodb-version: 4.0.28
+          mongodb-replica-set: test
+          mongodb-port: 27017
+      - name: Unit Test
+        run: make unittest
         env:
-          PIXLISE_API_TEST_AUTH0_CLIENT_ID: ${{ secrets.PIXLISE_API_TEST_AUTH0_CLIENT_ID }}
-          PIXLISE_API_TEST_AUTH0_DOMAIN: ${{ secrets.PIXLISE_API_TEST_AUTH0_DOMAIN }}
-          PIXLISE_API_TEST_AUTH0_SECRET: ${{ secrets.PIXLISE_API_TEST_AUTH0_SECRET }}
-          PIXLISE_API_TEST_AUTH0_NEWUSER_ROLE_ID: ${{ secrets.PIXLISE_API_TEST_AUTH0_NEWUSER_ROLE_ID }}
-          PIXLISE_API_TEST_ZENODO_URI: ${{ secrets.ZENODO_URI }}
-          PIXLISE_API_TEST_ZENODO_ACCESS_TOKEN: ${{ secrets.ZENODO_ACCESS_TOKEN }}
+          AWS_DEFAULT_REGION: us-east-1
           AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
+          AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
+      - name: Integration Test
+        run: |
+          make integrationtest
+          ./api-service -customConfigPath ./integration-test-config.json &
+          sleep 5
+          #pwd
+          #ls
+          cd ./internal/cmd-line-tools/api-integration-test
+          #pwd
+          #ls
+          #echo ./tester -apiHost localhost:8080 -datasetBucket "integration-test-data-pixlise" -usersBucket "integration-test-users-pixlise" -auth0Domain ${{ secrets.PIXLISE_API_TEST_AUTH0_DOMAIN }} -auth0ClientId ${{ secrets.PIXLISE_API_TEST_AUTH0_CLIENT_ID }} -auth0Secret ${{ secrets.PIXLISE_API_TEST_AUTH0_SECRET }} -auth0Audience "pixlise-backend" -testType "ci" -test1Username "[email protected]" -test1Password ${{ secrets.TEST_USER_1_PASSWORD }} -test2Username "[email protected]" -test2Password ${{ secrets.TEST_USER_2_PASSWORD }}
+          ./tester -apiHost localhost:8080 -datasetBucket "integration-test-data-pixlise" -usersBucket "integration-test-users-pixlise" -auth0Domain ${{ secrets.PIXLISE_API_TEST_AUTH0_DOMAIN }} -auth0ClientId ${{ secrets.PIXLISE_API_TEST_AUTH0_CLIENT_ID }} -auth0Secret ${{ secrets.PIXLISE_API_TEST_AUTH0_SECRET }} -auth0Audience "pixlise-backend" -testType "ci" -test1Username "[email protected]" -test1Password ${{ secrets.TEST_USER_1_PASSWORD }} -test2Username "[email protected]" -test2Password ${{ secrets.TEST_USER_2_PASSWORD }}
+        env:
           AWS_DEFAULT_REGION: us-east-1
+          AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
           AWS_SECRET_ACCESS_KEY: ${{ secrets.AWS_SECRET_ACCESS_KEY }}
-
+          PIXLISE_CONFIG_Auth0Domain: ${{ secrets.PIXLISE_API_TEST_AUTH0_DOMAIN }}
+          PIXLISE_CONFIG_Auth0ManagementClientID: ${{ secrets.PIXLISE_API_MGMT_AUTH0_CLIENT_ID }}
+          PIXLISE_CONFIG_Auth0ManagementSecret: ${{ secrets.PIXLISE_API_MGMT_AUTH0_SECRET }}
+          PIXLISE_CONFIG_ZenodoAccessToken: ${{ secrets.ZENODO_ACCESS_TOKEN }}
   build:
     runs-on: ubuntu-latest
-    needs: [version, test]
+    needs: [version]
     env:
       AWS_ACCESS_KEY_ID: ${{ secrets.AWS_ACCESS_KEY_ID }}
       AWS_DEFAULT_REGION: us-east-1
@@ -114,7 +148,7 @@ jobs:
           aws s3 cp . s3://corestack-buildartifactsf774a77d-105on4pno9pjm/ --recursive --region us-east-1
 
   docker:
-    needs: [version, test]
+    needs: [version]
     runs-on: ubuntu-latest
     permissions:
       contents: read

diff --git a/Makefile b/Makefile
@@ -4,23 +4,30 @@ PROJECT_NAME := core
 PKG := github.com/pixlise/$(PROJECT_NAME)
 PKG_LIST := $(shell go list ${PKG}/... | grep -v /vendor/)
 
-.PHONY: all build clean test lint
+.PHONY: all build clean unittest integrationtest lint
 
-all: codegen build test lint
+all: codegen build unittest integrationtest lint
 
 lint: ## Lint the files
 	echo "${PKG}"
 	#golint -set_exit_status ${PKG_LIST}
 	golint ${PKG_LIST}
 
-test: ## Run unittests
+unittest: ## Run unittests
 	pwd
 	cd ..
 	mkdir -p _out
 	go install github.com/favadi/protoc-go-inject-tag@latest
 	go run ./data-formats/codegen/main.go -protoPath ./data-formats/api-messages/ -goOutPath ./api/ws/
 	protoc-go-inject-tag -remove_tag_comment -input="./generated-protos/*.pb.go"
-	go test -p 1 -v ./...
+	go test -v ./...
+
+integrationtest:
+	mkdir -p _out
+	echo "version: ${BUILD_VERSION}"
+	echo "sha: ${GITHUB_SHA}"
+	GOOS=linux GOARCH=amd64 go build -ldflags "-X 'github.com/pixlise/core/v4/api/services.ApiVersion=${BUILD_VERSION}' -X 'github.com/pixlise/core/v4/api/services.GitHash=${GITHUB_SHA}'" -v -o ./api-service ./internal/api
+	GOOS=linux GOARCH=amd64 go build -ldflags "-X 'github.com/pixlise/core/v4/api/services.ApiVersion=${BUILD_VERSION}' -X 'github.com/pixlise/core/v4/api/services.GitHash=${GITHUB_SHA}'" -v -o ./internal/cmd-line-tools/api-integration-test/tester ./internal/cmd-line-tools/api-integration-test
 
 codegen:
 	./genproto.sh checkgen

diff --git a/api/dataimport/for-trigger_test.go b/api/dataimport/for-trigger_test.go
@@ -67,6 +67,14 @@ func initTest(testDir string, autoShareCreatorId string, autoShareCreatorGroupEd
 	return remoteFS, log, envName, configBucket, datasetBucket, manualBucket, db
 }
 
+/*
+func startTestWithMockMongo(name string, t *testing.T, testFunc func(mt *mtest.T)) {
+	mt := mtest.New(t, mtest.NewOptions().ClientType(mtest.Mock))
+	defer mt.Close()
+
+	mt.Run(name, testFunc)
+}
+*/
 // Import unknown dataset (simulate trigger by OCS pipeline), file goes to archive, then all files downloaded from archive, dataset create fails due to unknown data type
 func Example_importForTrigger_OCS_Archive_BadData() {
 	remoteFS, log, envName, configBucket, datasetBucket, manualBucket, db := initTest("Archive_BadData", specialUserIds.PIXLISESystemUserId, "PIXLFMGroupId")

diff --git a/api/endpoints/middlewareLogger_test.go b/api/endpoints/middlewareLogger_test.go
@@ -15,7 +15,6 @@ import (
 	"github.com/pixlise/core/v4/core/jwtparser"
 	"github.com/pixlise/core/v4/core/logger"
 	"github.com/pixlise/core/v4/core/timestamper"
-	"go.mongodb.org/mongo-driver/mongo/integration/mtest"
 )
 
 func Test_testLoggingDebug(t *testing.T) {
@@ -28,82 +27,65 @@ func Test_testLoggingInfo(t *testing.T) {
 }
 
 func runMiddlewareLoggingTest(t *testing.T, logLevel *logger.LogLevel) {
-	mt := mtest.New(t, mtest.NewOptions().ClientType(mtest.Mock))
-	defer mt.Close()
-
-	mt.Run("success", func(mt *mtest.T) {
-		//mt.AddMockResponses()
-
-		var mockS3 awsutil.MockS3Client
-		defer mockS3.FinishTest()
-
-		mockS3.ExpPutObjectInput = []s3.PutObjectInput{
-			{
-				Bucket: aws.String(UsersBucketForUnitTest), Key: aws.String("Activity/2022-11-11/id-123.json"), Body: bytes.NewReader([]byte(`{
-    "Instance": "",
-    "Time": "2022-11-11T04:56:19Z",
-    "Component": "/foo",
-    "Message": "the bodyyy",
-    "Response": "{\"alive\": true}",
-    "Version": "",
-    "Params": {
-        "method": "GET"
-    },
-    "Environment": "unit-test",
-    "User": "myuserid"
+	var mockS3 awsutil.MockS3Client
+	defer mockS3.FinishTest()
+
+	mockS3.ExpPutObjectInput = []s3.PutObjectInput{
+		{
+			Bucket: aws.String(UsersBucketForUnitTest), Key: aws.String("Activity/2022-11-11/id-123.json"), Body: bytes.NewReader([]byte(`{
+"Instance": "",
+"Time": "2022-11-11T04:56:19Z",
+"Component": "/foo",
+"Message": "the bodyyy",
+"Response": "{\"alive\": true}",
+"Version": "",
+"Params": {
+	"method": "GET"
+},
+"Environment": "unit-test",
+"User": "myuserid"
 }`)),
-			},
-		}
-		mockS3.QueuedPutObjectOutput = []*s3.PutObjectOutput{
-			{},
-		}
-
-		idGen := idgen.MockIDGenerator{
-			IDs: []string{"id-123"},
-		}
-
-		s := MakeMockSvcs(&mockS3, &idGen, logLevel)
-		s.TimeStamper = &timestamper.MockTimeNowStamper{
-			QueuedTimeStamps: []int64{1668142579},
-		}
-		/*
-			notifications, err := notifications.MakeNotificationStack(mt.Client, "unit_test", nil, &logger.StdOutLoggerForTest{}, []string{})
-			if err != nil {
-				t.Error(err)
-			}
-
-			s.Notifications = notifications
-
-			// Add requestor as a tracked user, so we should see activity saved
-			s.Notifications.SetTrack("myuserid", true)
-		*/
-		mockvalidator := jwtparser.MockJWTValidator{}
-		l := LoggerMiddleware{
-			APIServices:  &s,
-			JwtValidator: &mockvalidator,
-		}
-
-		handler := func(w http.ResponseWriter, r *http.Request) {
-			w.WriteHeader(http.StatusOK)
-			w.Header().Set("Content-Type", "application/json")
-
-			// In the future we could report back on the status of our DB, or our cache
-			// (e.g. Redis) by performing a simple PING, and include them in the response.
-			io.WriteString(w, `{"alive": true}`)
-		}
-
-		req := httptest.NewRequest("GET", "http://example.com/foo", bytes.NewReader([]byte("the bodyyy")))
-		w := httptest.NewRecorder()
-		handler(w, req)
-
-		h := http.HandlerFunc(handler)
-		handlerToTest := l.Middleware(h)
-
-		handlerToTest.ServeHTTP(httptest.NewRecorder(), req)
-
-		// Wait a bit for any threads to finish
-		time.Sleep(2 * time.Second)
-
-		checkResult(t, w, 200, "{\"alive\": true}")
-	})
+		},
+	}
+	mockS3.QueuedPutObjectOutput = []*s3.PutObjectOutput{
+		{},
+	}
+
+	idGen := idgen.MockIDGenerator{
+		IDs: []string{"id-123"},
+	}
+
+	s := MakeMockSvcs(&mockS3, &idGen, logLevel)
+	s.TimeStamper = &timestamper.MockTimeNowStamper{
+		QueuedTimeStamps: []int64{1668142579},
+	}
+
+	mockvalidator := jwtparser.MockJWTValidator{}
+	l := LoggerMiddleware{
+		APIServices:  &s,
+		JwtValidator: &mockvalidator,
+	}
+
+	handler := func(w http.ResponseWriter, r *http.Request) {
+		w.WriteHeader(http.StatusOK)
+		w.Header().Set("Content-Type", "application/json")
+
+		// In the future we could report back on the status of our DB, or our cache
+		// (e.g. Redis) by performing a simple PING, and include them in the response.
+		io.WriteString(w, `{"alive": true}`)
+	}
+
+	req := httptest.NewRequest("GET", "http://example.com/foo", bytes.NewReader([]byte("the bodyyy")))
+	w := httptest.NewRecorder()
+	handler(w, req)
+
+	h := http.HandlerFunc(handler)
+	handlerToTest := l.Middleware(h)
+
+	handlerToTest.ServeHTTP(httptest.NewRecorder(), req)
+
+	// Wait a bit for any threads to finish
+	time.Sleep(2 * time.Second)
+
+	checkResult(t, w, 200, "{\"alive\": true}")
 }
diff --git a/api/notificationSender/interfaceImplementation.go b/api/notificationSender/interfaceImplementation.go
@@ -141,6 +141,12 @@ func (n *NotificationSender) NotifyObjectShared(objectType string, objectId stri
 }
 
 func (n *NotificationSender) NotifyUserGroupMessage(subject string, message string, notificationType protos.NotificationType, actionLink string, groupId string, groupName string, sender string) {
+	userIds, err := wsHelpers.GetUserIdsForGroup([]string{groupId}, n.db)
+	if err != nil {
+		n.log.Errorf("Failed to get user ids for group: %v. Error: %v", groupId, err)
+		return
+	}
+
 	notifMsg := &protos.NotificationUpd{
 		Notification: &protos.Notification{
 			NotificationType: notificationType,
@@ -151,12 +157,6 @@ func (n *NotificationSender) NotifyUserGroupMessage(subject string, message stri
 		},
 	}
 
-	userIds, err := wsHelpers.GetUserIdsForGroup([]string{groupId}, n.db)
-	if err != nil {
-		n.log.Errorf("Failed to get user ids for group: %v. Error: %v", groupId, err)
-		return
-	}
-
 	n.sendNotification(subject, "", notifMsg, userIds)
 }
 

diff --git a/api/notificationSender/notifications.go b/api/notificationSender/notifications.go
@@ -91,7 +91,7 @@ func (n *NotificationSender) sendNotification(sourceId string, topicId string, n
 
 	for _, userId := range userIds {
 		// Write it to DB if needed
-		err := n.saveNotificationToDB(userId, notifMsg.Notification)
+		err := n.saveNotificationToDB(origId, userId, notifMsg.Notification)
 		if err != nil {
 			n.log.Errorf("Failed to save notification to DB for user: %v. Error: \"%v\". Notification was: %+v", userId, err, notifMsg.Notification)
 		}
@@ -129,7 +129,7 @@ func (n *NotificationSender) sendNotification(sourceId string, topicId string, n
 		for _, session := range sessions {
 			// Send it with a unique ID for this user
 			sessUser, err := wsHelpers.GetSessionUser(session)
-			if err != nil {
+			if err == nil {
 				notifMsg.Notification.Id = origId + "-" + sessUser.User.Id
 				msg := &protos.WSMessage{Contents: &protos.WSMessage_NotificationUpd{NotificationUpd: notifMsg}}
 
@@ -201,12 +201,12 @@ func (n *NotificationSender) sendEmail(notif *protos.Notification, userId string
 	awsutil.SESSendEmail(user.Info.Email, "UTF-8", text, html, notif.Subject, "[email protected]", []string{}, []string{})
 }
 
-func (n *NotificationSender) saveNotificationToDB(destUserId string, notification *protos.Notification) error {
+func (n *NotificationSender) saveNotificationToDB(notifId string, destUserId string, notification *protos.Notification) error {
 	// Make a copy which has the user id set
 	toSave := &protos.Notification{
 		DestUserId: destUserId,
 
-		Id:               notification.Id,
+		Id:               notifId + "-" + destUserId,
 		DestUserGroupId:  notification.DestUserGroupId,
 		MaxSecToExpiry:   notification.MaxSecToExpiry,
 		Subject:          notification.Subject,

diff --git a/api/ws/handlers/notification.go b/api/ws/handlers/notification.go
@@ -43,7 +43,7 @@ func HandleNotificationReq(req *protos.NotificationReq, hctx wsHelpers.HandlerCo
 
 func HandleNotificationDismissReq(req *protos.NotificationDismissReq, hctx wsHelpers.HandlerContext) (*protos.NotificationDismissResp, error) {
 	// Find this in the DB and clear it
-	if err := wsHelpers.CheckStringField(&req.Id, "Id", 1, wsHelpers.IdFieldMaxLength); err != nil {
+	if err := wsHelpers.CheckStringField(&req.Id, "Id", 1, wsHelpers.IdFieldMaxLength*2); err != nil {
 		return nil, err
 	}
 

diff --git a/integration-test-config.json b/integration-test-config.json
@@ -0,0 +1,29 @@
+{
+    "MaxQuantNodes": 20,
+    "AWSBucketRegion": "us-east-1",
+    "AWSCloudwatchRegion": "us-east-1",
+    "EnvironmentName": "unittest",
+    "LogLevel": 1,
+    "CoresPerNode": 4,
+    "PiquantDockerImage": "registry.gitlab.com/pixlise/piquant/runner:3.2.17",
+    "QuantNamespace": "unittest",
+    "QuantExecutor": "docker",
+    "SentryEndpoint": "",
+
+    "DatasetsBucket": "integration-test-data-pixlise",
+    "UsersBucket": "integration-test-users-pixlise",
+    "ConfigBucket": "integration-test-config-pixlise",
+    "PiquantJobsBucket": "integration-test-jobs-pixlise",
+    "ManualUploadBucket": "integration-test-upload-pixlise",
+
+    "BuildsBucket": "",
+    "DataSourceSNSTopic": "",
+
+	"WSWriteWaitMs": 10000,
+	"WSPongWaitMs": 60000,
+	"WSPingPeriodMs": 54000,
+	"WSMaxMessageSize": 40000,
+	"WSMessageBufferSize": 256,
+
+	"ZenodoURI": "https://sandbox.zenodo.org"
+}
diff --git a/internal/api/main.go b/internal/api/main.go
@@ -192,6 +192,7 @@ func loadConfig() config.APIConfig {
 	}
 
 	cfgStr := string(cfgJSON)
+	log.Println("API startup configuration:")
 	log.Println(cfgStr)
 	return cfg
 }