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

FR: Add spec validators for the WRP messages #84

Merged
merged 15 commits into from
Jun 15, 2022

Conversation

denopink
Copy link
Contributor

@denopink denopink commented Jun 8, 2022

Overview

related to #25, #78, xmidt-org/scytale#88, xmidt-org/talaria#153 and builds on top of #80.

tl;dr

This pr introduces our wrp spec validators built with our validation framework introduced in #80 which only validates the opinionated portions of the spec. Clients can leverage these prebuilt validators to validate their messages.

Explanation

Clients can leverage our prebuilt validators to validate their messages. One set of these validators are our spec validators:

msgv, err := NewTypeValidator(
	// Validates found msg types
	map[MessageType]Validator{
		// Validates opinionated portions of the spec
		SimpleEventMessageType: SpecValidators,
		// Only validates Source and nothing else
		SimpleRequestResponseMessageType: SourceValidator,
	},
	// Validates unfound msg types
	AlwaysInvalid)
if err != nil {
	return
}

foundErrSuccess1 := msgv.Validate(Message{
	Type:        SimpleEventMessageType,
	Source:      "MAC:11:22:33:44:55:66",
	Destination: "MAC:11:22:33:44:55:61",
}) // Found success
foundErrSuccess2 := msgv.Validate(Message{
	Type:        SimpleRequestResponseMessageType,
	Source:      "MAC:11:22:33:44:55:66",
	Destination: "invalid:a-BB-44-55",
}) // Found success
foundErrFailure := msgv.Validate(Message{
	Type:        Invalid0MessageType,
	Source:      "invalid:a-BB-44-55",
	Destination: "invalid:a-BB-44-55",
}) // Found error
unfoundErrFailure := msgv.Validate(Message{Type: CreateMessageType}) // Unfound error
fmt.Println(foundErrSuccess1 == nil, foundErrSuccess2 == nil, foundErrFailure == nil, unfoundErrFailure == nil)
// Output: true true false false
Type of Change(s)
  • Non-breaking Enhancement
  • All new and existing tests passed.

* Validates requirements described at https://xmidt.io/docs/wrp/basics/#overarching-guidelines
* Validates requirements described at https://xmidt.io/docs/wrp/basics/#locators
* validates message type values
* Does not cover validators for a specific message type (future prs)
* Clean up errors
* Remove multierr with some wrapping and regular errors
* Wrap SpecValidators within a func
…xample

* update testUTF8Validator success case to contain a correct source
* removed unused error list
* removed nil struct field inits
* added missing test cases for testValidateLocator
* improved test descriptions for testValidateLocator
* added missing test cases for TestSpecValidators
* improved example
@codecov
Copy link

codecov bot commented Jun 8, 2022

Codecov Report

Merging #84 (edeb68e) into main (be83fc7) will increase coverage by 0.82%.
The diff coverage is 100.00%.

@@            Coverage Diff             @@
##             main      #84      +/-   ##
==========================================
+ Coverage   49.67%   50.49%   +0.82%     
==========================================
  Files          20       21       +1     
  Lines        3259     3313      +54     
==========================================
+ Hits         1619     1673      +54     
  Misses       1469     1469              
  Partials      171      171              
Impacted Files Coverage Δ
utf8.go 88.23% <ø> (-0.66%) ⬇️
spec_validator.go 100.00% <100.00%> (ø)
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 be83fc7...edeb68e. Read the comment docs.

Copy link
Contributor

@kristinapathak kristinapathak left a comment

Choose a reason for hiding this comment

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

Great start! The code itself looks good, mostly some structure and error suggestions.

spec_validator.go Outdated Show resolved Hide resolved
spec_validator.go Outdated Show resolved Hide resolved
spec_validator.go Outdated Show resolved Hide resolved
spec_validator.go Show resolved Hide resolved
spec_validator.go Outdated Show resolved Hide resolved
* add unexported errors for `validateLocator` func to allow better/easier testing
* improve error message for `validateLocator` func when values to match locator pattern
* move `locatorPattern` var to the top of the file
* convert vars `DestinationValidator SourceValidator MessageTypeValidator UTF8Validator AlwaysInvalid AlwaysValid` to funcs that return their validator & update their tests and code position (if needed)
* Improve error message for `DestinationValidator SourceValidator` funcs
Copy link
Contributor

@kristinapathak kristinapathak left a comment

Choose a reason for hiding this comment

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

💯 Looks great!

@denopink denopink mentioned this pull request Aug 3, 2022
9 tasks
@denopink denopink mentioned this pull request Aug 24, 2022
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

None yet

2 participants