Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

adding ValidateOpts #295

Open
wants to merge 6 commits into
base: master
Choose a base branch
from
Open
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
10 changes: 8 additions & 2 deletions checkDetail.go
Original file line number Diff line number Diff line change
Expand Up @@ -243,7 +243,13 @@ func (cd *CheckDetail) String() string {

// Validate performs imagecashletter format rule checks on the record and returns an error if not Validated
// The first error encountered is returned and stops the parsing.
func (cd *CheckDetail) Validate() error {
func (cd *CheckDetail) Validate(opts ...*ValidateOpts) error {
mfdeveloper508 marked this conversation as resolved.
Show resolved Hide resolved

var opt *ValidateOpts
if len(opts) > 0 {
opt = opts[0]
}

if err := cd.fieldInclusion(); err != nil {
return err
}
Expand Down Expand Up @@ -284,7 +290,7 @@ func (cd *CheckDetail) Validate() error {
}
}
// Conditional
if cd.ArchiveTypeIndicator != "" {
if cd.ArchiveTypeIndicator != "" && opt.IsArchiveTypeIndicator() {
if err := cd.isArchiveTypeIndicator(cd.ArchiveTypeIndicator); err != nil {
return &FieldError{FieldName: "ArchiveTypeIndicator", Value: cd.ArchiveTypeIndicator, Msg: err.Error()}
}
Expand Down
9 changes: 9 additions & 0 deletions checkDetail_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -294,6 +294,15 @@ func TestCDArchiveTypeIndicator(t *testing.T) {
}
}

// TestCDArchiveTypeIndicatorWithValidationOption validation
func TestCDArchiveTypeIndicatorWithValidationOption(t *testing.T) {
cd := mockCheckDetail()
cd.ArchiveTypeIndicator = "W"
if err := cd.Validate(&ValidateOpts{ArchiveTypeIndicator: false}); err != nil {
t.Errorf("%T: %s", err, err)
}
}

// Field Inclusion

// TestCDFIRecordType validation
Expand Down
40 changes: 40 additions & 0 deletions cmd/writeImageCashLetter/main.go
Original file line number Diff line number Diff line change
Expand Up @@ -20,6 +20,12 @@ var (

// output formats
flagJson = flag.Bool("json", false, "Output file in json")

flagPretty = flag.Bool("pretty", false, "Display all values in their human readable format")
flagPrettyAmounts = flag.Bool("pretty.amounts", false, "Display human readable amounts instead of exact values")

flagSkipValidation = flag.Bool("skip-validation", false, "Skip all validation checks")
flagValidateOpts = flag.String("validate", "", "Path to config file in json format to enable validation opts")
)

// main creates an ICL File with 2 CashLetters
Expand Down Expand Up @@ -72,6 +78,12 @@ func write(path string) {
file := imagecashletter.NewFile()
file.SetHeader(fh)

// Read validation options from the command
validateOpts := readValidationOpts(*flagValidateOpts)
if validateOpts != nil {
file.SetValidation(validateOpts)
}

// Create 4 CashLetters
for i := 0; i < 4; i++ {

Expand Down Expand Up @@ -284,3 +296,31 @@ func write(path string) {
fmt.Printf("Wrote %s\n", path)

}

func readValidationOpts(path string) *imagecashletter.ValidateOpts {
if path != "" {
// read config file
bs, readErr := os.ReadFile(path)
if readErr != nil {
fmt.Printf("ERROR: reading validate opts failed: %v\n", readErr)
os.Exit(1)
}

var opts imagecashletter.ValidateOpts
if err := json.Unmarshal(bs, &opts); err != nil {
fmt.Printf("ERROR: unmarshal of validate opts failed: %v\n", err)
os.Exit(1)
}
if *flagSkipValidation {
opts.SkipAll = true
}
return &opts
}

if *flagSkipValidation {
var opts imagecashletter.ValidateOpts
opts.SkipAll = true
}

return nil
}
38 changes: 38 additions & 0 deletions file.go
Original file line number Diff line number Diff line change
Expand Up @@ -91,6 +91,15 @@ var (
msgFileCredit = "Credit outside of cash letter"
)

// ValidateOpts contains specific overrides from the default set of validations
type ValidateOpts struct {
// SkipAll will disable all validation checks of a File. It has no effect when set on records.
SkipAll bool `json:"skipAll"`
Comment on lines +96 to +97
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

SkipAll can be checked inside File.Validate() to bypass all checks.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

exact validation function of record will hit within reader function only
it seems that we should use skipall flag in validation function of record


// ArchiveTypeIndicator can be set to enable archiveTypeIndicator validation
ArchiveTypeIndicator bool `json:"ArchiveTypeIndicator"`
mfdeveloper508 marked this conversation as resolved.
Show resolved Hide resolved
}

// FileError is an error describing issues validating a file
type FileError struct {
FieldName string
Expand Down Expand Up @@ -119,6 +128,8 @@ type File struct {
Bundles []Bundle `json:"bundle,omitempty"`
// FileControl is an imagecashletter FileControl
Control FileControl `json:"fileControl"`

validateOpts *ValidateOpts
}

// NewFile constructs a file template with a FileHeader and FileControl.
Expand Down Expand Up @@ -322,3 +333,30 @@ func (f *File) setRecordTypes() {
}
f.Control.setRecordType()
}

// SetValidation stores ValidateOpts
func (f *File) SetValidation(opts *ValidateOpts) {
if f == nil || opts == nil {
return
}

f.validateOpts = opts
}

// ValidateOpts returns Validation option
func (f *File) ValidateOpts() *ValidateOpts {
return f.validateOpts
}

// IsArchiveTypeIndicator indicate to enable ArchiveTypeIndicator validation
func (v *ValidateOpts) IsArchiveTypeIndicator() bool {
mfdeveloper508 marked this conversation as resolved.
Show resolved Hide resolved
if v == nil {
return true
}

if v.SkipAll || v.ArchiveTypeIndicator == false {
return false
}

return true
}
2 changes: 1 addition & 1 deletion reader.go
Original file line number Diff line number Diff line change
Expand Up @@ -384,7 +384,7 @@ func (r *Reader) parseCheckDetail() error {
cd := new(CheckDetail)
cd.Parse(r.decodeLine(r.line))
// Ensure valid CheckDetail
if err := cd.Validate(); err != nil {
if err := cd.Validate(r.File.ValidateOpts()); err != nil {
return r.error(err)
}
// Add CheckDetail
Expand Down
10 changes: 8 additions & 2 deletions returnDetail.go
Original file line number Diff line number Diff line change
Expand Up @@ -265,7 +265,13 @@ func (rd *ReturnDetail) String() string {

// Validate performs image cash letter format rule checks on the record and returns an error if not Validated
// The first error encountered is returned and stops the parsing.
func (rd *ReturnDetail) Validate() error {
func (rd *ReturnDetail) Validate(opts ...*ValidateOpts) error {
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

In ACH we passed the ValidateOpts through a file so all records in a file have the same validation overrides.

Copy link
Contributor Author

@mfdeveloper508 mfdeveloper508 May 23, 2023

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Right, ACH used above logic
but imagecashletter is different with ACH
Actually we need to hit validation function of ReturnDetail and CheckDetail
Validation function of file doesn't hit these validation functions
The validation functions will hit only when parsing file

I am not sure the reason yet. anyway we should use this parameter for supporting current logic

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

cc @atonks2 thoughts?

Copy link
Collaborator

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

ACH adds an instance of validateOpts to several record types, but then still exposes a ValidateWith method on some of them to pass opts through.

If we add a validateOpts field to each record that has options defined and wire it all the way through from file creation, that would probably cover the most code paths (trying to think through HTTP API vs library). That said, it's not clear to me from a quick review of ACH how the opts get wired through.

@adamdecaf Can you provide more context on how the validate opts work in ACH? How/when are they wired through when using ACH as an API vs a library.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

They're wired through from the File object so you can accumulate records and the opts are respected during validation. The HTTP api accepts them as a top-level object in JSON.

The ACH library's Reader can also have ValidateOpts which are put onto each file that's read.


var opt *ValidateOpts
if len(opts) > 0 {
opt = opts[0]
}

if err := rd.fieldInclusion(); err != nil {
return err
}
Expand All @@ -288,7 +294,7 @@ func (rd *ReturnDetail) Validate() error {
return &FieldError{FieldName: "ReturnNotificationIndicator", Value: rd.ReturnNotificationIndicatorField(), Msg: err.Error()}
}
}
if rd.ArchiveTypeIndicator != "" {
if rd.ArchiveTypeIndicator != "" && opt.IsArchiveTypeIndicator() {
if err := rd.isArchiveTypeIndicator(rd.ArchiveTypeIndicator); err != nil {
return &FieldError{FieldName: "ArchiveTypeIndicator", Value: rd.ArchiveTypeIndicatorField(), Msg: err.Error()}
}
Expand Down
9 changes: 9 additions & 0 deletions returnDetail_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -248,6 +248,15 @@ func TestRDArchiveTypeIndicator(t *testing.T) {
}
}

// TestRDArchiveTypeIndicatorWithValidationOption validation
func TestRDArchiveTypeIndicatorWithValidationOption(t *testing.T) {
rd := mockReturnDetail()
rd.ArchiveTypeIndicator = "W"
if err := rd.Validate(&ValidateOpts{ArchiveTypeIndicator: false}); err != nil {
t.Errorf("%T: %s", err, err)
}
}

// TestRDTimesReturned validation
func TestRDTimesReturned(t *testing.T) {
rd := mockReturnDetail()
Expand Down