Repository: moov-io/ach
Pull request #895 (2021-04-14 23:35:09)

merge: count individual records rather than write/render File
This comes about where trying to Write an invalid file fails so MergeFiles can often fail.

Fixes: https://github.com/moov-io/ach/issues/862


Patch:
diff --git a/merge.go b/merge.go
index 574ef2fc..dc50af5d 100644
--- a/merge.go
+++ b/merge.go
@@ -18,8 +18,6 @@
 package ach
 
 import (
-	"bufio"
-	"bytes"
 	"fmt"
 	"time"
 )
@@ -133,21 +131,59 @@ func (fs *mergableFiles) lookupByHeader(f *File) *File {
 }
 
 func lineCount(f *File) (int, error) {
-	if len(f.Batches) < 100 {
-		// Ignore Files with low batch counts by returning a valid count.
-		// Calling Writer.Write() is costly and so we're going to ignore it in easy cases.
-		return 1, nil
-	}
-
-	var buf bytes.Buffer
-	if err := NewWriter(&buf).Write(f); err != nil {
-		return 0, err
+	lines := 2 // FileHeader, FileControl
+	for i := range f.Batches {
+		lines += 2 // BatchHeader, BatchControl
+		entries := f.Batches[i].GetEntries()
+		for j := range entries {
+			lines++
+			if entries[j].Addenda02 != nil {
+				lines++
+			}
+			lines += len(entries[j].Addenda05)
+			if entries[j].Addenda98 != nil {
+				lines++
+			}
+			if entries[j].Addenda99 != nil {
+				lines++
+			}
+		}
 	}
-	lines := 0
-	s := bufio.NewScanner(&buf)
-	for s.Scan() {
-		if v := s.Text(); v != "" {
+	for i := range f.IATBatches {
+		lines += 2 // IATBatchHeader, BatchControl
+		for j := range f.IATBatches[i].Entries {
 			lines++
+			if f.IATBatches[i].Entries[j].Addenda10 != nil {
+				lines++
+			}
+			if f.IATBatches[i].Entries[j].Addenda11 != nil {
+				lines++
+			}
+			if f.IATBatches[i].Entries[j].Addenda12 != nil {
+				lines++
+			}
+			if f.IATBatches[i].Entries[j].Addenda13 != nil {
+				lines++
+			}
+			if f.IATBatches[i].Entries[j].Addenda14 != nil {
+				lines++
+			}
+			if f.IATBatches[i].Entries[j].Addenda15 != nil {
+				lines++
+			}
+			if f.IATBatches[i].Entries[j].Addenda16 != nil {
+				lines++
+			}
+
+			lines += len(f.IATBatches[i].Entries[j].Addenda17)
+			lines += len(f.IATBatches[i].Entries[j].Addenda18)
+
+			if f.IATBatches[i].Entries[j].Addenda98 != nil {
+				lines++
+			}
+			if f.IATBatches[i].Entries[j].Addenda99 != nil {
+				lines++
+			}
 		}
 	}
 	return lines, nil


Test patch:
diff --git a/merge_test.go b/merge_test.go
index eaae7ff0..053c38ea 100644
--- a/merge_test.go
+++ b/merge_test.go
@@ -19,10 +19,16 @@ package ach
 
 import (
 	"fmt"
+	"math/rand"
 	"path/filepath"
 	"testing"
+	"time"
 )
 
+func init() {
+	rand.Seed(time.Now().Unix())
+}
+
 func filesAreEqual(f1, f2 *File) error {
 	// File Header
 	if f1.Header.ImmediateOrigin != f2.Header.ImmediateOrigin {
@@ -179,6 +185,35 @@ func TestMergeFiles__apart(t *testing.T) {
 	}
 }
 
+func BenchmarkMergeFiles__lineCount(b *testing.B) {
+	newACHFile := func() *File {
+		// Nacha files have a max of 10,000 lines and a batch is
+		// a header, entries, and control.
+		batches := int(rand.Int31n(3000) + 1)
+
+		file := NewFile()
+		file.SetHeader(mockFileHeader())
+		file.Control = mockFileControl()
+
+		for i := 0; i < batches; i++ {
+			file.AddBatch(mockBatchPPD())
+		}
+		if err := file.Create(); err != nil {
+			b.Fatal(err)
+		}
+		return file
+	}
+
+	for i := 0; i < b.N; i++ {
+		b.StopTimer() // pause timer so we can init our ACH file
+		file := newACHFile()
+		b.StartTimer() // resume benchmark
+
+		// Count lines in our file
+		lineCount(file)
+	}
+}
+
 func TestMergeFiles__lineCount(t *testing.T) {
 	file, err := readACHFilepath(filepath.Join("test", "testdata", "ppd-debit.ach"))
 	if err != nil {
@@ -188,7 +223,7 @@ func TestMergeFiles__lineCount(t *testing.T) {
 		t.Fatal(err)
 	}
 
-	if n, err := lineCount(file); n != 1 || err != nil {
+	if n, err := lineCount(file); n != 5 || err != nil {
 		// We've optimized small file line counts to bypass writing out the file
 		// into plain text as it's costly.
 		t.Errorf("did we change optimizations? n=%d error=%v", n, err)
@@ -200,14 +235,14 @@ func TestMergeFiles__lineCount(t *testing.T) {
 	if err := file.Create(); err != nil {
 		t.Fatal(err)
 	}
-	if n, err := lineCount(file); n != 310 || err != nil {
+	if n, err := lineCount(file); n != 305 || err != nil {
 		t.Errorf("unexpected line count of %d: %v", n, err)
 	}
 
-	// make the file invalid and ensure we error
+	// Remove BatchCount and still properly count lines
 	file.Control.BatchCount = 0
-	if n, err := lineCount(file); n != 0 || err == nil {
-		t.Errorf("expected error n=%d error=%v", n, err)
+	if n, err := lineCount(file); n != 305 || err != nil {
+		t.Errorf("unexpected error n=%d error=%v", n, err)
 	}
 }