diff --git a/config/assignment.go b/config/assignment.go index f05c956..4b6f94b 100644 --- a/config/assignment.go +++ b/config/assignment.go @@ -27,6 +27,10 @@ func GetAssignmentConfig(course, assignment string, onlyForStudentsOrGroups ...s Msg("configuration for assignment not found") } + // Resolve `extends` inheritance before reading any fields so the rest of + // the config loading sees the merged, effective configuration. + resolveAssignmentInheritance(course, assignment) + assignmentKey := course + "." + assignment per := per(assignmentKey) diff --git a/config/inheritance.go b/config/inheritance.go new file mode 100644 index 0000000..8f92100 --- /dev/null +++ b/config/inheritance.go @@ -0,0 +1,119 @@ +package config + +import ( + "fmt" + "strings" + + "github.com/rs/zerolog/log" + "github.com/spf13/viper" +) + +// inheritKey is the assignment config field used to inherit configuration from +// another assignment within the same course (OOP-style single inheritance). +// +// blatt10: +// extends: blatt09 # inherit everything from blatt09 ... +// assignmentpath: blatt-10 # ... and override only what differs +const inheritKey = "extends" + +// resolveAssignmentInheritance resolves the `extends` chain for the given +// assignment and writes the merged, effective configuration back into viper at +// the assignment key. After this call the rest of the config loading reads the +// inherited values transparently via the usual viper.Get* calls. +// +// Inheritance semantics (child overrides parent): +// - maps (e.g. mergeRequest, startercode, deferredBranches) are deep-merged, +// so a child can override a single nested field while keeping the rest; +// - scalars and slices (e.g. branches, issueNumbers) are replaced wholesale. +// +// Parents may themselves extend other assignments; chains are resolved +// recursively. Cycles and missing parents are fatal. +func resolveAssignmentInheritance(course, assignment string) { + assignmentKey := course + "." + assignment + if !viper.IsSet(assignmentKey + "." + inheritKey) { + return + } + + merged := mergedAssignmentMap(course, assignment, map[string]bool{}) + delete(merged, inheritKey) + viper.Set(assignmentKey, merged) +} + +// mergedAssignmentMap returns the assignment's configuration map with all parent +// configuration (via `extends`) merged in. The child's own values win. +func mergedAssignmentMap(course, assignment string, seen map[string]bool) map[string]any { + if seen[assignment] { + log.Fatal(). + Str("course", course). + Str("assignment", assignment). + Msg("cyclic 'extends' inheritance detected in assignment configuration") + } + seen[assignment] = true + + own := viper.GetStringMap(course + "." + assignment) + + parentRaw, ok := own[inheritKey] + if !ok { + return own + } + + parent, ok := parentRaw.(string) + if !ok || strings.TrimSpace(parent) == "" { + log.Fatal(). + Str("course", course). + Str("assignment", assignment). + Msg("'extends' must be the name of another assignment in the same course") + } + parent = strings.TrimSpace(parent) + + if !viper.IsSet(course + "." + parent) { + log.Fatal(). + Str("course", course). + Str("assignment", assignment). + Str("extends", parent). + Msg("assignment referenced by 'extends' not found") + } + + parentMap := mergedAssignmentMap(course, parent, seen) + + return deepMerge(parentMap, own) +} + +// deepMerge returns a new map with child merged onto parent. Nested maps are +// merged recursively; all other values (scalars, slices) are replaced by the +// child's value. +func deepMerge(parent, child map[string]any) map[string]any { + out := make(map[string]any, len(parent)+len(child)) + for k, v := range parent { + out[k] = v + } + for k, childVal := range child { + if parentVal, ok := out[k]; ok { + parentMap, parentIsMap := asStringMap(parentVal) + childMap, childIsMap := asStringMap(childVal) + if parentIsMap && childIsMap { + out[k] = deepMerge(parentMap, childMap) + continue + } + } + out[k] = childVal + } + return out +} + +// asStringMap normalizes the YAML/viper map representations (map[string]any or +// map[any]any) into map[string]any, reporting whether the value was a map. +func asStringMap(v any) (map[string]any, bool) { + switch m := v.(type) { + case map[string]any: + return m, true + case map[any]any: + out := make(map[string]any, len(m)) + for k, val := range m { + out[fmt.Sprint(k)] = val + } + return out, true + default: + return nil, false + } +} diff --git a/config/inheritance_test.go b/config/inheritance_test.go new file mode 100644 index 0000000..730a9dd --- /dev/null +++ b/config/inheritance_test.go @@ -0,0 +1,214 @@ +package config + +import ( + "testing" + + "github.com/spf13/viper" +) + +// baseAssignment sets up a fully-featured parent assignment ("blatt09") that +// children inherit from. +func baseAssignment(t *testing.T) { + t.Helper() + resetViper(t) + + viper.Set("gitlab.host", "https://gitlab.example.org") + viper.Set("mpd", true) + viper.Set("mpd.coursepath", "mpd") + viper.Set("mpd.semesterpath", "ss26") + viper.Set("mpd.useCoursenameAsPrefix", true) + viper.Set("mpd.students", []string{"alice"}) + + viper.Set("mpd.blatt09", true) + viper.Set("mpd.blatt09.assignmentpath", "blatt-09") + viper.Set("mpd.blatt09.description", "Blatt 9") + viper.Set("mpd.blatt09.per", "student") + viper.Set("mpd.blatt09.mergeRequest", map[string]any{ + "mergeMethod": "semi_linear", + "squashOption": "never", + "pipeline": true, + }) + viper.Set("mpd.blatt09.startercode", map[string]any{ + "url": "git@gitlab.lrz.de:mpd/labs/blatt-09.git", + "fromBranch": "startercode", + "template": true, + }) + viper.Set("mpd.blatt09.branches", []map[string]any{ + {"name": "main", "mergeOnly": true}, + }) + viper.Set("mpd.blatt09.deferredBranches", map[string]any{ + "devcontainer": map[string]any{ + "url": "git@gitlab.lrz.de:mpd/devcontainer.git", + "fromBranch": "main", + "toBranch": "devcontainer", + }, + "solution": map[string]any{ + "fromBranch": "solution", + "orphan": true, + "orphanMessage": "Lösung 9", + }, + }) +} + +func TestInheritance_OverridesAndInherits(t *testing.T) { + baseAssignment(t) + + // blatt10 extends blatt09 and only overrides what differs. + viper.Set("mpd.blatt10", true) + viper.Set("mpd.blatt10.extends", "blatt09") + viper.Set("mpd.blatt10.assignmentpath", "blatt-10") + viper.Set("mpd.blatt10.description", "Blatt 10") + // override only one nested field of startercode + viper.Set("mpd.blatt10.startercode", map[string]any{ + "url": "git@gitlab.lrz.de:mpd/labs/blatt-10.git", + }) + + cfg := GetAssignmentConfig("mpd", "blatt10") + + // overridden scalars + if cfg.Path != "mpd/ss26/blatt-10" { + t.Fatalf("Path = %q, want mpd/ss26/blatt-10", cfg.Path) + } + if cfg.Description != "Blatt 10" { + t.Fatalf("Description = %q, want Blatt 10", cfg.Description) + } + + // inherited mergeRequest + if cfg.MergeRequest == nil || cfg.MergeRequest.MergeMethod != SemiLinearHistory { + t.Fatalf("MergeRequest = %#v, want inherited semi_linear", cfg.MergeRequest) + } + if !cfg.MergeRequest.PipelineMustSucceed { + t.Fatal("PipelineMustSucceed should be inherited as true") + } + + // startercode: url overridden, rest deep-merged from parent + if cfg.Startercode == nil { + t.Fatal("Startercode should not be nil") + } + if cfg.Startercode.URL != "git@gitlab.lrz.de:mpd/labs/blatt-10.git" { + t.Fatalf("Startercode.URL = %q, want overridden blatt-10 url", cfg.Startercode.URL) + } + if cfg.Startercode.FromBranch != "startercode" { + t.Fatalf("Startercode.FromBranch = %q, want inherited 'startercode'", cfg.Startercode.FromBranch) + } + if !cfg.Startercode.Template { + t.Fatal("Startercode.Template should be inherited as true") + } + + // deferredBranches inherited + if len(cfg.DeferredBranches) != 2 { + t.Fatalf("DeferredBranches len = %d, want 2", len(cfg.DeferredBranches)) + } + if db, ok := cfg.DeferredBranches["solution"]; !ok || db.OrphanMessage != "Lösung 9" { + t.Fatalf("inherited solution deferred branch = %#v", db) + } + + // branches inherited + if len(cfg.Branches) != 1 || cfg.Branches[0].Name != "main" || !cfg.Branches[0].MergeOnly { + t.Fatalf("Branches = %#v, want inherited [main mergeOnly]", cfg.Branches) + } +} + +func TestInheritance_DeepMergeNestedDeferredBranch(t *testing.T) { + baseAssignment(t) + + viper.Set("mpd.blatt10", true) + viper.Set("mpd.blatt10.extends", "blatt09") + viper.Set("mpd.blatt10.assignmentpath", "blatt-10") + // override only the orphanMessage of the inherited "solution" deferred branch + viper.Set("mpd.blatt10.deferredBranches", map[string]any{ + "solution": map[string]any{ + "orphanMessage": "Lösung 10", + }, + }) + + cfg := GetAssignmentConfig("mpd", "blatt10") + + if len(cfg.DeferredBranches) != 2 { + t.Fatalf("DeferredBranches len = %d, want 2 (devcontainer kept)", len(cfg.DeferredBranches)) + } + sol, ok := cfg.DeferredBranches["solution"] + if !ok { + t.Fatal("solution deferred branch missing") + } + if sol.OrphanMessage != "Lösung 10" { + t.Fatalf("solution.OrphanMessage = %q, want overridden 'Lösung 10'", sol.OrphanMessage) + } + if sol.FromBranch != "solution" { + t.Fatalf("solution.FromBranch = %q, want inherited 'solution'", sol.FromBranch) + } + if _, ok := cfg.DeferredBranches["devcontainer"]; !ok { + t.Fatal("devcontainer deferred branch should be inherited") + } +} + +func TestInheritance_MultiLevelChain(t *testing.T) { + baseAssignment(t) + + viper.Set("mpd.blatt10", true) + viper.Set("mpd.blatt10.extends", "blatt09") + viper.Set("mpd.blatt10.assignmentpath", "blatt-10") + + viper.Set("mpd.blatt11", true) + viper.Set("mpd.blatt11.extends", "blatt10") + viper.Set("mpd.blatt11.assignmentpath", "blatt-11") + + cfg := GetAssignmentConfig("mpd", "blatt11") + + if cfg.Path != "mpd/ss26/blatt-11" { + t.Fatalf("Path = %q, want mpd/ss26/blatt-11", cfg.Path) + } + // inherited transitively from blatt09 via blatt10 + if cfg.MergeRequest == nil || cfg.MergeRequest.MergeMethod != SemiLinearHistory { + t.Fatalf("MergeRequest = %#v, want transitively inherited semi_linear", cfg.MergeRequest) + } + if cfg.Startercode == nil || cfg.Startercode.FromBranch != "startercode" { + t.Fatalf("Startercode = %#v, want transitively inherited", cfg.Startercode) + } +} + +func TestInheritance_NoExtendsIsUnaffected(t *testing.T) { + baseAssignment(t) + + cfg := GetAssignmentConfig("mpd", "blatt09") + + if cfg.Path != "mpd/ss26/blatt-09" { + t.Fatalf("Path = %q", cfg.Path) + } + if cfg.Startercode == nil || cfg.Startercode.URL != "git@gitlab.lrz.de:mpd/labs/blatt-09.git" { + t.Fatalf("Startercode = %#v", cfg.Startercode) + } +} + +func TestDeepMerge(t *testing.T) { + parent := map[string]any{ + "a": 1, + "b": map[string]any{"x": 1, "y": 2}, + "c": []string{"keep"}, + } + child := map[string]any{ + "b": map[string]any{"y": 20, "z": 30}, + "c": []string{"replaced"}, + "d": 4, + } + + got := deepMerge(parent, child) + + if got["a"] != 1 { + t.Fatalf("a = %v, want inherited 1", got["a"]) + } + if got["d"] != 4 { + t.Fatalf("d = %v, want 4", got["d"]) + } + bm, ok := asStringMap(got["b"]) + if !ok { + t.Fatalf("b is not a map: %#v", got["b"]) + } + if bm["x"] != 1 || bm["y"] != 20 || bm["z"] != 30 { + t.Fatalf("merged b = %#v, want {x:1 y:20 z:30}", bm) + } + cs, ok := got["c"].([]string) + if !ok || len(cs) != 1 || cs[0] != "replaced" { + t.Fatalf("c = %#v, want slice replaced wholesale", got["c"]) + } +} diff --git a/docs/configuration.md b/docs/configuration.md index 6c25154..88c704f 100644 --- a/docs/configuration.md +++ b/docs/configuration.md @@ -152,6 +152,64 @@ Patterns are **regular expressions**, so: | `containerRegistry` | Enable container registry | `false` | Auto-enabled if `release.dockerImages` set | | `students` | Override/add assignment-specific students | — | Merged with course-level | | `groups` | Override/add assignment-specific groups | — | Merged with course-level | +| `extends` | Inherit configuration from another assignment | — | See [Inheritance](#assignment-inheritance) | + +### Assignment inheritance + +Assignments are often nearly identical from one to the next. Instead of +copy-pasting the whole block, an assignment can inherit from another assignment +in the same course with `extends` and override only what differs — much like +subclassing in OOP. + +```yaml +blatt09: + assignmentpath: blatt-09 + description: Blatt 9 + per: student + mergeRequest: + mergeMethod: semi_linear + squashOption: never + pipeline: true + startercode: + url: git@gitlab.lrz.de:mpd/labs/blatt-09.git + fromBranch: startercode + template: true + branches: + - name: main + mergeOnly: true + deferredBranches: + devcontainer: + url: git@gitlab.lrz.de:mpd/devcontainer.git + fromBranch: main + toBranch: devcontainer + solution: + fromBranch: solution + orphanMessage: "Lösungsvorschlag [skip ci]" + +blatt10: + extends: blatt09 # inherit everything from blatt09 ... + assignmentpath: blatt-10 # ... and override only what differs + description: Blatt 10 + startercode: + url: git@gitlab.lrz.de:mpd/labs/blatt-10.git # other startercode fields are inherited +``` + +Merge semantics (the child always wins): + +- **Maps** (`mergeRequest`, `startercode`, `deferredBranches`, …) are + **deep-merged**. In the example above, `blatt10` only sets `startercode.url`; + `fromBranch` and `template` are inherited from `blatt09`. The same applies per + nested deferred branch — overriding `solution.orphanMessage` keeps the rest of + `solution` and the whole `devcontainer` entry. +- **Scalars and lists** (`description`, `branches`, `issues.issueNumbers`, …) + are **replaced wholesale**. To change one entry of a list you must restate the + full list. + +Notes: + +- `extends` refers to another assignment **in the same course** by its key name. +- Inheritance chains are allowed (`blatt11` → `blatt10` → `blatt09`); values are + resolved transitively. Cycles and missing parents are reported as errors. ### Access level values