lorenzodonini / ocpp-go

Open Charge Point Protocol implementation in Go
MIT License
275 stars 126 forks source link

Fix property constraint violation on ConfigurationKey #178

Closed lorenzodonini closed 1 year ago

lorenzodonini commented 1 year ago

Fixes the immediate issue identified in #175.

The validation for the ConfigurationKey.Value field now is run only when the value is not nil. This will prevent a PropertyConstraintViolation error to be thrown, when a nil value is set for a specific key.

andig commented 1 year ago

Still strange that it did even trigger, isn‘t it?

lorenzodonini commented 1 year ago

I guess it's specific for pointer fields. I found a related issue that seems to confirm the behavior.

By quickly checking the source code it also seems to make sense:

func (v *validate) traverseField(ctx context.Context, parent reflect.Value, current reflect.Value, ns []byte, structNs []byte, cf *cField, ct *cTag) {
    var typ reflect.Type
    var kind reflect.Kind

    current, kind, v.fldIsPointer = v.extractTypeInternal(current, false)

    switch kind {
    case reflect.Ptr, reflect.Interface, reflect.Invalid:

        if ct == nil {
            return
        }

        if ct.typeof == typeOmitEmpty || ct.typeof == typeIsDefault {
            return
        }

        if ct.hasTag {
            if kind == reflect.Invalid {
                v.str1 = string(append(ns, cf.altName...))
                if v.v.hasTagNameFunc {
                    v.str2 = string(append(structNs, cf.name...))
                } else {
                    v.str2 = v.str1
                }
                v.errs = append(v.errs,
                    &fieldError{
                        v:              v.v,
                        tag:            ct.aliasTag,
                        actualTag:      ct.tag,
                        ns:             v.str1,
                        structNs:       v.str2,
                        fieldLen:       uint8(len(cf.altName)),
                        structfieldLen: uint8(len(cf.name)),
                        param:          ct.param,
                        kind:           kind,
                    },
                )
                return
            }

            v.str1 = string(append(ns, cf.altName...))
            if v.v.hasTagNameFunc {
                v.str2 = string(append(structNs, cf.name...))
            } else {
                v.str2 = v.str1
            }
            if !ct.runValidationWhenNil {
                v.errs = append(v.errs,
                    &fieldError{
                        v:              v.v,
                        tag:            ct.aliasTag,
                        actualTag:      ct.tag,
                        ns:             v.str1,
                        structNs:       v.str2,
                        fieldLen:       uint8(len(cf.altName)),
                        structfieldLen: uint8(len(cf.name)),
                        value:          current.Interface(),
                        param:          ct.param,
                        kind:           kind,
                        typ:            current.Type(),
                    },
                )
                return
            }
        }

There is actually a way to achieve the same result by passing a callValidationEvenIfNull = true flag to https://pkg.go.dev/github.com/go-playground/validator/v10#Validate.RegisterValidation, however this isn't the case for built-in types (such as in this issue). I think just explicitly setting omitempty|required will do.