Skip to content

Commit

Permalink
Do not check presence field while validating
Browse files Browse the repository at this point in the history 
This change is required for backward compatibility

Change-Id: Ic818178dc8a9f52513085eb9d95ce12b6b609a5f
Closes-bug: 1787425
  • Loading branch information
Józef Wołoch authored and Michal Blotniak committed Aug 23, 2018
1 parent 01232f8 commit 3f262f5
Show file tree
Hide file tree
Showing 6 changed files with 33 additions and 159 deletions.
2 changes: 1 addition & 1 deletion Makefile
View file
Expand Up @@ -49,7 +49,7 @@ generate: reset_gen ## Run the source code generator
./bin/protoc -I ./vendor/ -I ./vendor/github.com/gogo/protobuf/protobuf -I ./proto --gogo_out=Mgoogle/protobuf/field_mask.proto=github.com/gogo/protobuf/types,plugins=grpc:$(GOPATH)/src/ proto/github.com/Juniper/contrail/pkg/services/generated.proto
./bin/protoc -I ./vendor/ -I ./vendor/github.com/gogo/protobuf/protobuf -I ./proto --doc_out=./doc --doc_opt=markdown,proto.md proto/github.com/Juniper/contrail/pkg/services/generated.proto proto/github.com/Juniper/contrail/pkg/models/generated.proto
go tool fix ./pkg/services/generated.pb.go
go fmt $(PACKAGE_PATH)/pkg/...
go fmt ./...
mkdir -p pkg/types/mock
mockgen -destination=pkg/types/mock/gen_types_mock.go -package=typesmock -source pkg/types/service.go
mkdir -p pkg/services/mock
Expand Down
40 changes: 14 additions & 26 deletions pkg/apisrv/test_data/test_validation.yml
View file
Expand Up @@ -40,21 +40,19 @@ test_data:
address_allocation_mode: "flat-subnet-preferred"
mac_limit_control: *mac_limit_control

vn_validation_test_fail_number_too_small: &vn_validation_test_fail_number_too_small
uuid: vn_validation_test_uuid
display_name: blue
alarm_validation_test_fail_number_too_small: &alarm_validation_test_fail_number_too_small
uuid: alarm_validation_test_uuid
display_name: alarm
parent_type: project
parent_uuid: admin_project_validation_test_uuid
mac_aging_time: -1
mac_limit_control: *mac_limit_control
alarm_severity: -1

vn_validation_test_fail_number_too_big: &vn_validation_test_fail_number_too_big
uuid: vn_validation_test_uuid
display_name: blue
vn_validation_test_fail_number_too_big: &alarm_validation_test_fail_number_too_big
uuid: alarm_validation_test_uuid
display_name: alarm
parent_type: project
parent_uuid: admin_project_validation_test_uuid
mac_aging_time: 86401
mac_limit_control: *mac_limit_control
alarm_severity: 3

vn_validation_test_fail_invalid_enum_value: &vn_validation_test_fail_invalid_enum_value
uuid: vn_validation_test_uuid
Expand Down Expand Up @@ -158,24 +156,24 @@ workflow:
- 200
expect: null

- name: create virtual network fail - number property too small
- name: create alarm fail - number property too small
request:
path: /virtual-networks
path: /alarms
method: POST
expected:
- 400
data:
virtual-network: *vn_validation_test_fail_number_too_small
alarm: *alarm_validation_test_fail_number_too_small
expect: null

- name: create virtual network fail - number property too big
- name: create alarm fail - number property too big
request:
path: /virtual-networks
method: POST
expected:
- 400
data:
virtual-network: *vn_validation_test_fail_number_too_big
alarm: *alarm_validation_test_fail_number_too_small
expect: null

- name: create virtual network fail - invalid enum value
Expand All @@ -196,14 +194,4 @@ workflow:
- 400
data:
virtual-network: *vn_validation_test_fail_missing_required_integer
expect: null

- name: create virtual network fail - required integer zero value
request:
path: /virtual-networks
method: POST
expected:
- 400
data:
virtual-network: *vn_validation_test_fail_required_integer_zero_value
expect: null
expect: null
98 changes: 0 additions & 98 deletions pkg/models/validation_test.go
View file
Expand Up @@ -305,47 +305,6 @@ func TestSchemaValidationJobTemplate(t *testing.T) {
FQName: []string{"a", "b"},
},
},
{
name: "Missing required string property",
jobTemplate: &JobTemplate{
UUID: "",
ParentUUID: "beef-beef",
ParentType: "global-system-config",
Name: "default-job-template",
JobTemplatePlaybooks: &PlaybookInfoListType{},
JobTemplateMultiDeviceJob: true,
JobTemplateConcurrencyLevel: "fabric",
FQName: []string{"a", "b"},
},
fails: true,
},
// {
// name: "Missing required boolean property",
// jobTemplate: &JobTemplate{
// UUID: "beef",
// ParentUUID: "beef-beef",
// ParentType: "global-system-config",
// Name: "default-job-template",
// JobTemplatePlaybooks: &PlaybookInfoListType{},
// JobTemplateMultiDeviceJob: false,
// FQName: []string{"a", "b"},
// },
// fails: true,
// },
{
name: "Missing required object property",
jobTemplate: &JobTemplate{
UUID: "beef",
ParentUUID: "beef-beef",
ParentType: "global-system-config",
Name: "default-job-template",
JobTemplatePlaybooks: nil,
JobTemplateMultiDeviceJob: true,
JobTemplateConcurrencyLevel: "fabric",
FQName: []string{"a", "b"},
},
fails: true,
},
{
name: "Bad parent type",
jobTemplate: &JobTemplate{
Expand All @@ -360,19 +319,6 @@ func TestSchemaValidationJobTemplate(t *testing.T) {
},
fails: true,
},
{
name: "missing name",
jobTemplate: &JobTemplate{
UUID: "beef",
ParentUUID: "beef-beef",
ParentType: "global-system-config",
JobTemplatePlaybooks: &PlaybookInfoListType{},
JobTemplateMultiDeviceJob: true,
JobTemplateConcurrencyLevel: "fabric",
FQName: []string{"a", "b"},
},
fails: true,
},
}

tv, err := NewTypeValidatorWithFormat()
Expand All @@ -397,11 +343,6 @@ func TestSchemaValidationBgpFamilyAttributes(t *testing.T) {
bgpFamilyAttributes *BgpFamilyAttributes
fails bool
}{
{
name: "Empty struct - validation fails",
bgpFamilyAttributes: &BgpFamilyAttributes{},
fails: true,
},
{
name: "Validation pass",
bgpFamilyAttributes: &BgpFamilyAttributes{
Expand All @@ -417,22 +358,6 @@ func TestSchemaValidationBgpFamilyAttributes(t *testing.T) {
},
fails: true,
},
{
name: "Number value too small",
bgpFamilyAttributes: &BgpFamilyAttributes{
AddressFamily: "inet",
LoopCount: -1,
},
fails: true,
},
{
name: "Number value too big",
bgpFamilyAttributes: &BgpFamilyAttributes{
AddressFamily: "inet",
LoopCount: 17,
},
fails: true,
},
}

tv, err := NewTypeValidatorWithFormat()
Expand All @@ -457,11 +382,6 @@ func TestSchemaValidationStaticMirrorNhType(t *testing.T) {
staticMirrorNhType *StaticMirrorNhType
fails bool
}{
{
name: "Empty struct - validation fails",
staticMirrorNhType: &StaticMirrorNhType{},
fails: true,
},
{
name: "Pass",
staticMirrorNhType: &StaticMirrorNhType{
Expand All @@ -471,24 +391,6 @@ func TestSchemaValidationStaticMirrorNhType(t *testing.T) {
},
fails: false,
},
{
name: "Missing string property",
staticMirrorNhType: &StaticMirrorNhType{
VtepDSTIPAddress: "",
VtepDSTMacAddress: "hoge",
Vni: 1,
},
fails: true,
},
{
name: "Missing integer property",
staticMirrorNhType: &StaticMirrorNhType{
VtepDSTIPAddress: "hoge",
VtepDSTMacAddress: "hoge",
Vni: 0,
},
fails: true,
},
}

tv, err := NewTypeValidatorWithFormat()
Expand Down
4 changes: 2 additions & 2 deletions pkg/services/quota_checker.go
View file
Expand Up @@ -13,9 +13,9 @@ type QuotaCheckerCounter struct {
}

// NewQuotaCheckerService creates QuotaCheckerService.
func NewQuotaCheckerService(db Service) *QuotaCheckerService {
func NewQuotaCheckerService(rs Service) *QuotaCheckerService {
return &QuotaCheckerService{
db: db,
ReadService: rs,
quotaCounter: &QuotaCheckerCounter{},
}
}
Expand Down
23 changes: 13 additions & 10 deletions tools/templates/quota_checker.tmpl
View file
Expand Up @@ -18,18 +18,21 @@ var _ = common.ErrorNotFound

type QuotaCheckerService struct {
BaseService
db Service
ReadService ReadService
quotaCounter QuotaCounter
}

type QuotaCounter interface {
{% for schema in schemas %}{% if schema.Type != "abstract" and schema.ID %}{% set ThisID = schema.ID %}
{% set name = schema.JSONSchema.GoName %}
{% set QuotaDef = types.QuotaType.Properties|dict_get_JSONSchema_by_string_key:ThisID %}
{% if QuotaDef %}
Count{{ name }}(ctx context.Context, obj *models.{{ name }}, prj *models.Project) (int64, error)
{% endif %}
{% endif %}{% endfor %}
{%- for schema in schemas -%}
{%- if schema.Type != "abstract" and schema.ID -%}
{%- set ThisID = schema.ID -%}
{%- set name = schema.JSONSchema.GoName -%}
{%- set QuotaDef = types.QuotaType.Properties|dict_get_JSONSchema_by_string_key:ThisID -%}
{%- if QuotaDef %}
Count{{ name }}(ctx context.Context, obj *models.{{ name }}, prj *models.Project) (int64, error)
{%- endif -%}
{%- endif -%}
{%- endfor %}
}


Expand All @@ -45,7 +48,7 @@ func (svc *QuotaCheckerService) Handle{{ name }}(ctx context.Context, obj *model
if err != nil || projectID == "" {
return errors.Wrapf(err, "error searching Project for {{ name }} with UUID %v (got '%v' Project UUID)", obj.GetUUID(), projectID)
}
project, err := svc.db.GetProject(ctx, &GetProjectRequest{ ID: projectID })
project, err := svc.ReadService.GetProject(ctx, &GetProjectRequest{ ID: projectID })
if err != nil {
return errors.Wrapf(err, "error (when handling {{ name }}) retrieving Project with UUID %v", projectID)
}
Expand Down Expand Up @@ -108,7 +111,7 @@ func (svc *QuotaCheckerService) GetProjectFor{{ name }}(ctx context.Context, obj
return parentUUID, nil
{% else %}
parent := obj.GetParentUUID()
parentResp, err := svc.db.Get{{ pdef.GoName }}(ctx, &Get{{ pdef.GoName }}Request{ ID: parent, })
parentResp, err := svc.ReadService.Get{{ pdef.GoName }}(ctx, &Get{{ pdef.GoName }}Request{ ID: parent, })
if err != nil {
return "", errors.Wrapf(err, "error retrieving parent for object {{ name }} with uuid %v", obj.GetUUID())
}
Expand Down
25 changes: 3 additions & 22 deletions tools/templates/type_validation.tmpl
View file
Expand Up @@ -7,12 +7,7 @@ import (

//This is needed to prevent an import error.
var _ = strings.Count

{# This macro is used to improve readability as braces has to be in the same line as if statement #}
{% macro reportMissingProperty(id, resourceType) %} {
return errors.Errorf("{{id}} property is missing for resource {{resourceType}}")
}
{% endmacro %}
var _ = errors.New

{# Validates properties of type: #}
{# - string (format, enum) #}
Expand Down Expand Up @@ -69,21 +64,7 @@ var _ = strings.Count

{% macro validateProperties(resource, properties) %}
{% for property, pdef in properties %}
{# Check required properties first. #}
{% if pdef.Presence == "required" or pdef.Presence == "true" and resource != "PolicyRuleType" %} {# TODO: temporary fix until validation is refactored #}
{% if pdef.Type == "string" %}
if obj.{{pdef.GoName}} == "" {{reportMissingProperty(pdef.ID, resource)}}
{% elif pdef.Type == "integer" or pdef.Type == "number" %}
if obj.{{pdef.GoName}} == 0 {{reportMissingProperty(pdef.ID, resource)}}
{% elif pdef.Type == "boolean" %}
if obj.{{pdef.GoName}} == false {{reportMissingProperty(pdef.ID, resource)}}
{% elif pdef.Type == "object" %}
if obj.{{pdef.GoName}} == nil {{reportMissingProperty(pdef.ID, resource)}}
{% elif pdef.Type == "array" %}
if len(obj.{{pdef.GoName}}) == 0 {{reportMissingProperty(pdef.ID, resource)}}
{% endif %}
{% endif %}

{# TODO: handle presence #}
{# In case of array we need to know definitions of objects stored in this array #}
{% if pdef.Type == "array" %}
{% set jsonSchema = pdef.Items %}
Expand All @@ -94,7 +75,7 @@ var _ = strings.Count
{# Check whether validation is needed #}
{% if jsonSchema.Type == "string" and (jsonSchema.Enum|length > 0 or jsonSchema.Format != "") %}
{% set validationRequired = true %}
{% elif jsonSchema.Type == "integer" and (jsonSchema.Maximum != nil or jsonSchema.Minimum != nil) %}
{% elif jsonSchema.Type == "integer" and (jsonSchema.Maximum != nil or jsonSchema.Minimum != nil) and jsonSchema.Presence == "required" %}
{% set validationRequired = true %}
{% elif jsonSchema.Type == "object" %}
{% set validationRequired = true %}
Expand Down

0 comments on commit 3f262f5

Please sign in to comment.