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

refactor: Remove ApiVersion from normal DTOs except EventDTO #538

Merged
merged 1 commit into from
Mar 15, 2021

Conversation

weichou1229
Copy link
Member

Close #537

PR Checklist

Please check if your PR fulfills the following requirements:

  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

If your build fails due to your commit message not passing the build checks, please review the guidelines here: https://github.com/edgexfoundry/go-mod-core-contracts/blob/master/.github/Contributing.md.

What is the current behavior?

The apiVersion is required.

Issue Number: #537

What is the new behavior?

  • Remove the required validation tag from apiVersion.
  • Check the requestDTOs and EventDTO which should contains valid api version

Does this PR introduce a breaking change?

  • Yes
  • No

New Imports

  • Yes
  • No

Specific Instructions

Are there any specific instructions or things that should be known prior to reviewing?

Other information

@weichou1229
Copy link
Member Author

weichou1229 commented Mar 9, 2021

I tried to write a custom tag to verify the DTO which should contain api version like:

type BaseRequest struct {
	Versionable  `json:",inline" validate:"edgex-dto-api-version-required"`
	RequestId     string `json:"requestId" validate:"len=0|uuid"`
}

But it didn't work because the validation lib ignore the embedded struct's tag. So I check the apiVersion in the Validation function.

@weichou1229 weichou1229 changed the title refactor: Set apiVersion to optional in normal DTOs refactor: Remove ApiVersion from normal DTOs except EventDTO Mar 15, 2021
@codecov-io
Copy link

Codecov Report

Merging #538 (581db9a) into master (2e84a19) will increase coverage by 0.07%.
The diff coverage is 100.00%.

Impacted file tree graph

@@            Coverage Diff             @@
##           master     #538      +/-   ##
==========================================
+ Coverage   55.79%   55.87%   +0.07%     
==========================================
  Files         119      119              
  Lines        3570     3551      -19     
==========================================
- Hits         1992     1984       -8     
+ Misses       1385     1374      -11     
  Partials      193      193              
Impacted Files Coverage Δ
v2/dtos/device.go 8.19% <ø> (+0.13%) ⬆️
v2/dtos/deviceservice.go 12.12% <ø> (+0.35%) ⬆️
v2/dtos/interval.go 0.00% <ø> (ø)
v2/dtos/intervalaction.go 0.00% <ø> (ø)
v2/dtos/provisionwatcher.go 15.00% <ø> (+0.36%) ⬆️
v2/dtos/requests/device.go 89.28% <ø> (-0.19%) ⬇️
v2/dtos/requests/deviceprofile.go 83.33% <ø> (-0.67%) ⬇️
v2/dtos/requests/deviceservice.go 94.73% <ø> (-0.14%) ⬇️
v2/dtos/requests/interval.go 72.50% <ø> (+1.76%) ⬆️
v2/dtos/requests/intervalaction.go 82.97% <ø> (-0.36%) ⬇️
... and 6 more

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 2e84a19...581db9a. Read the comment docs.

Copy link
Member

@lenny-goodell lenny-goodell left a comment

Choose a reason for hiding this comment

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

LGTM

@lenny-goodell lenny-goodell merged commit 16d295f into edgexfoundry:master Mar 15, 2021
This was referenced Mar 16, 2021
This was referenced Mar 17, 2021
@weichou1229 weichou1229 deleted the issue-537 branch July 16, 2021 09:07
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.

[V2] Set apiVersion to optional in normal DTOs, except for requestDTOs and EventDTO
3 participants