FR: Initial WRP Validation Framework #80

denopink · 2022-05-25T16:32:19Z

Overview

related to #25, #78, xmidt-org/scytale#88, xmidt-org/talaria#153 s.t. we want a validation mechanism that is configurable by clients & verifies the spec.

tl;dr

This pr introduces the initial validation framework, where clients supply validators (satisfying the Validator interface) to NewMsgTypeValidator and then used to verify the spec.

Explanation

Clients supply validators satisfying:

// Validator is a WRP validator that allows access to the Validate function.
type Validator interface {
	Validate(m Message) error
}

and listing which validators are used on known and unknown msg types (where unknown msg types are handled by defaultValidator):

var alwaysValid ValidatorFunc = func(msg Message) error { return nil }
msgv, err := NewTypeValidator(
	// Validates found msg types
	map[MessageType]Validators{SimpleEventMessageType: {alwaysValid}},
	// Validates unfound msg types
	AlwaysInvalid)
err = msgv.Validate(Message{Type: SimpleEventMessageType}) // Found success
err == nil // True
err = msgv.Validate(Message{Type: CreateMessageType}) // Unfound error
err == nil // False

if a default validator is not provided, all unknown msg type will fail by default

msgv, err := NewTypeValidator( // Omitted defaultValidator
	map[MessageType]Validators{SimpleEventMessageType: {alwaysValid}})
err = msgv.Validate(Message{Type: CreateMessageType}) // Unfound error
err == nil // False

Type of Change(s)

Non-breaking Enhancement
All new and existing tests passed.

## Overview related to xmidt-org#25, xmidt-org#78, xmidt-org/scytale#88, xmidt-org/talaria#153 s.t. we want a validation mechanism that is configurable by the application & verifies the spec. ### tl;rd This pr introduces the initial validation framework, where applications supply validators (satisfying the `Validator interface`) to `NewMsgTypeValidator` and then used to verify the spec. <details> <summary> Explanation</summary> Apps supply validators satisfying: ```go // Validator is a WRP validator that allows access to the Validate function. type Validator interface { Validate(m Message) error } ``` and listing which validators are used on known and unknown msg types (where unknown msg types are handled by `defaultValidator`): ```go var alwaysValidMsg ValidatorFunc = func(msg Message) error { return nil } msgv, err := NewMsgTypeValidator( // Validates known msg types m: map[MessageType]Validators{SimpleEventMessageType: {alwaysValidMsg}}, // Validates unknown msg types defaultValidator: alwaysValidMsg) err = msgv.Validate(Message{Type: SimpleEventMessageType}) // Known msg type err == nil // True err = msgv.Validate(Message{Type: CreateMessageType}) // Unknown msg type, uses defaultValidator err == nil // True ``` if a default validator is not provided, all unknown msg type will **fail** by default ```go var alwaysValidMsg ValidatorFunc = func(msg Message) error { return nil } msgv, err := NewMsgTypeValidator( // Omitted defaultValidator m: map[MessageType]Validators{SimpleEventMessageType: {alwaysInvalidMsg()}}) err = msgv.Validate(Message{Type: CreateMessageType}) err != nil // True ``` </details> <details> <summary>Type of Change(s)</summary> - Non-breaking Enhancement - All new and existing tests passed. </details> <details> <summary>Module Unit Testing: [PASSING]</summary> </details> <details> <summary>PR Affecting Unit Testing: validator_test.go [PASSING]</summary> ```console Running tool: /usr/local/bin/go test -timeout 30s -run ^(TestHelperValidators|TestMsgTypeValidator)$ github.com/xmidt-org/wrp-go/v3 === RUN TestHelperValidators === RUN TestHelperValidators/alwaysInvalidMsg --- PASS: TestHelperValidators (0.00s) --- PASS: TestHelperValidators/alwaysInvalidMsg (0.00s) === RUN TestMsgTypeValidator === RUN TestMsgTypeValidator/MsgTypeValidator_validate === RUN TestMsgTypeValidator/MsgTypeValidator_validate/known_message_type === RUN TestMsgTypeValidator/MsgTypeValidator_validate/unknown_message_type_with_provided_default_Validator === RUN TestMsgTypeValidator/MsgTypeValidator_validate/known_message_type_with_a_failing_Validator === RUN TestMsgTypeValidator/MsgTypeValidator_validate/unknown_message_type_without_provided_default_Validator === RUN TestMsgTypeValidator/MsgTypeValidator_factory === RUN TestMsgTypeValidator/MsgTypeValidator_factory/with_provided_default_Validator === RUN TestMsgTypeValidator/MsgTypeValidator_factory/without_provided_default_Validator === RUN TestMsgTypeValidator/MsgTypeValidator_factory/empty_list_of_message_type_Validators === RUN TestMsgTypeValidator/MsgTypeValidator_factory/empty_value_'m'_map[MessageType]Validators --- PASS: TestMsgTypeValidator (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_validate (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_validate/known_message_type (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_validate/unknown_message_type_with_provided_default_Validator (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_validate/known_message_type_with_a_failing_Validator (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_validate/unknown_message_type_without_provided_default_Validator (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_factory (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_factory/with_provided_default_Validator (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_factory/without_provided_default_Validator (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_factory/empty_list_of_message_type_Validators (0.00s) --- PASS: TestMsgTypeValidator/MsgTypeValidator_factory/empty_value_'m'_map[MessageType]Validators (0.00s) PASS ok github.com/xmidt-org/wrp-go/v3 0.303s > Test run finished at 5/25/2022, 11:39:22 AM < ``` </details>

codecov · 2022-05-25T16:33:19Z

Codecov Report

Merging #80 (0218fb9) into main (840a19e) will increase coverage by 0.37%.
The diff coverage is 100.00%.

❗ Current head 0218fb9 differs from pull request most recent head fc099bf. Consider uploading reports for the commit fc099bf to get more accurate results

@@            Coverage Diff             @@
##             main      #80      +/-   ##
==========================================
+ Coverage   49.30%   49.67%   +0.37%     
==========================================
  Files          19       20       +1     
  Lines        3235     3259      +24     
==========================================
+ Hits         1595     1619      +24     
  Misses       1469     1469              
  Partials      171      171

Impacted Files	Coverage Δ
validator.go	`100.00% <100.00%> (ø)`

Continue to review full report at Codecov.

Legend - Click here to learn more
Δ = absolute <relative> (impact), ø = not affected, ? = missing data
Powered by Codecov. Last update 840a19e...fc099bf. Read the comment docs.

simplifies the usage of alwaysInvalidMsg

Return `ErrInvalidMsgTypeValidator` and `ErrInvalidMsgType` without additional details. We can add those error details downstream later

kristinapathak

Good start, left some small suggestions.

header_wrp.go

validator.go

validator_test.go

@kristinapathak

Thx @kristinapathak for the feedback! 🍻

already covered by "Not found success"

…ort & added examples

denopink · 2022-05-26T16:54:35Z

@kristinapathak I added some testable examples just to try them out. it's pretty neat

I can take them out if it's too early for doc examples.

…om/denopink/wrp-go into denopink/FR-WRPValidationFramework

kristinapathak · 2022-05-26T18:28:38Z

@denopink, example looks good! 👍

validator.go

validator_test.go

update examples Doc/var update Add test for `Nil default Validators` edge case update examples output doc update Updates based on PR review * exported alwaysValid * decoupled data structures leveraging `Validators` to `Validator` * added validateValidator Add multierr to Validator

kristinapathak

Generally the functions are looking good - I think I only focused on that last time, so have more unit tests comments now. 👍

validator.go

validator_test.go

* converted remaining `Validators` to `Validator` * remove func `validateValidator` * move nil checks to `Validate` func * remove Test struct * add new unexported field to check if `TypeValidato` is valid * add func `IsBad` to TypeValidato to say whether `TypeValidato` is valid * update tests for testAlwaysInvalid and testAlwaysValid * update tests names for `TestTypeValidator` * add tests for `Validators`

* [misunderstanding] Remove unexported field `isbad` from `TypeValidator` and its references * [misunderstanding] Remove `IsBad` func from `TypeValidator` * Use `assert.Zero/NotZero` funcs to test `TypeValidator`'s state

… type is used

… `testAlwaysInvalid`

kristinapathak

Tests look great! Realized I missed these two small suggestions earlier.

validator.go

* rename `defaultValidators` to `defaultValidator` * replace `defaultValidators` variadic to `defaultValidator Validator` in `NewTypeValidator` func

kristinapathak

🎉

denopink added 2 commits May 24, 2022 09:22

Add VSCode to .gitignore

dfeee98

denopink added the enhancement New feature or request label May 25, 2022

denopink requested review from schmidtw, johnabass and kristinapathak May 25, 2022 16:32

denopink self-assigned this May 25, 2022

denopink added 5 commits May 25, 2022 12:34

formatting

9319c12

formatting

209ffdb

convert alwaysInvalidMsg to literal func

ddd0acf

simplifies the usage of alwaysInvalidMsg

Formating and exported alwaysInvalidMsg

a2a4093

Decouple error from api

f5a2e90

Return `ErrInvalidMsgTypeValidator` and `ErrInvalidMsgType` without additional details. We can add those error details downstream later

kristinapathak reviewed May 25, 2022

View reviewed changes

denopink and others added 6 commits May 25, 2022 20:42

Updates based on PR review

eb5bf9b

Thx @kristinapathak for the feedback! 🍻

removed duplicated test

afa08cb

already covered by "Not found success"

Fix type test name

8cb0fa7

Fix type test name

10afaa2

Add missing test/edge cases for Validators

1923ec7

Updated defaultValidator to work as a variadic for multi-default supp…

98c4b4a

…ort & added examples

denopink added 2 commits May 26, 2022 13:22

rename test var name to description

3197568

Merge branch 'denopink/FR-WRPValidationFramework' of https://github.c…

1ee6c2e

…om/denopink/wrp-go into denopink/FR-WRPValidationFramework

kristinapathak reviewed May 26, 2022

View reviewed changes

validator.go Show resolved Hide resolved

validator.go Outdated Show resolved Hide resolved

validator.go Outdated Show resolved Hide resolved

kristinapathak reviewed May 26, 2022

View reviewed changes

validator_test.go Outdated Show resolved Hide resolved

denopink requested a review from kristinapathak June 1, 2022 17:37

denopink force-pushed the denopink/FR-WRPValidationFramework branch from d943b60 to 793d6f4 Compare June 1, 2022 20:32

formatting

22ca097

Add missing test for AlwaysValid func

5ff49e3

denopink mentioned this pull request Jun 3, 2022

Add spec validators for the WRP messages (BAD BRANCH) #82

Closed

kristinapathak reviewed Jun 6, 2022

View reviewed changes

denopink requested review from kristinapathak and renaz6 and removed request for renaz6 June 7, 2022 20:40

denopink added 6 commits June 8, 2022 05:54

Update based on PR review

df7e2ff

* [misunderstanding] Remove unexported field `isbad` from `TypeValidator` and its references * [misunderstanding] Remove `IsBad` func from `TypeValidator` * Use `assert.Zero/NotZero` funcs to test `TypeValidator`'s state

Move exported fields to the top

ffd531f

update tests to include correct mac length

13a9a57

testAlwaysValid & testAlwaysInvalid ensure a non-existing message…

3ac1d02

… type is used

testTypeValidatorFactory swap assert.Zero/NotZero

e553e7e

Add missing scheme to source fields & add missing assert.ErrorIs to…

8e6882b

… `testAlwaysInvalid`

kristinapathak reviewed Jun 8, 2022

View reviewed changes

validator.go Outdated Show resolved Hide resolved

validator.go Outdated Show resolved Hide resolved

Update based on PR feedback

fc099bf

* rename `defaultValidators` to `defaultValidator` * replace `defaultValidators` variadic to `defaultValidator Validator` in `NewTypeValidator` func

kristinapathak approved these changes Jun 8, 2022

View reviewed changes

denopink merged commit be83fc7 into xmidt-org:main Jun 8, 2022

This was referenced Jun 8, 2022

FR: Add spec validators for the WRP messages #84

Merged

FR: Add simple message types validators for the WRP messages #85

Merged

denopink deleted the denopink/FR-WRPValidationFramework branch July 29, 2022 19:04

denopink mentioned this pull request Aug 24, 2022

Release v3.1.4 #100

Merged

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

FR: Initial WRP Validation Framework #80

FR: Initial WRP Validation Framework #80

denopink commented May 25, 2022 •

edited

Loading

codecov bot commented May 25, 2022 •

edited

Loading

kristinapathak left a comment

denopink commented May 26, 2022 •

edited

Loading

kristinapathak commented May 26, 2022

kristinapathak left a comment

kristinapathak left a comment

kristinapathak left a comment

FR: Initial WRP Validation Framework #80

FR: Initial WRP Validation Framework #80

Conversation

denopink commented May 25, 2022 • edited Loading

Overview

tl;dr

codecov bot commented May 25, 2022 • edited Loading

Codecov Report

kristinapathak left a comment

Choose a reason for hiding this comment

denopink commented May 26, 2022 • edited Loading

kristinapathak commented May 26, 2022

kristinapathak left a comment

Choose a reason for hiding this comment

kristinapathak left a comment

Choose a reason for hiding this comment

kristinapathak left a comment

Choose a reason for hiding this comment

denopink commented May 25, 2022 •

edited

Loading

codecov bot commented May 25, 2022 •

edited

Loading

denopink commented May 26, 2022 •

edited

Loading