BCDA-7480: Update errors returned from import-cclf (#885)

## 🎫 Ticket

https://jira.cms.gov/browse/BCDA-7480

## 🛠 Changes

Updating `import-cclf` command to return an error when at least one file
has failed to be imported or a file has been skipped, or if the
successfully imported file count is 0.

Smoke tests are failing because the suppression files get added to the
skipped count, if they exist in the directory. Since we don't want false
positives, a check has been added to see if the file we are currently
importing is an opt out file. If it is, we ignore it altogether and
don't increment the count of the files skipped.

## ℹ️ Context for reviewers

Smoke tests in dev are failing because opt-out files are being added to
the skipped count, which causes the synthetic EFT import to fail.

## ✅ Acceptance Validation

modified tests that import cclf files to also check for opt out files
and not add to skipped count.

## 🔒 Security Implications

- [ ] This PR adds a new software dependency or dependencies.
- [ ] This PR modifies or invalidates one or more of our security
controls.
- [ ] This PR stores or transmits data that was not stored or
transmitted before.
- [ ] This PR requires additional review of its security implications
for other reasons.

If any security implications apply, add Jason Ashbaugh (GitHub username:
StewGoin) as a reviewer and do not merge this PR without his approval.

bcda/bcdacli/cli_test.go

@@ -29,6 +29,7 @@ import (
 	"github.com/CMSgov/bcda-app/bcda/utils"
 	"github.com/CMSgov/bcda-app/conf"
 	logger "github.com/CMSgov/bcda-app/log"
+	"github.com/pkg/errors"
 
 	"github.com/go-chi/chi/v5"
 	"github.com/pborman/uuid"
@@ -703,31 +704,47 @@ func (s *CLITestSuite) TestImportCCLFDirectory() {
 	assert := assert.New(s.T())
 	hook := test.NewLocal(testUtils.GetLogger(logger.API))
 
-	postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, targetACO)
-	defer postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, targetACO)
+	type test struct {
+		path         string
+		err          error
+		expectedLogs []string
+	}
 
-	path, cleanup := testUtils.CopyToTemporaryDirectory(s.T(), "../../shared_files/cclf/archives/valid2/")
-	defer cleanup()
+	tests := []test{
+		{path: "../../shared_files/cclf/archives/valid2/", err: errors.New("Files skipped or failed import. See logs for more details."), expectedLogs: []string{"Successfully imported 2 files.", "Failed to import 0 files.", "Skipped 1 files."}},
+		{path: "../../shared_files/cclf/archives/invalid_bcd/", err: errors.New("one or more files failed to import correctly"), expectedLogs: []string{"error returned from ImportCCLFDirectory", "", ""}},
+		{path: "../../shared_files/cclf/archives/skip/", err: errors.New("Files failed to import or no files were imported. See logs for more details."), expectedLogs: []string{"Successfully imported 0 files.", "Failed to import 0 files.", "Skipped 1 files."}},
+	}
 
-	args := []string{"bcda", "import-cclf-directory", constants.DirectoryArg, path}
-	err := s.testApp.Run(args)
-	assert.NotNil(err)
-	var success, failed, skipped bool
-	for _, entry := range hook.AllEntries() {
-		if strings.Contains(entry.Message, "Successfully imported 2 files.") {
-			success = true
-		}
-		if strings.Contains(entry.Message, "Failed to import 0 files.") {
-			failed = true
+	for _, tc := range tests {
+		postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, targetACO)
+		defer postgrestest.DeleteCCLFFilesByCMSID(s.T(), s.db, targetACO)
+		path, cleanup := testUtils.CopyToTemporaryDirectory(s.T(), tc.path)
+		defer cleanup()
+		args := []string{"bcda", "import-cclf-directory", constants.DirectoryArg, path}
+		err := s.testApp.Run(args)
+		if tc.err == nil {
+			assert.Nil(err)
+		} else {
+			assert.NotNil(err)
 		}
-		if strings.Contains(entry.Message, "Skipped 1 files.") {
-			skipped = true
+
+		var success, failed, skipped bool
+		for _, entry := range hook.AllEntries() {
+			if strings.Contains(entry.Message, tc.expectedLogs[0]) {
+				success = true
+			}
+			if strings.Contains(entry.Message, tc.expectedLogs[1]) {
+				failed = true
+			}
+			if strings.Contains(entry.Message, tc.expectedLogs[2]) {
+				skipped = true
+			}
 		}
+		assert.True(success)
+		assert.True(failed)
+		assert.True(skipped)
 	}
-	assert.True(success)
-	assert.True(failed)
-	assert.True(skipped)
-
 }
 
 func (s *CLITestSuite) TestDeleteDirectoryContents() {

bcda/cclf/fileprocessor.go

@@ -14,6 +14,7 @@ import (
 	"github.com/CMSgov/bcda-app/bcda/utils"
 	"github.com/CMSgov/bcda-app/conf"
 	"github.com/CMSgov/bcda-app/log"
+	"github.com/CMSgov/bcda-app/optout"
 
 	"github.com/pkg/errors"
 )
@@ -56,8 +57,17 @@ func (p *processor) walk(path string, info os.FileInfo, err error) error {
 		return nil
 	}
 
+	// ignore the opt out file, and don't add it to the skipped count
+	optOut, _ := optout.IsOptOut(info.Name())
+	if optOut {
+		fmt.Print("Skipping opt-out file: ", info.Name())
+		log.API.Info("Skipping opt-out file: ", info.Name())
+		return nil
+	}
+
 	zipReader, err := zip.OpenReader(filepath.Clean(path))
 	if err != nil {
+
 		p.skipped = p.skipped + 1
 		msg := fmt.Sprintf("Skipping %s: file could not be opened as a CCLF archive. %s", path, err.Error())
 		fmt.Println(msg)

bcda/cclf/fileprocessor_test.go

@@ -66,7 +66,7 @@ func (s *FileProcessorTestSuite) TestProcessCCLFArchives() {
 	}{
 		{filepath.Join(s.basePath, "cclf/archives/valid/"), 2, 1, 1, 1},
 		{filepath.Join(s.basePath, "cclf/archives/bcd/"), 2, 1, 1, 1},
-		{filepath.Join(s.basePath, "cclf/mixed/with_invalid_filenames/"), 2, 5, 1, 1},
+		{filepath.Join(s.basePath, "cclf/mixed/with_invalid_filenames/"), 2, 4, 1, 1},
 		{filepath.Join(s.basePath, "cclf/mixed/0/valid_names/"), 3, 3, 3, 0},
 		{filepath.Join(s.basePath, "cclf/archives/8/valid/"), 5, 0, 0, 5},
 		{filepath.Join(s.basePath, "cclf/files/9/valid_names/"), 0, 4, 0, 0},
@@ -112,7 +112,7 @@ func (s *FileProcessorTestSuite) TestProcessCCLFArchives_ExpireFiles() {
 	assert.Nil(err)
 	cclfList := cclfMap["A0001"][key]
 	assert.Equal(2, len(cclfList))
-	assert.Equal(5, skipped)
+	assert.Equal(4, skipped)
 	// assert that this file is still here.
 	_, err = os.Open(filePath)
 	assert.Nil(err)
@@ -127,7 +127,7 @@ func (s *FileProcessorTestSuite) TestProcessCCLFArchives_ExpireFiles() {
 	assert.Nil(err)
 	cclfList = cclfMap["A0001"][key]
 	assert.Equal(2, len(cclfList))
-	assert.Equal(5, skipped)
+	assert.Equal(4, skipped)
 
 	// assert that this file is not still here.
 	_, err = os.Open(filePath)

optout/utils.go

@@ -24,17 +24,14 @@ const (
 
 func ParseMetadata(filename string) (OptOutFilenameMetadata, error) {
 	var metadata OptOutFilenameMetadata
-	// Beneficiary Data Sharing Preferences File sent by 1-800-Medicare: P#EFT.ON.ACO.NGD1800.DPRF.Dyymmdd.Thhmmsst
-	// Prefix: T = test, P = prod;
-	filenameRegexp := regexp.MustCompile(`((P|T)\#EFT)\.ON\.ACO\.NGD1800\.DPRF\.(D\d{6}\.T\d{6})\d`)
-	filenameMatches := filenameRegexp.FindStringSubmatch(filename)
-	if len(filenameMatches) < 4 {
+	isOptOut, matches := IsOptOut(filename)
+	if !isOptOut {
 		fmt.Printf("Invalid filename for file: %s.\n", filename)
 		err := fmt.Errorf("invalid filename for file: %s", filename)
 		return metadata, err
 	}
 
-	filenameDate := filenameMatches[3]
+	filenameDate := matches[3]
 	t, err := time.Parse("D060102.T150405", filenameDate)
 	if err != nil || t.IsZero() {
 		fmt.Printf("Failed to parse date '%s' from file: %s.\n", filenameDate, filename)
@@ -43,11 +40,20 @@ func ParseMetadata(filename string) (OptOutFilenameMetadata, error) {
 	}
 
 	metadata.Timestamp = t
-	metadata.Name = filenameMatches[0]
+	metadata.Name = matches[0]
 
 	return metadata, nil
 }
 
+func IsOptOut(filename string) (isOptOut bool, matches []string) {
+	filenameRegexp := regexp.MustCompile(`((P|T)\#EFT)\.ON\.ACO\.NGD1800\.DPRF\.(D\d{6}\.T\d{6})\d`)
+	matches = filenameRegexp.FindStringSubmatch(filename)
+	if len(matches) > 3 {
+		isOptOut = true
+	}
+	return isOptOut, matches
+}
+
 func ParseRecord(metadata *OptOutFilenameMetadata, b []byte) (*OptOutRecord, error) {
 	ds := string(bytes.TrimSpace(b[effectiveDtStart:effectiveDtEnd]))
 	dt, err := ConvertDt(ds)