Skip to content

Commit

Permalink
Added SchemaProxy mutex #163
Browse files Browse the repository at this point in the history
Rendering causes bits to be set on structs in the model. Concurrency causes race issues with reading a writing of these bits, as there is no locking in the model.

Until now. locking prevents concurrent renders that use a shared model from conflicting with one another.

Addresses #163 and pb33f/libopenapi-validator#23

Signed-off-by: quobix <dave@quobix.com>
  • Loading branch information
daveshanley committed Sep 18, 2023
1 parent dab6346 commit 53a46f4
Show file tree
Hide file tree
Showing 3 changed files with 45 additions and 36 deletions.
59 changes: 30 additions & 29 deletions datamodel/high/base/schema.go
Original file line number Diff line number Diff line change
Expand Up @@ -176,50 +176,50 @@ func NewSchema(schema *base.Schema) *Schema {
s.UniqueItems = &schema.UniqueItems.Value
}
if !schema.Contains.IsEmpty() {
s.Contains = &SchemaProxy{schema: &lowmodel.NodeReference[*base.SchemaProxy]{
s.Contains = NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: schema.Contains.ValueNode,
Value: schema.Contains.Value,
}}
})
}
if !schema.If.IsEmpty() {
s.If = &SchemaProxy{schema: &lowmodel.NodeReference[*base.SchemaProxy]{
s.If = NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: schema.If.ValueNode,
Value: schema.If.Value,
}}
})
}
if !schema.Else.IsEmpty() {
s.Else = &SchemaProxy{schema: &lowmodel.NodeReference[*base.SchemaProxy]{
s.Else = NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: schema.Else.ValueNode,
Value: schema.Else.Value,
}}
})
}
if !schema.Then.IsEmpty() {
s.Then = &SchemaProxy{schema: &lowmodel.NodeReference[*base.SchemaProxy]{
s.Then = NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: schema.Then.ValueNode,
Value: schema.Then.Value,
}}
})
}
if !schema.PropertyNames.IsEmpty() {
s.PropertyNames = &SchemaProxy{schema: &lowmodel.NodeReference[*base.SchemaProxy]{
s.PropertyNames = NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: schema.PropertyNames.ValueNode,
Value: schema.PropertyNames.Value,
}}
})
}
if !schema.UnevaluatedItems.IsEmpty() {
s.UnevaluatedItems = &SchemaProxy{schema: &lowmodel.NodeReference[*base.SchemaProxy]{
s.UnevaluatedItems = NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: schema.UnevaluatedItems.ValueNode,
Value: schema.UnevaluatedItems.Value,
}}
})
}
// check if unevaluated properties is a schema
if !schema.UnevaluatedProperties.IsEmpty() && schema.UnevaluatedProperties.Value.IsA() {
s.UnevaluatedProperties = &DynamicValue[*SchemaProxy, *bool]{
A: &SchemaProxy{
schema: &lowmodel.NodeReference[*base.SchemaProxy]{
A: NewSchemaProxy(
&lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: schema.UnevaluatedProperties.ValueNode,
Value: schema.UnevaluatedProperties.Value.A,
},
},
),
N: 0,
}
}
Expand Down Expand Up @@ -325,11 +325,11 @@ func NewSchema(schema *base.Schema) *Schema {

// for every item, build schema async
buildSchema := func(sch lowmodel.ValueReference[*base.SchemaProxy], idx int, bChan chan buildResult) {
p := &SchemaProxy{schema: &lowmodel.NodeReference[*base.SchemaProxy]{
p := NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: sch.ValueNode,
Value: sch.Value,
Reference: sch.GetReference(),
}}
})

bChan <- buildResult{idx: idx, s: p}
}
Expand Down Expand Up @@ -358,13 +358,12 @@ func NewSchema(schema *base.Schema) *Schema {
buildProps := func(k lowmodel.KeyReference[string], v lowmodel.ValueReference[*base.SchemaProxy],
props map[string]*SchemaProxy, sw int,
) {
props[k.Value] = &SchemaProxy{
schema: &lowmodel.NodeReference[*base.SchemaProxy]{
Value: v.Value,
KeyNode: k.KeyNode,
ValueNode: v.ValueNode,
},
}
props[k.Value] = NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{
Value: v.Value,
KeyNode: k.KeyNode,
ValueNode: v.ValueNode,
},
)

switch sw {
case 0:
Expand Down Expand Up @@ -418,11 +417,13 @@ func NewSchema(schema *base.Schema) *Schema {
}
if !schema.Items.IsEmpty() {
if schema.Items.Value.IsA() {
items = &DynamicValue[*SchemaProxy, bool]{A: &SchemaProxy{schema: &lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: schema.Items.ValueNode,
Value: schema.Items.Value.A,
KeyNode: schema.Items.KeyNode,
}}}
items = &DynamicValue[*SchemaProxy, bool]{
A: NewSchemaProxy(&lowmodel.NodeReference[*base.SchemaProxy]{
ValueNode: schema.Items.ValueNode,
Value: schema.Items.Value.A,
KeyNode: schema.Items.KeyNode,
},
)}
} else {
items = &DynamicValue[*SchemaProxy, bool]{N: 1, B: schema.Items.Value.B}
}
Expand Down
14 changes: 11 additions & 3 deletions datamodel/high/base/schema_proxy.go
Original file line number Diff line number Diff line change
Expand Up @@ -9,6 +9,7 @@ import (
"github.com/pb33f/libopenapi/datamodel/low/base"
"github.com/pb33f/libopenapi/utils"
"gopkg.in/yaml.v3"
"sync"
)

// SchemaProxy exists as a stub that will create a Schema once (and only once) the Schema() method is called. An
Expand Down Expand Up @@ -49,40 +50,47 @@ type SchemaProxy struct {
buildError error
rendered *Schema
refStr string
lock *sync.Mutex
}

// NewSchemaProxy creates a new high-level SchemaProxy from a low-level one.
func NewSchemaProxy(schema *low.NodeReference[*base.SchemaProxy]) *SchemaProxy {
return &SchemaProxy{schema: schema}
return &SchemaProxy{schema: schema, lock: &sync.Mutex{}}
}

// CreateSchemaProxy will create a new high-level SchemaProxy from a high-level Schema, this acts the same
// as if the SchemaProxy is pre-rendered.
func CreateSchemaProxy(schema *Schema) *SchemaProxy {
return &SchemaProxy{rendered: schema}
return &SchemaProxy{rendered: schema, lock: &sync.Mutex{}}
}

// CreateSchemaProxyRef will create a new high-level SchemaProxy from a reference string, this is used only when
// building out new models from scratch that require a reference rather than a schema implementation.
func CreateSchemaProxyRef(ref string) *SchemaProxy {
return &SchemaProxy{refStr: ref}
return &SchemaProxy{refStr: ref, lock: &sync.Mutex{}}
}

// Schema will create a new Schema instance using NewSchema from the low-level SchemaProxy backing this high-level one.
// If there is a problem building the Schema, then this method will return nil. Use GetBuildError to gain access
// to that building error.
func (sp *SchemaProxy) Schema() *Schema {
sp.lock.Lock()
if sp.rendered == nil {

s := sp.schema.Value.Schema()
if s == nil {
sp.buildError = sp.schema.Value.GetBuildError()
sp.lock.Unlock()
return nil
}
sch := NewSchema(s)
sch.ParentProxy = sp

sp.rendered = sch
sp.lock.Unlock()
return sch
} else {
sp.lock.Unlock()
return sp.rendered
}
}
Expand Down
8 changes: 4 additions & 4 deletions datamodel/high/base/schema_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -57,7 +57,7 @@ func TestNewSchemaProxy(t *testing.T) {
ValueNode: idxNode.Content[0],
}

sch1 := SchemaProxy{schema: &lowproxy}
sch1 := NewSchemaProxy(&lowproxy)
assert.Nil(t, sch1.Schema())
assert.Error(t, sch1.GetBuildError())

Expand Down Expand Up @@ -98,7 +98,7 @@ func TestNewSchemaProxyRender(t *testing.T) {
ValueNode: idxNode.Content[0],
}

sch1 := SchemaProxy{schema: &lowproxy}
sch1 := NewSchemaProxy(&lowproxy)
assert.NotNil(t, sch1.Schema())
assert.NoError(t, sch1.GetBuildError())

Expand Down Expand Up @@ -1125,7 +1125,7 @@ components:
ValueNode: idxNode.Content[0],
}

sch1 := SchemaProxy{schema: &lowproxy}
sch1 := NewSchemaProxy(&lowproxy)
compiled := sch1.Schema()

// now render it out, it should be identical.
Expand Down Expand Up @@ -1177,7 +1177,7 @@ components:
ValueNode: idxNode.Content[0],
}

sch1 := SchemaProxy{schema: &lowproxy}
sch1 := NewSchemaProxy(&lowproxy)
compiled := sch1.Schema()

// now render it out, it should be identical.
Expand Down

0 comments on commit 53a46f4

Please sign in to comment.