Skip to content

Commit

Permalink
Validate require vs optional and return validation errors (#527)
Browse files Browse the repository at this point in the history 
Introduce the first error state required vs optional. This showcases the
concept before going into the breadth of error states.
  • Loading branch information
@sourishkrout
sourishkrout authored Mar 13, 2024
1 parent f7d0aca commit 9d19f68
Show file tree
Hide file tree
Showing 11 changed files with 885 additions and 321 deletions.
8 changes: 8 additions & 0 deletions internal/api/runme/runner/v1/runner.proto
View file
Original file line number Diff line number Diff line change
Expand Up @@ -320,6 +320,14 @@ message MonitorEnvStoreResponseSnapshot {
string create_time = 7;

string update_time = 8;

repeated Error errors = 9;
}

message Error {
uint32 code = 1;

string message = 2;
}

enum Status {
Expand Down
337 changes: 211 additions & 126 deletions internal/gen/proto/go/runme/runner/v1/runner.pb.go
View file

Large diffs are not rendered by default.

24 changes: 24 additions & 0 deletions internal/gen/proto/ts/runme/runner/v1/runner_pb.d.ts
View file
Original file line number Diff line number Diff line change
Expand Up @@ -570,6 +570,23 @@ export interface MonitorEnvStoreResponseSnapshot_SnapshotEnv {
* @generated from protobuf field: string update_time = 8;
*/
updateTime: string;
/**
* @generated from protobuf field: repeated runme.runner.v1.MonitorEnvStoreResponseSnapshot.Error errors = 9;
*/
errors: MonitorEnvStoreResponseSnapshot_Error[];
}
/**
* @generated from protobuf message runme.runner.v1.MonitorEnvStoreResponseSnapshot.Error
*/
export interface MonitorEnvStoreResponseSnapshot_Error {
/**
* @generated from protobuf field: uint32 code = 1;
*/
code: number;
/**
* @generated from protobuf field: string message = 2;
*/
message: string;
}
/**
* @generated from protobuf enum runme.runner.v1.MonitorEnvStoreResponseSnapshot.Status
Expand Down Expand Up @@ -850,6 +867,13 @@ declare class MonitorEnvStoreResponseSnapshot_SnapshotEnv$Type extends MessageTy
* @generated MessageType for protobuf message runme.runner.v1.MonitorEnvStoreResponseSnapshot.SnapshotEnv
*/
export declare const MonitorEnvStoreResponseSnapshot_SnapshotEnv: MonitorEnvStoreResponseSnapshot_SnapshotEnv$Type;
declare class MonitorEnvStoreResponseSnapshot_Error$Type extends MessageType<MonitorEnvStoreResponseSnapshot_Error> {
constructor();
}
/**
* @generated MessageType for protobuf message runme.runner.v1.MonitorEnvStoreResponseSnapshot.Error
*/
export declare const MonitorEnvStoreResponseSnapshot_Error: MonitorEnvStoreResponseSnapshot_Error$Type;
declare class MonitorEnvStoreResponse$Type extends MessageType<MonitorEnvStoreResponse> {
constructor();
}
Expand Down
16 changes: 15 additions & 1 deletion internal/gen/proto/ts/runme/runner/v1/runner_pb.js
View file
Original file line number Diff line number Diff line change
Expand Up @@ -474,7 +474,8 @@ class MonitorEnvStoreResponseSnapshot_SnapshotEnv$Type extends MessageType {
{ no: 5, name: "original_value", kind: "scalar", T: 9 /*ScalarType.STRING*/ },
{ no: 6, name: "resolved_value", kind: "scalar", T: 9 /*ScalarType.STRING*/ },
{ no: 7, name: "create_time", kind: "scalar", T: 9 /*ScalarType.STRING*/ },
{ no: 8, name: "update_time", kind: "scalar", T: 9 /*ScalarType.STRING*/ }
{ no: 8, name: "update_time", kind: "scalar", T: 9 /*ScalarType.STRING*/ },
{ no: 9, name: "errors", kind: "message", repeat: 1 /*RepeatType.PACKED*/, T: () => MonitorEnvStoreResponseSnapshot_Error }
]);
}
}
Expand All @@ -483,6 +484,19 @@ class MonitorEnvStoreResponseSnapshot_SnapshotEnv$Type extends MessageType {
*/
export const MonitorEnvStoreResponseSnapshot_SnapshotEnv = new MonitorEnvStoreResponseSnapshot_SnapshotEnv$Type();
// @generated message type with reflection information, may provide speed optimized methods
class MonitorEnvStoreResponseSnapshot_Error$Type extends MessageType {
constructor() {
super("runme.runner.v1.MonitorEnvStoreResponseSnapshot.Error", [
{ no: 1, name: "code", kind: "scalar", T: 13 /*ScalarType.UINT32*/ },
{ no: 2, name: "message", kind: "scalar", T: 9 /*ScalarType.STRING*/ }
]);
}
}
/**
* @generated MessageType for protobuf message runme.runner.v1.MonitorEnvStoreResponseSnapshot.Error
*/
export const MonitorEnvStoreResponseSnapshot_Error = new MonitorEnvStoreResponseSnapshot_Error$Type();
// @generated message type with reflection information, may provide speed optimized methods
class MonitorEnvStoreResponse$Type extends MessageType {
constructor() {
super("runme.runner.v1.MonitorEnvStoreResponse", [
Expand Down
143 changes: 103 additions & 40 deletions internal/owl/graph.go
View file
Original file line number Diff line number Diff line change
Expand Up @@ -16,9 +16,9 @@ type specType struct {
}

var (
Schema graphql.Schema
EnvironmentType, ValidateType *graphql.Object
SpecTypes map[string]*specType
Schema graphql.Schema
EnvironmentType, ValidateType, RenderType, SpecTypeErrorsType *graphql.Object
SpecTypes map[string]*specType
)

type QueryNodeReducer func(*ast.OperationDefinition, *ast.SelectionSet) (*ast.SelectionSet, error)
Expand Down Expand Up @@ -66,7 +66,30 @@ func registerSpec(spec string, sensitive, mask bool, resolver graphql.FieldResol
},
},
"errors": &graphql.Field{
Type: graphql.NewList(graphql.String),
Type: graphql.NewList(SpecTypeErrorsType),
Resolve: func(p graphql.ResolveParams) (interface{}, error) {
opSet, ok := p.Source.(*OperationSet)
if !ok {
return nil, errors.New("source is not an OperationSet")
}

// todo(sebastian): move into interface?
var verrs []*SetVarError
for _, spec := range opSet.specs {
if spec.Spec.Error == nil {
continue
}

code := spec.Spec.Error.Code()
verr := &SetVarError{
Code: int(code),
Message: spec.Spec.Error.Message(),
}
verrs = append(verrs, verr)
}

return verrs, nil
},
},
"done": &graphql.Field{
Type: EnvironmentType,
Expand Down Expand Up @@ -98,26 +121,34 @@ func specResolver(mutator SpecResolverMutator) graphql.FieldResolveFn {
opSet := p.Source.(*OperationSet)
for _, kArg := range keysArg {
k := kArg.(string)
v, vOk := opSet.values[k]
s, sOk := opSet.specs[k]
if !vOk && !sOk {
// todo(sebastian): should UNSET/delete ever come in here?
val, valOk := opSet.values[k]
spec, specOk := opSet.specs[k]
if !valOk && !specOk {
// todo(sebastian): superfluous keys are only possible in hand-written queries
continue
}

// skip if last known status was DELETED
if vOk && v.Value.Status == "DELETED" {
if valOk && val.Value.Status == "DELETED" {
continue
}

if vOk && v.Value.Status == "UNRESOLVED" {
// todo(sebastian): most obvious validation error?
continue
// todo(sebastian): poc, move to something more generic
if valOk && val.Value.Status == "UNRESOLVED" {
if !specOk {
// todo(sebastian): without spec, we can't decide if unresolved is valid - should be impossible
continue
}

if spec.Spec.Required {
spec.Spec.Error = RequiredError{varItem: &SetVarItem{Var: val.Var, Value: val.Value, Spec: spec.Spec}}
continue
}
}

mutator(v, s, insecure)
if sOk {
s.Spec.Checked = true
mutator(val, spec, insecure)
if specOk {
spec.Spec.Checked = true
}
}

Expand Down Expand Up @@ -162,11 +193,21 @@ func resolveSnapshot() graphql.FieldResolveFn {
if !ok {
return nil, fmt.Errorf("missing spec for %s", v.Var.Key)
}
snapshot = append(snapshot, &SetVarItem{

item := &SetVarItem{
Var: v.Var,
Value: v.Value,
Spec: s.Spec,
})
}
if s.Spec != nil && s.Spec.Error != nil {
code := s.Spec.Error.Code()
item.Errors = append(item.Errors, &SetVarError{
Code: int(code),
Message: s.Spec.Error.Message(),
})
}

snapshot = append(snapshot, item)
}

snapshot.sort()
Expand Down Expand Up @@ -347,6 +388,18 @@ func init() {
}),
)

SpecTypeErrorsType = graphql.NewObject(graphql.ObjectConfig{
Name: "SpecTypeErrorsType",
Fields: graphql.Fields{
"message": &graphql.Field{
Type: graphql.String,
},
"code": &graphql.Field{
Type: graphql.Int,
},
},
})

ValidateType = graphql.NewObject(graphql.ObjectConfig{
Name: "ValidateType",
Fields: (graphql.FieldsThunk)(func() graphql.Fields {
Expand Down Expand Up @@ -445,10 +498,39 @@ func init() {
},
}),
},
"errors": &graphql.Field{
Type: graphql.NewList(SpecTypeErrorsType),
Resolve: func(p graphql.ResolveParams) (interface{}, error) {
vars, ok := p.Source.(*SetVarItem)
if !ok {
return nil, errors.New("source is not a *SetVarItem")
}

return vars.Errors, nil
},
},
},
},
)

RenderType = graphql.NewObject(graphql.ObjectConfig{
Name: "RenderType",
Fields: (graphql.FieldsThunk)(func() graphql.Fields {
return graphql.Fields{
"snapshot": &graphql.Field{
Type: graphql.NewNonNull(graphql.NewList(VariableType)),
Args: graphql.FieldConfigArgument{
"insecure": &graphql.ArgumentConfig{
Type: graphql.Boolean,
DefaultValue: false,
},
},
Resolve: resolveSnapshot(),
},
}
}),
})

VariableInputType := graphql.NewInputObject(graphql.InputObjectConfig{
Name: "VariableInput",
Fields: graphql.InputObjectConfigFieldMap{
Expand Down Expand Up @@ -608,30 +690,11 @@ func init() {
return p.Source, nil
},
},
// "location": &graphql.Field{
// Type: graphql.NewNonNull(graphql.String),
// Resolve: func(p graphql.ResolveParams) (interface{}, error) {
// // todo(sebastian): bring this back?
// switch p.Source.(type) {
// case *OperationSet:
// opSet := p.Source.(*OperationSet)
// return opSet.operation.location, nil
// default:
// // noop
// }

// return nil, nil
// },
// },
"snapshot": &graphql.Field{
Type: graphql.NewNonNull(graphql.NewList(VariableType)),
Args: graphql.FieldConfigArgument{
"insecure": &graphql.ArgumentConfig{
Type: graphql.Boolean,
DefaultValue: false,
},
"render": &graphql.Field{
Type: RenderType,
Resolve: func(p graphql.ResolveParams) (interface{}, error) {
return p.Source, nil
},
Resolve: resolveSnapshot(),
},
}
}),
Expand Down
353 changes: 270 additions & 83 deletions internal/owl/graph_test.go
View file

Large diffs are not rendered by default.

Loading

0 comments on commit 9d19f68

Please sign in to comment.