✅ Add test case for storage/blob/s3/writer/option (#656)

* feat: add test and new validation logic

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* fix: const value

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* feat: add ignore options for goleak

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* fix: apply suggestion

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* feat: error for option

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* feat: add option error

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* fix: guideline

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* fix: lint error

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* Update docs/contributing/coding-style.md

Co-authored-by: Kevin Diu <kevin_diu@yahoo.com.hk>

* Update docs/contributing/coding-style.md

* Update docs/contributing/coding-style.md

* Update docs/contributing/coding-style.md

* Update docs/contributing/coding-style.md

* feat: add option error type

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* Update docs/contributing/coding-style.md

* Update docs/contributing/coding-style.md

* feat: add test case

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* Update internal/db/storage/blob/s3/writer/option_test.go

* fix: apply suggestion

Signed-off-by: hlts2 <hiroto.funakoshi.hiroto@gmail.com>

* 🤖 Update license headers / Format go codes and yaml files

Signed-off-by: vdaas-ci <ci@vdaas.org>

Co-authored-by: Kevin Diu <kevin_diu@yahoo.com.hk>
Co-authored-by: vdaas-ci <ci@vdaas.org>

docs/contributing/coding-style.md

@@ -453,12 +453,20 @@ You can refer to [this section](#Struct-initialization) for more details of the
 
 We strongly recommend the following implementation to set the value using functional option.
 
+If an invalid value is set to the functional option, the `ErrInvalidOption` error defined in the [internal/errors/option.go](https://github.com/vdaas/vald/blob/master/internal/errors/option.go) should be returned.
+
+The name argument (the first argument) of the `ErrInvalidOption` error should be the same as the functional option name without the `With` prefix. 
+
+For example, the functional option name `WithVersion` should return the error with the argument `name` as `version`.
+
+
 ```go
 func WithVersion(version string) Option {
     return func(c *client) error {
-        if len(version) != 0 {
-            c.version = version
+        if len(version) == 0 {
+            return errors.ErrInvalidOption("version", version)
         }
+        c.version = version
         return nil
     }
 }
@@ -470,7 +478,7 @@ We recommend the following implementation to parse the time string and set the t
 func WithTimeout(dur string) Option {
     func(c *client) error {
         if dur == "" {
-            return nil
+            return errors.ErrInvalidOption("timeout", dur)
         }
         d, err := timeutil.Parse(dur)
         if err != nil {
@@ -488,7 +496,7 @@ We recommend the following implementation to append the value to the slice if th
 func WithHosts(hosts ...string) Option {
     return func(c *client) error {
         if len(hosts) == 0 {
-            return nil
+            return errors.ErrInvalidOption("hosts", hosts)
         }
         if c.hosts == nil {
             c.hosts = hosts
@@ -500,6 +508,21 @@ func WithHosts(hosts ...string) Option {
 }
 ```
 
+We recommend the following implementation to apply the options.
+
+If the option failed to apply, an error wrapped with `ErrOptionFailed` defined in the [internal/errors/errors.go](https://github.com/vdaas/vald/blob/master/internal/errors/errors.go) should be returned.
+
+```go
+func New(opts ...Option) (Server, error) {
+    srv := new(server)
+    for _, opt := range opts {
+        if err := opt(srv); err != nil {
+            return nil, errors.ErrOptionFailed(err, reflect.ValueOf(opt))
+        }
+    }
+}
+```
+
 ## Program comments
 
 Program comments make easier to understand the source code. We suggest not to write many comments inside the source code unless the source code is very complicated and confusing; otherwise we should divide the source code into methods to keep the readability and usability of the source code.

internal/db/storage/blob/s3/writer/option.go

@@ -18,10 +18,13 @@ package writer
 
 import (
 	"github.com/aws/aws-sdk-go/service/s3"
+	"github.com/aws/aws-sdk-go/service/s3/s3manager"
 	"github.com/vdaas/vald/internal/errgroup"
+	"github.com/vdaas/vald/internal/errors"
 )
 
-type Option func(w *writer)
+// Option represents the functional option for writer.
+type Option func(w *writer) error
 
 var (
 	defaultOpts = []Option{
@@ -31,44 +34,68 @@ var (
 	}
 )
 
+// WithErrGroup returns the option to set eg for writer.
 func WithErrGroup(eg errgroup.Group) Option {
-	return func(w *writer) {
-		if eg != nil {
-			w.eg = eg
+	return func(w *writer) error {
+		if eg == nil {
+			return errors.ErrInvalidOption("errgroup", eg)
 		}
+		w.eg = eg
+		return nil
 	}
 }
 
+// WithService returns the option to set s for writer.
 func WithService(s *s3.S3) Option {
-	return func(w *writer) {
-		if s != nil {
-			w.service = s
+	return func(w *writer) error {
+		if s == nil {
+			return errors.ErrInvalidOption("service", s)
 		}
+		w.service = s
+		return nil
 	}
 }
 
+// WithBucket returns the option to set bucket for writer.
 func WithBucket(bucket string) Option {
-	return func(w *writer) {
+	return func(w *writer) error {
+		if len(bucket) == 0 {
+			return errors.ErrInvalidOption("bucket", bucket)
+		}
 		w.bucket = bucket
+		return nil
 	}
 }
 
+// WithKey returns the option to set key for writer.
 func WithKey(key string) Option {
-	return func(w *writer) {
+	return func(w *writer) error {
+		if len(key) == 0 {
+			return errors.ErrInvalidOption("key", key)
+		}
 		w.key = key
+		return nil
 	}
 }
 
+// WithContentType returns the option to set ct for writer.
 func WithContentType(ct string) Option {
-	return func(w *writer) {
-		if ct != "" {
-			w.contentType = ct
+	return func(w *writer) error {
+		if len(ct) == 0 {
+			return errors.ErrInvalidOption("contentType", ct)
 		}
+		w.contentType = ct
+		return nil
 	}
 }
 
+// WithMaxPartSize returns the option to set max for writer.
 func WithMaxPartSize(max int64) Option {
-	return func(w *writer) {
+	return func(w *writer) error {
+		if max < s3manager.MinUploadPartSize {
+			return errors.ErrInvalidOption("maxPartSize", max)
+		}
 		w.maxPartSize = max
+		return nil
 	}
 }