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

Go 0.5.1 have error with UUID check #462

Closed
vooon opened this issue Apr 14, 2021 · 6 comments
Closed

Go 0.5.1 have error with UUID check #462

vooon opened this issue Apr 14, 2021 · 6 comments

Comments

@vooon
Copy link

vooon commented Apr 14, 2021

Hello,

After update to 0.5.1 I am having compilation error because uuid pattern var was removed.

import "validate/validate.proto";
import "patch/go.proto";

message Rule {
	string source_id = 3 [(validate.rules).string.uuid = true, (go.field).name = "SourceID"];
};

Tried to leave only validation rule - nothing changes. Perhaps something wrong in #420 check.

@vooon
Copy link
Author

vooon commented Apr 14, 2021

Hmm, interesting addition:

  • types.proto which have only messages - has _types_uuidPattern
  • api.proto which provides gRPC definitions and request-response messages - don't.

@vooon
Copy link
Author

vooon commented Apr 14, 2021

Found what causes problem: inner message with validator.
Here is better example:

message Envelope {
	message Rule {
		string source_id = 3 [(validate.rules).string.uuid = true, (go.field).name = "SourceID"];
	}

	// uncomment to get uuidPattern var
	// string id = 1 [(validate.rules).string.uuid = true, (go.field).name = "ID"];
	repeated Rule rules = 2;
}

@twistedkento
Copy link

Same thing happened to me today. Current workaround for me is having one dummy message, that is not nested, with uuid validation in it.

It looks like the addition of FileNeeds function in 872b28c introduced this issue.

Would probably be easy to fix by just replacing f.Messages() with f.AllMessages() in the for loop inside of the FileNeeds function?

@akonradi
Copy link
Contributor

@twistedretard would you be able to send a PR? We'd want a test case under tests/harness/executor/cases.go with a message/sub-message somewhere in tests/harness/cases. That will validate the fix you propose

@twistedkento
Copy link

Sure I'll take a stab at it.
Noticed that all the proto files in the tests/harness/cases share the same package so I can't put the test there since the other messages testing UUID that aren't nested will generate a _cases_uuidPattern in that package. So I'll put the package containing the nested UUID validating message in tests/harness/cases/other_package if that is ok?

@akonradi
Copy link
Contributor

Great! Yeah that sounds good.

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

No branches or pull requests

4 participants