Repository: yorkie-team/yorkie
Pull request #354 (2022-07-05 09:13:16)

Apply gRPC error details to update project api
<!--  Thanks for sending a pull request! -->

**What this PR does / why we need it**:

- [x] deliver information for error details to [where the error convert to status](https://github.com/yorkie-team/yorkie/blob/f6a4aa15ea4cf3a13ae1288e1f9b191ac536aee0/server/grpchelper/status.go#L37)
    - delivered via custom error
- [x] fulfill the error details with proper key-value set
- [ ] handling error details at yorkie-house


**Which issue(s) this PR fixes**:
<!--
*Automatically closes linked issue when PR is merged.
Usage: `Fixes #<issue number>`, or `Fixes (paste link of issue)`.
-->
Fixes #338 

**Special notes for your reviewer**:

**Does this PR introduce a user-facing change?**:
<!--
If no, just write "NONE" in the release-note block below.
If yes, a release note is required:
Enter your extended release note in the block below. If the PR requires additional action from users switching to the new release, include the string "action required".
-->
```release-note

```

**Additional documentation**:

<!--
This section can be blank if this pull request does not require a release note.

Please use the following format for linking documentation:
- [Usage]: <link>
- [Other doc]: <link>
-->
```docs

```

**Checklist**:
- [x] Added relevant tests or not required
- [x] Didn't break anything


codecov[bot]: # [Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/354?src=pr&el=h1&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) Report
> Merging [#354](https://codecov.io/gh/yorkie-team/yorkie/pull/354?src=pr&el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) (0605f58) into [main](https://codecov.io/gh/yorkie-team/yorkie/commit/f11d9336454042c5476c0806468f5c46cd77b566?el=desc&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) (f11d933) will **increase** coverage by `0.08%`.
> The diff coverage is `83.33%`.

```diff
@@            Coverage Diff             @@
##             main     #354      +/-   ##
==========================================
+ Coverage   47.58%   47.67%   +0.08%     
==========================================
  Files          66       66              
  Lines        5405     5418      +13     
==========================================
+ Hits         2572     2583      +11     
- Misses       2545     2547       +2     
  Partials      288      288              
```


| [Impacted Files](https://codecov.io/gh/yorkie-team/yorkie/pull/354?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team) | Coverage Δ | |
|---|---|---|
| [api/types/updatable\_project\_fields.go](https://codecov.io/gh/yorkie-team/yorkie/pull/354/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team#diff-YXBpL3R5cGVzL3VwZGF0YWJsZV9wcm9qZWN0X2ZpZWxkcy5nbw==) | `88.88% <83.33%> (-4.87%)` | :arrow_down: |
| [pkg/document/json/rga\_tree\_split.go](https://codecov.io/gh/yorkie-team/yorkie/pull/354/diff?src=pr&el=tree&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team#diff-cGtnL2RvY3VtZW50L2pzb24vcmdhX3RyZWVfc3BsaXQuZ28=) | `73.75% <0.00%> (+0.23%)` | :arrow_up: |

------

[Continue to review full report at Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/354?src=pr&el=continue&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team).
> **Legend** - [Click here to learn more](https://docs.codecov.io/docs/codecov-delta?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team)
> `Δ = absolute <relative> (impact)`, `ø = not affected`, `? = missing data`
> Powered by [Codecov](https://codecov.io/gh/yorkie-team/yorkie/pull/354?src=pr&el=footer&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team). Last update [f11d933...0605f58](https://codecov.io/gh/yorkie-team/yorkie/pull/354?src=pr&el=lastupdated&utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team). Read the [comment docs](https://docs.codecov.io/docs/pull-request-comments?utm_medium=referral&utm_source=github&utm_content=comment&utm_campaign=pr+comments&utm_term=yorkie-team).


Patch:
diff --git a/api/types/updatable_project_fields.go b/api/types/updatable_project_fields.go
index ebfc022d..693c6165 100644
--- a/api/types/updatable_project_fields.go
+++ b/api/types/updatable_project_fields.go
@@ -19,22 +19,13 @@ package types
 
 import (
 	"errors"
-	"fmt"
 	"regexp"
 
 	"github.com/go-playground/validator/v10"
 )
 
-var (
-	// ErrEmptyProjectFields is returned when all the fields are empty.
-	ErrEmptyProjectFields = errors.New("UpdatableProjectFields is empty")
-
-	// ErrNotSupportedMethod is returned when the method is not supported.
-	ErrNotSupportedMethod = errors.New("not supported method for authorization webhook")
-
-	// ErrInvalidProjectField is returned when the field is invalid.
-	ErrInvalidProjectField = errors.New("invalid project field")
-)
+// ErrEmptyProjectFields is returned when all the fields are empty.
+var ErrEmptyProjectFields = errors.New("updatable project fields are empty")
 
 var (
 	// reservedNames is a map of reserved names. It is used to check if the
@@ -47,6 +38,22 @@ var (
 	nameRegex = regexp.MustCompile("^[a-z0-9\\-._~]+$")
 )
 
+// FieldViolation is used to describe a single bad request field
+type FieldViolation struct {
+	// A Field of which field of the reques is bad.
+	Field string
+	// A description of why the request element is bad.
+	Description string
+}
+
+// InvalidFieldsError is used to describe invalid fields.
+type InvalidFieldsError struct {
+	Violations []*FieldViolation
+}
+
+// Error returns the error message.
+func (e *InvalidFieldsError) Error() string { return "invalid fields" }
+
 func isReservedName(name string) bool {
 	if _, ok := reservedNames[name]; ok {
 		return true
@@ -57,13 +64,13 @@ func isReservedName(name string) bool {
 // UpdatableProjectFields is a set of fields that use to update a project.
 type UpdatableProjectFields struct {
 	// Name is the name of this project.
-	Name *string `bson:"name,omitempty" validate:"omitempty,min=2,max=30,name"`
+	Name *string `bson:"name,omitempty" validate:"omitempty,min=2,max=30,slug,reservedname"`
 
 	// AuthWebhookURL is the url of the authorization webhook.
 	AuthWebhookURL *string `bson:"auth_webhook_url,omitempty"`
 
 	// AuthWebhookMethods is the methods that run the authorization webhook.
-	AuthWebhookMethods *[]string `bson:"auth_webhook_methods,omitempty"`
+	AuthWebhookMethods *[]string `bson:"auth_webhook_methods,omitempty" validate:"omitempty,invalidmethod"`
 }
 
 // Validate validates the UpdatableProjectFields.
@@ -71,27 +78,43 @@ func (i *UpdatableProjectFields) Validate() error {
 	if i.Name == nil && i.AuthWebhookURL == nil && i.AuthWebhookMethods == nil {
 		return ErrEmptyProjectFields
 	}
-	if i.AuthWebhookMethods != nil {
-		for _, method := range *i.AuthWebhookMethods {
-			if !IsAuthMethod(method) {
-				return fmt.Errorf("%s: %w", method, ErrNotSupportedMethod)
-			}
-		}
-	}
 
 	if err := defaultValidator.Struct(i); err != nil {
+		invalidFieldsError := &InvalidFieldsError{}
 		for _, err := range err.(validator.ValidationErrors) {
-			return fmt.Errorf("%s: %w", err, ErrInvalidProjectField)
+			v := &FieldViolation{
+				Field:       err.StructField(),
+				Description: err.Translate(trans),
+			}
+			invalidFieldsError.Violations = append(invalidFieldsError.Violations, v)
 		}
-		return err
+		return invalidFieldsError
 	}
 
 	return nil
 }
 
 func init() {
-	registerValidation("name", func(level validator.FieldLevel) bool {
+	registerValidation("slug", func(level validator.FieldLevel) bool {
+		name := level.Field().String()
+		return nameRegex.MatchString(name)
+	})
+	registerTranslation("slug", "{0} must only contain slug available characters")
+
+	registerValidation("reservedname", func(level validator.FieldLevel) bool {
 		name := level.Field().String()
-		return !isReservedName(name) && nameRegex.MatchString(name)
+		return !isReservedName(name)
+	})
+	registerTranslation("reservedname", "given {0} is reserved name")
+
+	registerValidation("invalidmethod", func(level validator.FieldLevel) bool {
+		methods := level.Field().Interface().([]string)
+		for _, method := range methods {
+			if !IsAuthMethod(method) {
+				return false
+			}
+		}
+		return true
 	})
+	registerTranslation("invalidmethod", "given {0} is invalid method")
 }
diff --git a/api/types/validator.go b/api/types/validator.go
index 584cd3e8..e1a04609 100644
--- a/api/types/validator.go
+++ b/api/types/validator.go
@@ -18,7 +18,10 @@
 package types
 
 import (
+	"github.com/go-playground/locales/en"
+	ut "github.com/go-playground/universal-translator"
 	"github.com/go-playground/validator/v10"
+	en_translations "github.com/go-playground/validator/v10/translations/en"
 )
 
 var (
@@ -26,6 +29,15 @@ var (
 	// api package. In this package, some fields are provided by the user, and
 	// we need to validate them.
 	defaultValidator = validator.New()
+	// defaultEn is the default translator instance for the 'en' locale.
+	defaultEn = en.New()
+	// uni is the UniversalTranslator instance set with
+	// the fallback locale and locales it should support.
+	uni = ut.New(defaultEn, defaultEn)
+
+	// trans is the specified translator for the given locale,
+	// or fallback if not found.
+	trans, _ = uni.GetTranslator(defaultEn.Locale())
 )
 
 // registerValidation is shortcut of defaultValidator.RegisterValidation
@@ -35,3 +47,22 @@ func registerValidation(tag string, fn validator.Func) {
 		panic(err)
 	}
 }
+
+// registerTranslation is shortcut of defaultValidator.RegisterTranslation
+// that registers translations against the provided tag with given msg.
+func registerTranslation(tag, msg string) {
+	if err := defaultValidator.RegisterTranslation(tag, trans, func(ut ut.Translator) error {
+		return ut.Add(tag, msg, true)
+	}, func(ut ut.Translator, fe validator.FieldError) string {
+		t, _ := ut.T(tag, fe.Field())
+		return t
+	}); err != nil {
+		panic(err)
+	}
+}
+
+func init() {
+	if err := en_translations.RegisterDefaultTranslations(defaultValidator, trans); err != nil {
+		panic(err)
+	}
+}
diff --git a/server/grpchelper/status.go b/server/grpchelper/status.go
index fd0d5d18..44006c8c 100644
--- a/server/grpchelper/status.go
+++ b/server/grpchelper/status.go
@@ -19,8 +19,10 @@ package grpchelper
 import (
 	"errors"
 
+	"google.golang.org/genproto/googleapis/rpc/errdetails"
 	"google.golang.org/grpc/codes"
 	"google.golang.org/grpc/status"
+	"google.golang.org/protobuf/runtime/protoiface"
 
 	"github.com/yorkie-team/yorkie/api/converter"
 	"github.com/yorkie-team/yorkie/api/types"
@@ -31,6 +33,24 @@ import (
 	"github.com/yorkie-team/yorkie/server/rpc/auth"
 )
 
+func detailsFromError(err error) (protoiface.MessageV1, bool) {
+	invalidFieldsError, ok := err.(*types.InvalidFieldsError)
+	if !ok {
+		return nil, false
+	}
+
+	violations := invalidFieldsError.Violations
+	br := &errdetails.BadRequest{}
+	for _, violation := range violations {
+		v := &errdetails.BadRequest_FieldViolation{
+			Field:       violation.Field,
+			Description: violation.Description,
+		}
+		br.FieldViolations = append(br.FieldViolations, v)
+	}
+	return br, true
+}
+
 // ToStatusError returns a status.Error from the given logic error. If an error
 // occurs while executing logic in API handler, gRPC status.error should be
 // returned so that the client can know more about the status of the request.
@@ -41,6 +61,7 @@ func ToStatusError(err error) error {
 		return status.Error(codes.Unauthenticated, err.Error())
 	}
 
+	var invalidFieldsError *types.InvalidFieldsError
 	if errors.Is(err, converter.ErrPackRequired) ||
 		errors.Is(err, converter.ErrCheckpointRequired) ||
 		errors.Is(err, time.ErrInvalidHexString) ||
@@ -49,9 +70,12 @@ func ToStatusError(err error) error {
 		errors.Is(err, clients.ErrInvalidClientID) ||
 		errors.Is(err, clients.ErrInvalidClientKey) ||
 		errors.Is(err, types.ErrEmptyProjectFields) ||
-		errors.Is(err, types.ErrNotSupportedMethod) ||
-		errors.Is(err, types.ErrInvalidProjectField) {
-		return status.Error(codes.InvalidArgument, err.Error())
+		errors.As(err, &invalidFieldsError) {
+		st := status.New(codes.InvalidArgument, err.Error())
+		if details, ok := detailsFromError(err); ok {
+			st, _ = st.WithDetails(details)
+		}
+		return st.Err()
 	}
 
 	if errors.Is(err, converter.ErrUnsupportedOperation) ||


Test patch:
diff --git a/api/types/updatable_project_fields_test.go b/api/types/updatable_project_fields_test.go
index fce01a2f..b3a06654 100644
--- a/api/types/updatable_project_fields_test.go
+++ b/api/types/updatable_project_fields_test.go
@@ -25,6 +25,7 @@ import (
 )
 
 func TestUpdatableProjectFields(t *testing.T) {
+	var invalidFieldsError *types.InvalidFieldsError
 	t.Run("validation test", func(t *testing.T) {
 		newName := "changed-name"
 		newAuthWebhookURL := "newWebhookURL"
@@ -62,7 +63,7 @@ func TestUpdatableProjectFields(t *testing.T) {
 			AuthWebhookURL:     &newAuthWebhookURL,
 			AuthWebhookMethods: &newAuthWebhookMethods,
 		}
-		assert.ErrorIs(t, fields.Validate(), types.ErrNotSupportedMethod)
+		assert.ErrorAs(t, fields.Validate(), &invalidFieldsError)
 	})
 
 	t.Run("project name format test", func(t *testing.T) {
@@ -76,36 +77,36 @@ func TestUpdatableProjectFields(t *testing.T) {
 		fields = &types.UpdatableProjectFields{
 			Name: &invalidName,
 		}
-		assert.ErrorIs(t, fields.Validate(), types.ErrInvalidProjectField)
+		assert.ErrorAs(t, fields.Validate(), &invalidFieldsError)
 
 		reservedName := "new"
 		fields = &types.UpdatableProjectFields{
 			Name: &reservedName,
 		}
-		assert.ErrorIs(t, fields.Validate(), types.ErrInvalidProjectField)
+		assert.ErrorAs(t, fields.Validate(), &invalidFieldsError)
 
 		reservedName = "default"
 		fields = &types.UpdatableProjectFields{
 			Name: &reservedName,
 		}
-		assert.ErrorIs(t, fields.Validate(), types.ErrInvalidProjectField)
+		assert.ErrorAs(t, fields.Validate(), &invalidFieldsError)
 
 		invalidName = "1"
 		fields = &types.UpdatableProjectFields{
 			Name: &invalidName,
 		}
-		assert.ErrorIs(t, fields.Validate(), types.ErrInvalidProjectField)
+		assert.ErrorAs(t, fields.Validate(), &invalidFieldsError)
 
 		invalidName = "over_30_chracaters_is_invalid_name"
 		fields = &types.UpdatableProjectFields{
 			Name: &invalidName,
 		}
-		assert.ErrorIs(t, fields.Validate(), types.ErrInvalidProjectField)
+		assert.ErrorAs(t, fields.Validate(), &invalidFieldsError)
 
 		invalidName = "invalid/name"
 		fields = &types.UpdatableProjectFields{
 			Name: &invalidName,
 		}
-		assert.ErrorIs(t, fields.Validate(), types.ErrInvalidProjectField)
+		assert.ErrorAs(t, fields.Validate(), &invalidFieldsError)
 	})
 }