forked from kubeflow/model-registry
-
Notifications
You must be signed in to change notification settings - Fork 0
Commit
This commit does not belong to any branch on this repository, and may belong to a fork outside of the repository.
Override not editable fields during updates (kubeflow#200)
* Override not editable fields during updates * Setup test for not editable fields override
- Loading branch information
Showing
8 changed files
with
457 additions
and
32 deletions.
There are no files selected for viewing
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -3,6 +3,7 @@ | |
| model-registry | ||
| metadata.sqlite.db | ||
| vendor | ||
| coverage.txt | ||
|
|
||
| # Robot Framework files | ||
| log.html | ||
|
|
||
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
Some generated files are not rendered by default. Learn more about how customized files appear on GitHub.
Oops, something went wrong.
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
This file contains bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,160 @@ | ||
| package converter | ||
|
|
||
| import ( | ||
| "fmt" | ||
| "go/ast" | ||
| "go/parser" | ||
| "go/token" | ||
| "os" | ||
| "reflect" | ||
| "regexp" | ||
| "strings" | ||
| "testing" | ||
|
|
||
| "github.com/opendatahub-io/model-registry/pkg/openapi" | ||
| ) | ||
|
|
||
| // visitor | ||
| type visitor struct { | ||
| t *testing.T | ||
| entities map[string]*oapiEntity | ||
| } | ||
|
|
||
| func newVisitor(t *testing.T, f *ast.File) visitor { | ||
| return visitor{ | ||
| t: t, | ||
| entities: map[string]*oapiEntity{ | ||
| "RegisteredModel": { | ||
| obj: openapi.RegisteredModel{}, | ||
| }, | ||
| "ModelVersion": { | ||
| obj: openapi.ModelVersion{}, | ||
| }, | ||
| "ModelArtifact": { | ||
| obj: openapi.ModelArtifact{}, | ||
| }, | ||
| "ServingEnvironment": { | ||
| obj: openapi.ServingEnvironment{}, | ||
| }, | ||
| "InferenceService": { | ||
| obj: openapi.InferenceService{}, | ||
| }, | ||
| "ServeModel": { | ||
| obj: openapi.ServeModel{}, | ||
| }, | ||
| }, | ||
| } | ||
| } | ||
|
|
||
| func (v *visitor) extractGroup(regex *regexp.Regexp, s string) string { | ||
| extracted := regex.FindStringSubmatch(s) | ||
| if len(extracted) != 2 { | ||
| v.t.Errorf("unable to extract groups from %s for %s", regex.String(), s) | ||
| } | ||
| // the first one is the wole matched string, the second one is the group | ||
| return extracted[1] | ||
| } | ||
|
|
||
| func (v *visitor) getEntity(name string) *oapiEntity { | ||
| val, ok := v.entities[name] | ||
| if !ok { | ||
| v.t.Errorf("openapi entity not found in the entities map: %s", name) | ||
| } | ||
| return val | ||
| } | ||
|
|
||
| func (v visitor) Visit(n ast.Node) ast.Visitor { | ||
| if n == nil { | ||
| return nil | ||
| } | ||
|
|
||
| switch d := n.(type) { | ||
| case *ast.InterfaceType: | ||
| for _, m := range d.Methods.List { | ||
| methodName := m.Names[0].Name | ||
|
|
||
| if converterMethodPattern.MatchString(methodName) { | ||
| entityName := v.extractGroup(converterMethodPattern, methodName) | ||
| entity := v.getEntity(entityName) | ||
| // there should be just one doc comment matching ignoreDirectivePattern | ||
| for _, c := range m.Doc.List { | ||
| if ignoreDirectivePattern.MatchString(c.Text) { | ||
| entity.notEditableFields = v.extractGroup(ignoreDirectivePattern, c.Text) | ||
| } | ||
| } | ||
| } else if overrideNotEditableMethodPattern.MatchString(methodName) { | ||
| entityName := v.extractGroup(overrideNotEditableMethodPattern, methodName) | ||
| entity := v.getEntity(entityName) | ||
| // there should be just one doc comment matching ignoreDirectivePattern | ||
| for _, c := range m.Doc.List { | ||
| if ignoreDirectivePattern.MatchString(c.Text) { | ||
| entity.ignoredFields = v.extractGroup(ignoreDirectivePattern, c.Text) | ||
| } | ||
| } | ||
| } | ||
| } | ||
| v.checkEntities() | ||
| } | ||
| return v | ||
| } | ||
|
|
||
| // checkEntities check if all editable fields are listed in the goverter ignore directive of OverrideNotEditableFor | ||
| func (v *visitor) checkEntities() { | ||
| errorMsgs := map[string][]string{} | ||
| for k, v := range v.entities { | ||
| msgs := checkEntity(v) | ||
| if len(msgs) > 0 { | ||
| errorMsgs[k] = msgs | ||
| } | ||
| } | ||
|
|
||
| if len(errorMsgs) > 0 { | ||
| missingFieldsMsg := "" | ||
| for k, fields := range errorMsgs { | ||
| missingFieldsMsg += fmt.Sprintf("%s: %v\n", k, fields) | ||
| } | ||
| v.t.Errorf("missing fields to be ignored for OverrideNotEditableFor* goverter methods:\n%v", missingFieldsMsg) | ||
| } | ||
| } | ||
|
|
||
| // checkEntity check if there are missing fields to be ignored in the override method | ||
| func checkEntity(entity *oapiEntity) []string { | ||
| res := []string{} | ||
| objType := reflect.TypeOf(entity.obj) | ||
| for i := 0; i < objType.NumField(); i++ { | ||
| field := objType.Field(i) | ||
| if !strings.Contains(entity.notEditableFields, field.Name) && !strings.Contains(entity.ignoredFields, field.Name) { | ||
| // check if the not editable field (first check) is not present in the ignored fields (second check) | ||
| // if this condition is true, we missed that field in the Override method ignore list | ||
| res = append(res, field.Name) | ||
| } | ||
| } | ||
| return res | ||
| } | ||
|
|
||
| // test | ||
|
|
||
| var converterMethodPattern *regexp.Regexp = regexp.MustCompile(`Convert(?P<entity>\w+)Update`) | ||
| var overrideNotEditableMethodPattern *regexp.Regexp = regexp.MustCompile(`OverrideNotEditableFor(?P<entity>\w+)`) | ||
| var ignoreDirectivePattern *regexp.Regexp = regexp.MustCompile(`// goverter:ignore (?P<fields>.+)`) | ||
|
|
||
| func TestOverrideNotEditableFields(t *testing.T) { | ||
| _ = setup(t) | ||
|
|
||
| fset := token.NewFileSet() // positions are relative to fset | ||
| wd, err := os.Getwd() | ||
| if err != nil { | ||
| t.Errorf("error getting current working directory") | ||
| } | ||
| filePath := fmt.Sprintf("%s/opeanpi_converter.go", wd) | ||
| f, _ := parser.ParseFile(fset, filePath, nil, parser.ParseComments) | ||
|
|
||
| v := newVisitor(t, f) | ||
| ast.Walk(v, f) | ||
| } | ||
|
|
||
| type oapiEntity struct { | ||
| obj any | ||
| notEditableFields string | ||
| ignoredFields string | ||
| } |
Oops, something went wrong.