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

Fix property constraint violation on ConfigurationKey #178

Merged

Conversation

lorenzodonini
Copy link
Owner

@lorenzodonini lorenzodonini commented Mar 16, 2023

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.

Signed-off-by: Lorenzo <lorenzo.donini90@gmail.com>
@lorenzodonini lorenzodonini added the ocpp1.6 Related to OCPP 1.6 package label Mar 16, 2023
@lorenzodonini lorenzodonini merged commit 0374c84 into master Mar 16, 2023
@lorenzodonini lorenzodonini deleted the 175-propertyconstraintviolation-configurationkey branch March 16, 2023 23:34
@andig
Copy link
Contributor

andig commented Mar 17, 2023

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

@lorenzodonini
Copy link
Owner Author

lorenzodonini commented Mar 17, 2023

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.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
ocpp1.6 Related to OCPP 1.6 package
Projects
None yet
Development

Successfully merging this pull request may close these issues.

ocpp1.6: error is swallowed and GenericError on timeout returned instead
2 participants