validation for required fields was implemented

[tengu-alt]

No description provided.

[testisnullus]

fieldsvalidator or reqfieldsvalidator maybe?

[tengu-alt]

agree, renamed to requiredfieldsvalidator

[testisnullus]

Let's create a variable for it

[tengu-alt]

done

[testisnullus]

Let's also create a variable for "JSON"

[tengu-alt]

done

[DoodgeMatvey]

Your code is good, I just have a suggestion, I didn't test it but you can see on my basic code structure and tell me WDYT. I think it would just a bit simplier

Suggested change

	func validateValue(val reflect.Value) error {
	kind := val.Kind()
	_ = kind
	switch val.Kind() {
	case reflect.Struct:
	return validateStruct(val)
	case reflect.Array, reflect.Slice:
	return validateArrayOrSlice(val)
	case reflect.Map:
	return validateMap(val)
	case reflect.Ptr:
	return validatePointer(val)
	case reflect.Bool:
	return nil
	default:
	if val.IsZero() {
	return fmt.Errorf("required field is empty")
	}
	}
	return nil
	}
	func isStructPtr(fieldValue reflect.Value) bool {
	return fieldValue.Kind() == reflect.Ptr && !fieldValue.IsNil() && fieldValue.Elem().Kind() == reflect.Struct
	}
	func tagContainsOmitempty(tag string) bool {
	return strings.Contains(tag, models.OmitemptyTag)
	}
	func validateStruct(val reflect.Value) error {
	for i := 0; i < val.NumField(); i++ {
	field := val.Type().Field(i)
	fieldValue := val.Field(i)
	if (fieldValue.Kind() == reflect.Array || fieldValue.Kind() == reflect.Slice) && fieldValue.Len() > 0 {
	err := validateValue(fieldValue)
	if err != nil {
	return fmt.Errorf("%s, %w", field.Name, err)
	}
	}
	if tag, ok := field.Tag.Lookup(models.JsonTag); ok && tagContainsOmitempty(tag) {
	if fieldValue.Kind() == reflect.Ptr || fieldValue.Kind() == reflect.Map {
	if !fieldValue.IsZero() {
	err := validateValue(fieldValue)
	if err != nil {
	return fmt.Errorf("%s, %w", field.Name, err)
	}
	}
	}
	continue
	}
	err := validateValue(fieldValue)
	if err != nil {
	return fmt.Errorf("%s, %w", field.Name, err)
	}
	}
	return nil
	}
	func validateArrayOrSlice(val reflect.Value) error {
	if val.Len() < 1 {
	return fmt.Errorf("len is 0")
	}
	for i := 0; i < val.Len(); i++ {
	fieldValue := val.Index(i)
	if fieldValue.Kind() == reflect.Struct ||
	isStructPtr(fieldValue) {
	err := validateValue(val.Index(i))
	if err != nil {
	return err
	}
	}
	}
	return nil
	}
	func validateMap(val reflect.Value) error {
	if val.Len() < 1 {
	return fmt.Errorf("empty map")
	}
	iter := val.MapRange()
	for iter.Next() {
	v := iter.Value()
	if v.Kind() == reflect.Struct ||
	isStructPtr(v) {
	err := validateValue(v)
	if err != nil {
	return err
	}
	}
	}
	return nil
	}
	func validatePointer(val reflect.Value) error {
	if val.IsNil() {
	return fmt.Errorf("pointer to nil")
	}
	return validateValue(val.Elem())
	}
	func validateStruct(val reflect.Value) error {
	for i := 0; i < val.NumField(); i++ {
	field := val.Type().Field(i)
	fieldValue := val.Field(i)
	if tag, ok := field.Tag.Lookup(JsonTag); ok && strings.Contains(tag, OmitemptyTag) && fieldValue.IsZero() {
	continue
	}
	if err := validateField(fieldValue); err != nil {
	return fmt.Errorf("%s: %w", field.Name, err)
	}
	}
	return nil
	}
	func validateField(fieldValue reflect.Value) error {
	switch fieldValue.Kind() {
	case reflect.Struct:
	return validateStruct(fieldValue)
	case reflect.Array, reflect.Slice:
	if fieldValue.Len() == 0 {
	return fmt.Errorf("array or slice is empty")
	}
	for i := 0; i < fieldValue.Len(); i++ {
	if err := validateField(fieldValue.Index(i)); err != nil {
	return err
	}
	}
	case reflect.Map:
	if fieldValue.Len() == 0 {
	return fmt.Errorf("map is empty")
	}
	for _, key := range fieldValue.MapKeys() {
	if err := validateField(fieldValue.MapIndex(key)); err != nil {
	return err
	}
	}
	case reflect.Ptr, reflect.Interface:
	if fieldValue.IsNil() {
	return fmt.Errorf("nil pointer or interface")
	}
	return validateField(fieldValue.Elem())
	default:
	if fieldValue.IsZero() {
	return fmt.Errorf("field is empty")
	}
	}
	return nil
	}

[tengu-alt]

it hasn't pass the tests on arrays that initialized but empty

[DoodgeMatvey]

Yea, I missed check for json and omitempty tags.

func ValidateRequiredFields(v any) error {
	val := reflect.ValueOf(v)

	if val.Kind() == reflect.Ptr {
		val = val.Elem()
	}

	if val.Kind() != reflect.Struct {
		return fmt.Errorf("expected struct type - got %s", val.Kind())
	}

	return validateStruct(val)
}

func validateStruct(val reflect.Value) error {
	typ := val.Type()
	for i := 0; i < val.NumField(); i++ {
		field := typ.Field(i)
		fieldValue := val.Field(i)
		
		tag := field.Tag.Get("json")
		if err := validateField(fieldValue, tag); err != nil {
			return fmt.Errorf("%s: %w", field.Name, err)
		}
	}
	return nil
}

func validateField(fieldValue reflect.Value, tag string) error {
	if strings.Contains(tag, ",omitempty") && isZeroValue(fieldValue) {
		return nil
	}

	switch fieldValue.Kind() {
	case reflect.Struct:
		return validateStruct(fieldValue)
	case reflect.Array, reflect.Slice:
		if fieldValue.Len() == 0 && !strings.Contains(tag, ",omitempty") {
			return fmt.Errorf("array or slice is empty")
		}
		for i := 0; i < fieldValue.Len(); i++ {
			if err := validateField(fieldValue.Index(i), ""); err != nil {
				return err
			}
		}
	case reflect.Map:
		if fieldValue.Len() == 0 && !strings.Contains(tag, ",omitempty") {
			return fmt.Errorf("map is empty")
		}
		iter := fieldValue.MapRange()
		for iter.Next() {
			if err := validateField(iter.Value(), ""); err != nil {
				return err
			}
		}
	case reflect.Ptr, reflect.Interface:
		if fieldValue.IsNil() {
			if !strings.Contains(tag, ",omitempty") {
				return fmt.Errorf("nil pointer or interface")
			}
			return nil
		}
		return validateField(fieldValue.Elem(), "")
	default:
		if isZeroValue(fieldValue) {
			return fmt.Errorf("field is empty")
		}
	}
	return nil
}

func isZeroValue(val reflect.Value) bool {
	switch val.Kind() {
	case reflect.Ptr, reflect.Slice, reflect.Map:
		return val.IsNil()
	default:
		return val.IsZero()
	}
}

[worryg0d]

What do you think about these changes? It simplifies the code by removing wrapping of if statements

Suggested change

	if (fieldValue.Kind() == reflect.Array || fieldValue.Kind() == reflect.Slice) && fieldValue.Len() > 0 {
	err := validateValue(fieldValue)
	if err != nil {
	return fmt.Errorf("%s, %w", field.Name, err)
	}
	}
	if tag, ok := field.Tag.Lookup(models.JsonTag); ok && tagContainsOmitempty(tag) {
	if fieldValue.Kind() == reflect.Ptr || fieldValue.Kind() == reflect.Map {
	if !fieldValue.IsZero() {
	err := validateValue(fieldValue)
	if err != nil {
	return fmt.Errorf("%s, %w", field.Name, err)
	}
	}
	}
	continue
	}
	if (fieldValue.Kind() == reflect.Array || fieldValue.Kind() == reflect.Slice || fieldValue.Kind() == reflect.Map) && fieldValue.Len() > 0 {
	err := validateValue(fieldValue)
	if err != nil {
	return fmt.Errorf("%s, %w", field.Name, err)
	}
	}
	if tag, ok := field.Tag.Lookup(models.JsonTag); ok && tagContainsOmitempty(tag) {
	if fieldValue.Kind() == reflect.Ptr && fieldValue.IsZero() {
	continue
	}
	err := validateValue(fieldValue)
	if err != nil {
	return fmt.Errorf("%s, %w", field.Name, err)
	}
	}

[worryg0d]

Also what do you think about adding the the func for the next code:

func hasLength(val reflect.Value) bool {
  return val.Kind() == reflect.Array || val.Kind() == reflect.Slice || val.Kind() == reflect.Map
}

And you it inside your validateStruct func:

...
if hasLength(fieldValue) && fieldValue.Len() > 0 {
	...
}

[tengu-alt]

in the first suggestion you don't consider the required pointers and the non-required numbers but in general, I agree with simplification.
Also, agree with hasLength func

[worryg0d]

I see you reuse the following code multiple times. What do you think to wrap this logic inside the func?

func structValOrPtr(val reflect.Value) bool {
  return val.Kind() == reflect.Struct || isStructPtr(val)
}

[tengu-alt]

agree, refactored

[worryg0d]

I suggest you to add comma to the value omitempty because the current implementation skips even if there is no omitempty constraint, but there is an omitempty field name.
For instance we have the following struct:

type t struct {
  Omitempty string `json:"omitempty"` // this field considered to be required because there is no an omitempty 
                                      // constraint, but the current implementation doesn't validates the following one because 
                                      // there is a json field name omitempty
}

Also I think this is not a tag, but a constraint omitemptyConstraint

[tengu-alt]

agree, fixed

[worryg0d]

Let's keep code consistent and call this as one as you called other similar function

Suggested change

	func validatePointer(val reflect.Value) error {
	func validatePtr(val reflect.Value) error {

[tengu-alt]

refactored

[ribaraka]

I suggest you to move all these validations into the existing validation directory (i.e. pkg/validation)

[tengu-alt]

i think that is more appropriate to keep it in the utils because of more specific appointment(validation of manifests that are going to be applied)

[ribaraka]

isCollectionType

[tengu-alt]

agree, fixed