Skip to content

Commit

Permalink
Fix field tag parsing for referenced schemas (#20)
Browse files Browse the repository at this point in the history
  • Loading branch information
vearutop authored Apr 25, 2021
1 parent 91aefa3 commit 6bece79
Show file tree
Hide file tree
Showing 9 changed files with 121 additions and 109 deletions.
4 changes: 2 additions & 2 deletions .github/workflows/golangci-lint.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,10 +14,10 @@ jobs:
steps:
- uses: actions/checkout@v2
- name: golangci-lint
uses: golangci/[email protected].1
uses: golangci/[email protected].2
with:
# Required: the version of golangci-lint is required and must be specified without patch version: we always use the latest patch version.
version: v1.38.0
version: v1.39.0

# Optional: working directory, useful for monorepos
# working-directory: somedir
Expand Down
6 changes: 4 additions & 2 deletions .gitignore
Original file line number Diff line number Diff line change
@@ -1,3 +1,5 @@
/.idea
/unit.coverprofile
/bench-*.txt
/*.coverprofile
/.vscode
/bench-*.txt
/vendor
22 changes: 15 additions & 7 deletions .golangci.yml
Original file line number Diff line number Diff line change
@@ -1,7 +1,6 @@
# See https://github.com/golangci/golangci-lint/blob/master/.golangci.example.yml
run:
tests: true
deadline: 5m

linters-settings:
errcheck:
Expand All @@ -22,21 +21,30 @@ linters:
enable-all: true
disable:
- goerr113
- gochecknoglobals
- funlen
- gocognit
- wrapcheck
- exhaustivestruct
- cyclop
- interfacer
- lll
- maligned
- gochecknoglobals
- gomnd
- wrapcheck
- paralleltest
- forbidigo
- exhaustivestruct
- interfacer # deprecated
- forcetypeassert
- scopelint # deprecated
- ifshort # too many false positives

issues:
exclude-use-default: false
exclude-rules:
- linters:
- gomnd
- goconst
- paralleltest
- forbidigo
- goerr113
- noctx
- funlen
- dupl
path: "_test.go"
9 changes: 6 additions & 3 deletions Makefile
Original file line number Diff line number Diff line change
@@ -1,4 +1,5 @@
JSON_CLI_VERSION := "v1.7.7"
#GOLANGCI_LINT_VERSION := "v1.39.0" # Optional configuration to pinpoint golangci-lint version.

# The head of Makefile determines location of dev-go to include standard targets.
GO ?= go
Expand All @@ -9,7 +10,7 @@ ifneq "$(GOFLAGS)" ""
endif

ifneq "$(wildcard ./vendor )" ""
$(info >> using vendor)
$(info Using vendor)
modVendor = -mod=vendor
ifeq (,$(findstring -mod,$(GOFLAGS)))
export GOFLAGS := ${GOFLAGS} ${modVendor}
Expand All @@ -28,10 +29,12 @@ ifeq ($(DEVGO_PATH),)
endif

-include $(DEVGO_PATH)/makefiles/main.mk
-include $(DEVGO_PATH)/makefiles/test-unit.mk
-include $(DEVGO_PATH)/makefiles/lint.mk
-include $(DEVGO_PATH)/makefiles/test-unit.mk
-include $(DEVGO_PATH)/makefiles/bench.mk
-include $(DEVGO_PATH)/makefiles/github-actions.mk
-include $(DEVGO_PATH)/makefiles/reset-ci.mk

# Add your custom targets here.

## Run tests
test: test-unit
Expand Down
3 changes: 3 additions & 0 deletions dev_test.go
Original file line number Diff line number Diff line change
@@ -0,0 +1,3 @@
package jsonschema_test

import _ "github.com/bool64/dev" // Include CI/Dev scripts to project.
4 changes: 2 additions & 2 deletions go.mod
Original file line number Diff line number Diff line change
Expand Up @@ -3,9 +3,9 @@ module github.com/swaggest/jsonschema-go
go 1.13

require (
github.com/bool64/dev v0.1.24
github.com/bool64/dev v0.1.27
github.com/stretchr/testify v1.4.0
github.com/swaggest/assertjson v1.6.3
github.com/swaggest/assertjson v1.6.4
github.com/swaggest/refl v0.1.7
github.com/yudai/gojsondiff v1.0.0
)
113 changes: 37 additions & 76 deletions go.sum

Large diffs are not rendered by default.

40 changes: 28 additions & 12 deletions reflect.go
Original file line number Diff line number Diff line change
Expand Up @@ -109,10 +109,15 @@ func checkSchemaSetup(v reflect.Value, s *Schema) (bool, error) {
//
// Values can be populated from field tags of original field:
// type MyObj struct {
// BoundedNumber `query:"boundedNumber" minimum:"-100" maximum:"100"`
// SpecialString `json:"specialString" pattern:"^[a-z]{4}$" minLength:"4" maxLength:"4"`
// BoundedNumber int `query:"boundedNumber" minimum:"-100" maximum:"100"`
// SpecialString string `json:"specialString" pattern:"^[a-z]{4}$" minLength:"4" maxLength:"4"`
// }
//
// Note: field tags are only applied to inline schemas, if you use named type then referenced schema
// will be created and tags will be ignored. This happens because referenced schema can be used in
// multiple fields with conflicting tags, therefore customization of referenced schema has to done on
// the type itself via RawExposer, Exposer or Preparer.
//
// These tags can be used:
// - `title`, https://json-schema.org/draft-04/json-schema-validation.html#rfc.section.6.1
// - `description`, https://json-schema.org/draft-04/json-schema-validation.html#rfc.section.6.1
Expand Down Expand Up @@ -157,7 +162,7 @@ func (r *Reflector) Reflect(i interface{}, options ...func(*ReflectContext)) (Sc
option(&rc)
}

schema, err := r.reflect(i, &rc)
schema, err := r.reflect(i, &rc, false)
if err == nil && len(rc.definitions) > 0 {
schema.Definitions = make(map[string]SchemaOrBool, len(rc.definitions))

Expand Down Expand Up @@ -195,7 +200,7 @@ func removeNull(t *Type) {
}
}

func (r *Reflector) reflectDefer(defName string, typeString refl.TypeString, rc *ReflectContext, schema Schema) Schema {
func (r *Reflector) reflectDefer(defName string, typeString refl.TypeString, rc *ReflectContext, schema Schema, keepType bool) Schema {
if rc.RootNullable && len(rc.Path) == 0 {
schema.AddType(Null)
}
Expand Down Expand Up @@ -231,10 +236,16 @@ func (r *Reflector) reflectDefer(defName string, typeString refl.TypeString, rc
ref := Ref{Path: rc.DefinitionsPrefix, Name: defName}
rc.definitionRefs[typeString] = ref

return ref.Schema()
s := ref.Schema()

if keepType {
s.Type = schema.Type
}

return s
}

func (r *Reflector) reflect(i interface{}, rc *ReflectContext) (schema Schema, err error) {
func (r *Reflector) reflect(i interface{}, rc *ReflectContext, keepType bool) (schema Schema, err error) {
var (
typeString refl.TypeString
defName string
Expand All @@ -253,7 +264,7 @@ func (r *Reflector) reflect(i interface{}, rc *ReflectContext) (schema Schema, e
return
}

schema = r.reflectDefer(defName, typeString, rc, schema)
schema = r.reflectDefer(defName, typeString, rc, schema, keepType)
}()

if t == nil || t == typeOfEmptyInterface {
Expand Down Expand Up @@ -444,7 +455,7 @@ func (r *Reflector) kindSwitch(t reflect.Type, v reflect.Value, schema *Schema,
itemValue = reflect.New(elemType).Interface()
}

itemsSchema, err := r.reflect(itemValue, rc)
itemsSchema, err := r.reflect(itemValue, rc, false)
if err != nil {
return err
}
Expand All @@ -462,7 +473,7 @@ func (r *Reflector) kindSwitch(t reflect.Type, v reflect.Value, schema *Schema,
itemValue = reflect.New(elemType).Interface()
}

additionalPropertiesSchema, err := r.reflect(itemValue, rc)
additionalPropertiesSchema, err := r.reflect(itemValue, rc, false)
if err != nil {
return err
}
Expand Down Expand Up @@ -568,7 +579,7 @@ func (r *Reflector) walkProperties(v reflect.Value, parent *Schema, rc *ReflectC

rc.Path = append(rc.Path, propName)

propertySchema, err := r.reflect(fieldVal, rc)
propertySchema, err := r.reflect(fieldVal, rc, true)
if err != nil {
return err
}
Expand All @@ -585,7 +596,6 @@ func (r *Reflector) walkProperties(v reflect.Value, parent *Schema, rc *ReflectC
}

err = refl.PopulateFieldsFromTags(&propertySchema, field.Tag)

if err != nil {
return err
}
Expand All @@ -597,6 +607,11 @@ func (r *Reflector) walkProperties(v reflect.Value, parent *Schema, rc *ReflectC

reflectEnum(&propertySchema, field, fieldVal)

// Remove temporary kept type from referenced schema.
if propertySchema.Ref != nil {
propertySchema.Type = nil
}

if parent.Properties == nil {
parent.Properties = make(map[string]SchemaOrBool, 1)
}
Expand Down Expand Up @@ -673,7 +688,8 @@ func checkDefault(propertySchema *Schema, field reflect.StructField) error {
}

func checkNullability(propertySchema *Schema, rc *ReflectContext, ft reflect.Type) {
if propertySchema.HasType(Array) || (propertySchema.HasType(Object) && len(propertySchema.Properties) == 0) {
if propertySchema.HasType(Array) ||
(propertySchema.HasType(Object) && len(propertySchema.Properties) == 0 && propertySchema.Ref == nil) {
propertySchema.AddType(Null)
}

Expand Down
29 changes: 24 additions & 5 deletions reflect_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -272,10 +272,7 @@ func TestReflector_Reflect_mapping(t *testing.T) {
s, err := rf.Reflect(testWrapParams{}, jsonschema.RootRef)
require.NoError(t, err)

j, err := json.MarshalIndent(s, "", " ")
require.NoError(t, err)

assertjson.Equal(t, []byte(`{
assertjson.EqualMarshal(t, []byte(`{
"$ref": "#/definitions/JsonschemaGoTestTestWrapParams",
"definitions": {
"JsonschemaGoTestDeepReplacementTag": {
Expand All @@ -299,7 +296,7 @@ func TestReflector_Reflect_mapping(t *testing.T) {
"type": "object"
}
}
}`), j, string(j))
}`), s)
}

func TestReflector_Reflect_map(t *testing.T) {
Expand Down Expand Up @@ -738,3 +735,25 @@ func TestInterceptType(t *testing.T) {
assert.NoError(t, err)
assertjson.EqualMarshal(t, []byte(`{"type":["null", "number"]}`), s)
}

func TestReflector_Reflect_Ref(t *testing.T) {
type Symbol string

type topTracesInput struct {
RootSymbol Symbol `json:"rootSymbol" minLength:"5" example:"my_func" default:"main()"`
}

r := jsonschema.Reflector{}
s, err := r.Reflect(topTracesInput{})
assert.NoError(t, err)
assertjson.EqualMarshal(t, []byte(`{
"definitions":{"JsonschemaGoTestSymbol":{"type":"string"}},
"properties":{
"rootSymbol":{
"$ref":"#/definitions/JsonschemaGoTestSymbol","default":"main()",
"examples":["my_func"],"minLength":5
}
},
"type":"object"
}`), s)
}

0 comments on commit 6bece79

Please sign in to comment.