Repository: moov-io/ach
Pull request #931 (2021-05-13 16:25:02)

merge: combine entries together when Batch Headers match
Issue: https://github.com/moov-io/ach/issues/927

codecov-commenter: # [Codecov](https://codecov.io/gh/moov-io/ach/pull/931?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io) Report
> Merging [#931](https://codecov.io/gh/moov-io/ach/pull/931?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io) (a1084e7) into [master](https://codecov.io/gh/moov-io/ach/commit/2a47d01f8116c36603a64c189fe67c61b4e8b586?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=moov-io) (2a47d01) will **decrease** coverage by `0.42%`.
> The diff coverage is `82.35%`.

> :exclamation: Current head a1084e7 differs from pull request most recent head 9814ac7. Consider uploading reports for the commit 9814ac7 to get more accurate results

```diff
@@            Coverage Diff             @@
##           master     #931      +/-   ##
==========================================
- Coverage   91.83%   91.40%   -0.43%     
==========================================
  Files          66       66              
  Lines        5204     5236      +32     
==========================================
+ Hits         4779     4786       +7     
- Misses        258      280      +22     
- Partials      167      170       +3     
```


Patch:
diff --git a/batch.go b/batch.go
index b18fa676..72630a28 100644
--- a/batch.go
+++ b/batch.go
@@ -1021,23 +1021,7 @@ func (batch *Batch) Equal(other Batcher) bool {
 	if batch == nil || other == nil || batch.Header == nil || other.GetHeader() == nil {
 		return false
 	}
-	ohead := other.GetHeader()
-	if batch.Header.ServiceClassCode != ohead.ServiceClassCode {
-		return false
-	}
-	if !strings.EqualFold(batch.Header.CompanyName, ohead.CompanyName) {
-		return false
-	}
-	if batch.Header.CompanyIdentification != ohead.CompanyIdentification {
-		return false
-	}
-	if batch.Header.StandardEntryClassCode != ohead.StandardEntryClassCode {
-		return false
-	}
-	if batch.Header.EffectiveEntryDate != ohead.EffectiveEntryDate {
-		return false
-	}
-	if batch.Header.ODFIIdentification != ohead.ODFIIdentification {
+	if !batch.Header.Equal(other.GetHeader()) {
 		return false
 	}
 	oentries := other.GetEntries()
diff --git a/batchHeader.go b/batchHeader.go
index e490d51d..a53c1728 100644
--- a/batchHeader.go
+++ b/batchHeader.go
@@ -25,8 +25,6 @@ import (
 	"unicode/utf8"
 )
 
-// msgServiceClass
-
 // BatchHeader identifies the originating entity and the type of transactions
 // contained in the batch (i.e., the standard entry class, PPD for consumer, CCD
 // or CTX for corporate). This record also contains the effective date, or desired
@@ -213,6 +211,30 @@ func (bh *BatchHeader) String() string {
 	return buf.String()
 }
 
+// Equal returns true only if two BatchHeaders are equal.
+// Equality is determined by the Nacha defined fields of each record.
+func (bh *BatchHeader) Equal(other *BatchHeader) bool {
+	if bh.ServiceClassCode != other.ServiceClassCode {
+		return false
+	}
+	if !strings.EqualFold(bh.CompanyName, other.CompanyName) {
+		return false
+	}
+	if bh.CompanyIdentification != other.CompanyIdentification {
+		return false
+	}
+	if bh.StandardEntryClassCode != other.StandardEntryClassCode {
+		return false
+	}
+	if bh.EffectiveEntryDate != other.EffectiveEntryDate {
+		return false
+	}
+	if bh.ODFIIdentification != other.ODFIIdentification {
+		return false
+	}
+	return true
+}
+
 // SetValidation stores ValidateOpts on the BatchHeader which are to be used to override
 // the default NACHA validation rules.
 func (bh *BatchHeader) SetValidation(opts *ValidateOpts) {
diff --git a/merge.go b/merge.go
index 156d0214..f4f46814 100644
--- a/merge.go
+++ b/merge.go
@@ -47,6 +47,13 @@ func MergeFiles(files []*File) ([]*File, error) {
 			for k := range outf.Batches {
 				if fs.infiles[i].Batches[j].Equal(outf.Batches[k]) {
 					batchExistsInMerged = true
+
+					batch, err := mergeEntries(outf.Batches[k], fs.infiles[i].Batches[j])
+					if err != nil {
+						return nil, err
+					}
+					outf.Batches[k] = batch
+					break
 				}
 			}
 			if !batchExistsInMerged {
@@ -66,7 +73,7 @@ func MergeFiles(files []*File) ([]*File, error) {
 					f := *outf
 					fs.locMaxed = append(fs.locMaxed, &f)
 
-					outf = fs.create(outf) // replace output file with the one we just created
+					outf = fs.swapLocMaxedFile(outf) // replace output file with the one we just created
 
 					outf.AddBatch(fs.infiles[i].Batches[j])
 					if err := outf.Create(); err != nil {
@@ -85,9 +92,16 @@ func MergeFiles(files []*File) ([]*File, error) {
 		for i, b := range f.Batches {
 			b.GetHeader().BatchNumber = i + 1
 			b.GetControl().BatchNumber = i + 1
+			// Tabulate each Batch after combining them
+			if err := b.Create(); err != nil {
+				return out, err
+			}
+		}
+		// Tabulate the files before returning them
+		if err := f.Create(); err != nil {
+			return out, err
 		}
 	}
-
 	return out, nil
 }
 
@@ -97,8 +111,9 @@ type mergableFiles struct {
 	locMaxed []*File
 }
 
-// create returns the index of a newly created file in fs.outfiles given the details from f.Header
-func (fs *mergableFiles) create(f *File) *File { // returns the outfiles index of the created file
+// swapLocMaxedFile replaces an ACH file that is over the Nacha line limit with an empty file containing
+// a matching FileHeader record. This allows future iterations inside of MergeFiles to append
+func (fs *mergableFiles) swapLocMaxedFile(f *File) *File {
 	now := time.Now()
 
 	// remove the current file from outfiles
@@ -151,13 +166,29 @@ func (fs *mergableFiles) findOutfile(f *File) *File {
 				return fs.outfiles[i]
 			}
 		}
-		// Record a newly mergable file we can use in future merge attempts
-		fs.outfiles = append(fs.outfiles, f)
-		return f
+		// Record a newly mergable File/FileHeader we can use in future merge attempts
+		outf := NewFile()
+		outf.Header = f.Header
+		outf.Control = f.Control
+		fs.outfiles = append(fs.outfiles, outf)
+		return outf
 	}
 	return lookup(0)
 }
 
+func mergeEntries(b1, b2 Batcher) (Batcher, error) {
+	b, _ := NewBatch(b1.GetHeader())
+	entries := sortEntriesByTraceNumber(append(b1.GetEntries(), b2.GetEntries()...))
+	for i := range entries {
+		b.AddEntry(entries[i])
+	}
+	b.SetControl(b1.GetControl())
+	if err := b.Create(); err != nil {
+		return nil, err
+	}
+	return b, nil
+}
+
 func lineCount(f *File) (int, error) {
 	lines := 2 // FileHeader, FileControl
 	for i := range f.Batches {


Test patch:
diff --git a/merge_test.go b/merge_test.go
index 053c38ea..e78b463e 100644
--- a/merge_test.go
+++ b/merge_test.go
@@ -21,6 +21,7 @@ import (
 	"fmt"
 	"math/rand"
 	"path/filepath"
+	"strconv"
 	"testing"
 	"time"
 )
@@ -289,25 +290,41 @@ func TestMergeFiles__splitFiles(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	// Merge our big file into another file and verify we get two back
-	// TODO(adam): We should probably recurse back on `file` to ensure we don't exceed the
-	// 10k line limit. That shouldn't happen as MergeFiles processes one batch at a time, but
-	// an incoming file might be invalid in that way.
+	traceNumbersBefore := countTraceNumbers(file, f2, f3)
+
 	out, err := MergeFiles([]*File{file, f2, f3})
-	if err != nil || len(out) != 2 {
+	if err != nil || len(out) != 1 {
 		t.Fatalf("got %d files, error=%v", len(out), err)
 	}
-	if len(out[0].Batches) != 4001 || len(out[1].Batches) != 5 {
-		// These batch counts will change when we recurse back through out[0]
-		// so it doesn't exceed the 10k line limit.
-		t.Errorf("out[0].Batches:%d out[1].Batches:%d", len(out[0].Batches), len(out[1].Batches))
+	if n := len(out[0].Batches); n != 2006 {
+		t.Fatalf("out[0] has %d batches", n)
+	}
+
+	traceNumbersAfter := countTraceNumbers(out...)
+	if traceNumbersBefore != traceNumbersAfter {
+		t.Fatalf("found %d of %d trace numbers", traceNumbersBefore, traceNumbersAfter)
 	}
 
 	for _, f := range out {
 		if err := f.Validate(); err != nil {
 			t.Fatalf("invalid file: %v", err)
 		}
+		min, err := f.FlattenBatches()
+		if err != nil {
+			t.Fatal(err)
+		}
+		if err := min.Validate(); err != nil {
+			t.Fatal(err)
+		}
+	}
+}
+
+func countTraceNumbers(files ...*File) int {
+	var total int
+	for i := range files {
+		total += len(getTraceNumbers(files[i]))
 	}
+	return total
 }
 
 func TestMergeFiles__invalid(t *testing.T) {
@@ -333,6 +350,7 @@ func populateFileWithMockBatches(t *testing.T, numBatches int, file *File) {
 	lastBatchIdx := len(file.Batches) - 1
 	var startSeq = file.Batches[lastBatchIdx].GetHeader().BatchNumber + 1
 	var entryDetail = file.Batches[0].GetEntries()[0]
+
 	for i := startSeq; i < (numBatches + startSeq); i++ {
 		header := mockBatchHeader()
 		header.StandardEntryClassCode = "PPD"
@@ -346,7 +364,11 @@ func populateFileWithMockBatches(t *testing.T, numBatches int, file *File) {
 			t.Fatal(err)
 		}
 
-		batch.AddEntry(entryDetail)
+		ed := *entryDetail
+		n, _ := strconv.Atoi(ed.TraceNumber)
+		ed.TraceNumber = strconv.Itoa(n + i + 1e5)
+		batch.AddEntry(&ed)
+
 		batch.GetHeader().BatchNumber = i
 		batch.GetControl().BatchNumber = i