From e41e8f5129fe55267d82f4325bc962d558e70410 Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Sat, 23 Dec 2023 15:28:23 -0800 Subject: [PATCH 01/16] Added a utility for clearing a database. --- cmd/clear-db/main.go | 33 +++++++++++++++++++++++++++++++++ 1 file changed, 33 insertions(+) create mode 100644 cmd/clear-db/main.go diff --git a/cmd/clear-db/main.go b/cmd/clear-db/main.go new file mode 100644 index 00000000..797c250b --- /dev/null +++ b/cmd/clear-db/main.go @@ -0,0 +1,33 @@ +package main + +import ( + "github.com/alecthomas/kong" + "github.com/rs/zerolog/log" + + "github.com/eriq-augustine/autograder/config" + "github.com/eriq-augustine/autograder/db" +) + +var args struct { + config.ConfigArgs +} + +func main() { + kong.Parse(&args, + kong.Description("Clear the current database. Be careful."), + ); + + err := config.HandleConfigArgs(args.ConfigArgs); + if (err != nil) { + log.Fatal().Err(err).Msg("Could not load config options."); + } + + db.MustOpen(); + defer db.MustClose(); + + err = db.Clear(); + if (err != nil) { + log.Fatal().Err(err).Msg("Failed to clear database."); + } + +} From 96344107c7ed5700e27a51ed9b8e87c27215cf9c Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Sat, 23 Dec 2023 15:29:12 -0800 Subject: [PATCH 02/16] Changed the base image to point to the eriqaugustine repo on dockerhub. --- _tests/COURSE101/HW0/assignment.json | 2 +- _tests/course-languages/cpp/assignment.json | 2 +- _tests/course-languages/java/assignment.json | 2 +- _tests/course-with-zero-limit/HW0/assignment.json | 2 +- docker/model.go | 2 +- grader/nodocker.go | 3 ++- task/backup_test.go | 2 +- 7 files changed, 8 insertions(+), 7 deletions(-) diff --git a/_tests/COURSE101/HW0/assignment.json b/_tests/COURSE101/HW0/assignment.json index 94ba7033..925db83e 100644 --- a/_tests/COURSE101/HW0/assignment.json +++ b/_tests/COURSE101/HW0/assignment.json @@ -4,5 +4,5 @@ "static-files": [ "grader.py" ], - "image": "autograder.python" + "image": "eriqaugustine/autograder.python" } diff --git a/_tests/course-languages/cpp/assignment.json b/_tests/course-languages/cpp/assignment.json index ba761b18..b612e679 100644 --- a/_tests/course-languages/cpp/assignment.json +++ b/_tests/course-languages/cpp/assignment.json @@ -10,7 +10,7 @@ "path": "https://github.com/nlohmann/json/releases/download/v3.11.3/json.hpp" } ], - "image": "autograder.base", + "image": "eriqaugustine/autograder.base", "invocation": ["bash", "./grader.sh"], "post-submission-files-ops": [ ["cp", "input/assignment.cpp", "work/assignment.cpp"] diff --git a/_tests/course-languages/java/assignment.json b/_tests/course-languages/java/assignment.json index 9d580b15..53492305 100644 --- a/_tests/course-languages/java/assignment.json +++ b/_tests/course-languages/java/assignment.json @@ -5,7 +5,7 @@ "grader.sh", "Grader.java" ], - "image": "autograder.base", + "image": "eriqaugustine/autograder.base", "invocation": ["bash", "./grader.sh"], "post-static-docker-commands": [ "RUN apt-get update", diff --git a/_tests/course-with-zero-limit/HW0/assignment.json b/_tests/course-with-zero-limit/HW0/assignment.json index 9a22c660..cbc18a53 100644 --- a/_tests/course-with-zero-limit/HW0/assignment.json +++ b/_tests/course-with-zero-limit/HW0/assignment.json @@ -1,5 +1,5 @@ { "id": "hw0", "name": "Homework 0", - "image": "autograder.python" + "image": "eriqaugustine/autograder.python" } diff --git a/docker/model.go b/docker/model.go index 0c6ab5aa..5a8cdf5b 100644 --- a/docker/model.go +++ b/docker/model.go @@ -7,7 +7,7 @@ import ( ) const ( - DEFAULT_IMAGE = "autograder.base" + DEFAULT_IMAGE = "eriqaugustine/autograder.base" ) type ImageInfo struct { diff --git a/grader/nodocker.go b/grader/nodocker.go index 93b6de31..2314b14e 100644 --- a/grader/nodocker.go +++ b/grader/nodocker.go @@ -17,6 +17,7 @@ import ( const PYTHON_AUTOGRADER_INVOCATION = "python3 -m autograder.cli.grading.grade-dir --grader --dir --outpath " const PYTHON_GRADER_FILENAME = "grader.py" +const PYTHON_DOCKER_IMAGE_BASENAME = "autograder.python"; func runNoDockerGrader(assignment *model.Assignment, submissionPath string, options GradeOptions, fullSubmissionID string) ( *model.GradingInfo, map[string][]byte, string, string, error) { @@ -110,7 +111,7 @@ func getAssignmentInvocation(assignment *model.Assignment, } // Special case for the python grader (we know how to invoke that). - if ((rawCommand == nil) && (imageInfo.Image == "autograder.python")) { + if ((rawCommand == nil) && (strings.Contains(imageInfo.Image, PYTHON_DOCKER_IMAGE_BASENAME))) { rawCommand = strings.Split(PYTHON_AUTOGRADER_INVOCATION, " "); } diff --git a/task/backup_test.go b/task/backup_test.go index e6ccc25b..4b9abd6c 100644 --- a/task/backup_test.go +++ b/task/backup_test.go @@ -11,7 +11,7 @@ import ( "github.com/eriq-augustine/autograder/util" ) -const EXPECTED_MD5 = "fe81323768a15b03cdad218d35aff976"; +const EXPECTED_MD5 = "1b441fc47a0efe46b3437973c10097c1"; func TestBackupTempDir(test *testing.T) { tempDir, err := util.MkDirTemp("autograder-test-task-backup-"); From 3bb406ef11e64a1c8cb24b596799b6917dcb8c25 Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Sun, 24 Dec 2023 00:07:09 -0800 Subject: [PATCH 03/16] Fixed up some text in the create users file help text. --- cmd/create-users-file/main.go | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/cmd/create-users-file/main.go b/cmd/create-users-file/main.go index 54c3a7ab..22c8f1e7 100644 --- a/cmd/create-users-file/main.go +++ b/cmd/create-users-file/main.go @@ -16,15 +16,15 @@ var args struct { config.ConfigArgs Path string `help:"Path to the new users JSON file." arg:"" type:"path"` - Email string `help:"Email for the user." arg:"" required:""` - Name string `help:"Name for the user." short:"n"` - Role string `help:"Role for the user. Defaults to student." short:"r" default:"student"` - Pass string `help:"Password for the user. Defaults to a random string (will be output)." short:"p"` + Email string `help:"Email for the new user." arg:"" required:""` + Name string `help:"Name for the new user." short:"n"` + Role string `help:"Role for the new user. Defaults to student." short:"r" default:"student"` + Pass string `help:"Password for the new user. Defaults to a random string (will be output)." short:"p"` } func main() { kong.Parse(&args, - kong.Description("Create a users file, which can be used to see users in a course." + + kong.Description("Create a users file, which can be used to seed users in a course." + " A full user must be specified, which will be added to the file." + " If an existing users file is specified, then the user will be added to the file."), ); From 6a98a080ebd077a7ce4ed789ba70ab0ac0f516f5 Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Sun, 24 Dec 2023 22:32:33 -0800 Subject: [PATCH 04/16] Changed the configuration loading procedure. Config will not be loaded in the following order: WORK_DIR, env variables, current working directory, --config-path, and then --config. --- README.md | 20 +++-- config/args.go | 66 +++++++++++++- config/args_test.go | 205 ++++++++++++++++++++++++++++++++++++++++++++ config/config.go | 23 ++--- config/options.go | 8 +- 5 files changed, 295 insertions(+), 27 deletions(-) create mode 100644 config/args_test.go diff --git a/README.md b/README.md index 7c9039f1..ea4f3b7f 100644 --- a/README.md +++ b/README.md @@ -53,7 +53,7 @@ go run cmd/version/main.go ## Configuration This project uses configuration options to set the behavior of its executables. -All executables use the same configuration infrastructure +All executables that use autograder resources use the same configuration infrastructure and can therefore be configured the same way and with the same options. To see all the available options, @@ -63,7 +63,7 @@ use the `cmd/list-options` executable. ./bin/list-options ``` -Options can be set on the command line using the `-c`/`--config` flag. +Options can be set on the command-line using the `-c`/`--config` flag. For example: ``` ./bin/logs-example --config log.level=debug @@ -84,14 +84,16 @@ and defaults to `autograder`. ### Loading Options -When an autograder executable is run, -it will automatically look for config files in two locations: - - `/config/config.json` - - `/config/secrets.json` +Configurations will be loaded in the following order (later options override earlier ones): + 0. The command-line options are checked for `BASE_DIR`. + 1. Load options from environmental variables. + 2. Options are loaded from `WORK_DIR/config` (config.json then secrets.json). + 3. Options are loaded from the current working directory (config.json then secrets.json). + 4. Options are loaded from any files specified with `--config-path` (ordered by appearance). + 5. Options are loaded from the command-line (`--config` / `-c`). -Use these files to set persistent options. - -To load other config (JSON) files, use the `--config-path` flag. +The base directory (`dirs.base`) can ONLY be set via the command-line or environmental variables. +This prevents cycles from the base directory changing and loading new options. ### Key Configuration Options diff --git a/config/args.go b/config/args.go index 86a84c8d..6a38d3db 100644 --- a/config/args.go +++ b/config/args.go @@ -1,7 +1,24 @@ package config; +// These arguments and semantics are used for all command-line utilities that deal directly with autograder resources +// (which is almost all utilities). +// ConfigArgs should be embedded into whatever Kong argument structure you are using. +// HandleConfigArgs() should be called with the parsed args, +// and this will handle initing the entire configuration infrastructure. +// +// Configurations will be loaded in the following order (later options override earlier ones): +// 0) The command-line options are checked for BASE_DIR. +// 1) Load options from environmental variables. +// 2) Options are loaded from WORK_DIR/config (config.json then secrets.json). +// 3) Options are loaded from the current working directory (config.json then secrets.json). +// 4) Options are loaded from any files specified with --config-path (ordered by appearance). +// 5) Options are loaded from the command-line (--config / -c). + import ( "fmt" + "os" + + "github.com/rs/zerolog/log" ) // A Kong-style struct for adding on all the config-related options to a CLI. @@ -14,6 +31,40 @@ type ConfigArgs struct { } func HandleConfigArgs(args ConfigArgs) error { + return HandleConfigArgsFull(args, ""); +} + +func HandleConfigArgsFull(args ConfigArgs, cwd string) error { + defer InitLogging(); + + if (cwd == "") { + cwd = shouldGetCWD(); + } + + // Step 0: Check the command-line options for BASE_DIR. + value, ok := args.Config[BASE_DIR.Key]; + if (ok) { + BASE_DIR.Set(value); + } + + // Step 1: Load options from environmental variables. + LoadEnv(); + + // Step 2: Load options from WORK_DIR. + dir := GetConfigDir(); + err := LoadConfigFromDir(dir); + if (err != nil) { + return fmt.Errorf("Failed to load config from work dir ('%s'): '%w'.", dir, err); + } + + // Step 3: Load options from the current working directory. + dir = cwd; + err = LoadConfigFromDir(dir); + if (err != nil) { + return fmt.Errorf("Failed to load config from work dir ('%s'): '%w'.", dir, err); + } + + // Step 4: Load files from --config-path. for _, path := range args.ConfigPath { err := LoadFile(path); if (err != nil) { @@ -21,10 +72,13 @@ func HandleConfigArgs(args ConfigArgs) error { } } + // Step 5: Load options from the command-line (--config). for key, value := range args.Config { Set(key, value); } + // Set special options. + if (args.Debug) { DEBUG.Set(true); } @@ -44,7 +98,15 @@ func HandleConfigArgs(args ConfigArgs) error { } } - InitLogging(); - return nil; } + +func shouldGetCWD() string { + cwd, err := os.Getwd(); + if (err != nil) { + log.Error().Err(err).Msg("Failed to get working directory."); + return "."; + } + + return cwd; +} diff --git a/config/args_test.go b/config/args_test.go new file mode 100644 index 00000000..33911f3a --- /dev/null +++ b/config/args_test.go @@ -0,0 +1,205 @@ +package config + +import ( + "path/filepath" + "reflect" + "testing" + + "github.com/eriq-augustine/autograder/util" +) + +func TestConfigArgs(test *testing.T) { + defer Reset(); + + tempDir := setupConfigTempDir(test); + defer util.RemoveDirent(tempDir); + + testCases := []struct{baseDir string; cwd string; args ConfigArgs; hasError bool; expected map[string]any}{ + // Empty config. + {filepath.Join(tempDir, "empty"), tempDir, ConfigArgs{}, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "empty"), + }}, + + // Base dirs cannot be set in config files. + {filepath.Join(tempDir, "bad-base-dir"), tempDir, ConfigArgs{}, true, nil}, + + // config and secrets file without overriding. + {filepath.Join(tempDir, "dual-different"), tempDir, ConfigArgs{}, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "dual-different"), + "a": "A", + "b": "B", + }}, + + // config and secrets file without overriding. + {filepath.Join(tempDir, "dual-override"), tempDir, ConfigArgs{}, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "dual-override"), + "a": "secret", + }}, + + // Config on cmd. + {filepath.Join(tempDir, "empty"), tempDir, + ConfigArgs{ + Config: map[string]string{ + "a": "A", + }, + }, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "empty"), + "a": "A", + }}, + + // Load config and path on cmd. + {filepath.Join(tempDir, "empty"), tempDir, + ConfigArgs{ + Config: map[string]string{ + "c": "C", + }, + ConfigPath: []string{ + getConfigPath(tempDir, "dual-different", "config.json"), + }, + }, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "empty"), + "a": "A", + "c": "C", + }}, + + // Load config and path on cmd, with overriding. + {filepath.Join(tempDir, "empty"), tempDir, + ConfigArgs{ + Config: map[string]string{ + "a": "ZZZ", + }, + ConfigPath: []string{ + getConfigPath(tempDir, "dual-different", "config.json"), + }, + }, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "empty"), + "a": "ZZZ", + }}, + + // config file, secrets file, cmd, config path; with override. + {filepath.Join(tempDir, "dual-override"), tempDir, + ConfigArgs{ + Config: map[string]string{ + "a": "ZZZ", + }, + ConfigPath: []string{ + getConfigPath(tempDir, "dual-different", "config.json"), + }, + }, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "dual-override"), + "a": "ZZZ", + }}, + + // cwd config and cwd secrets file without overriding. + {filepath.Join(tempDir, "empty"), getConfigDir(tempDir, "dual-different"), ConfigArgs{}, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "empty"), + "a": "A", + "b": "B", + }}, + + // cwd config and cwd secrets file with overriding. + {filepath.Join(tempDir, "empty"), getConfigDir(tempDir, "dual-override"), ConfigArgs{}, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "empty"), + "a": "secret", + }}, + + // sys config and secrets, cwd config and secrets, cmd, config path; with override. + {filepath.Join(tempDir, "dual-override"), getConfigDir(tempDir, "dual-override"), + ConfigArgs{ + Config: map[string]string{ + "a": "ZZZ", + }, + ConfigPath: []string{ + getConfigPath(tempDir, "dual-different", "config.json"), + }, + }, false, map[string]any{ + "dirs.base": filepath.Join(tempDir, "dual-override"), + "a": "ZZZ", + }}, + }; + + for i, testCase := range testCases { + Reset(); + BASE_DIR.Set(testCase.baseDir); + + err := HandleConfigArgsFull(testCase.args, testCase.cwd); + + if ((err == nil) && (testCase.hasError)) { + test.Errorf("Case %d: Found no error when there should be one.", i); + continue; + } + + if ((err != nil) && (!testCase.hasError)) { + test.Errorf("Case %d: Found error when there should not be one: '%v'.", i, err); + continue; + } + + if (testCase.hasError) { + continue; + } + + if (!reflect.DeepEqual(testCase.expected, configValues)) { + test.Errorf("Case %d: Config values not as expected. Expected: '%v', Actual: '%v'.", i, testCase.expected, configValues); + continue; + } + } +} + +func setupConfigTempDir(test *testing.T) string { + tempDir, err := util.MkDirTemp("autograder-config-"); + if (err != nil) { + test.Fatalf("Failed to create temp dir: '%v'.", err); + } + + contents := []struct{text string; path string}{ + { + text: `{}`, + path: getConfigPath(tempDir, "empty", "config.json"), + }, + + { + text: `{"dirs.base": "/dev/null"}`, + path: getConfigPath(tempDir, "bad-base-dir", "config.json"), + }, + + { + text: `{"a": "A"}`, + path: getConfigPath(tempDir, "dual-different", "config.json"), + }, + { + text: `{"b": "B"}`, + path: getConfigPath(tempDir, "dual-different", "secrets.json"), + }, + + { + text: `{"a": "config"}`, + path: getConfigPath(tempDir, "dual-override", "config.json"), + }, + { + text: `{"a": "secret"}`, + path: getConfigPath(tempDir, "dual-override", "secrets.json"), + }, + }; + + for _, content := range contents { + err = util.MkDir(filepath.Dir(content.path)); + if (err != nil) { + test.Fatalf("Failed to make temp subdir: '%v'.", err); + } + + err = util.WriteFile(content.text, content.path); + if (err != nil) { + test.Fatalf("Failed to write contents to '%s': '%v'.", content.path, err); + } + } + + return tempDir; +} + +func getConfigDir(tempDir string, basename string) string { + return filepath.Join(tempDir, basename, NAME.Get(), CONFIG_DIRNAME); +} + +func getConfigPath(tempDir string, basename string, filename string) string { + return filepath.Join(getConfigDir(tempDir, basename), filename); +} diff --git a/config/config.go b/config/config.go index 1159854f..d51b668f 100644 --- a/config/config.go +++ b/config/config.go @@ -1,8 +1,5 @@ package config; -// For the defaulted getters, the defualt will be returned on ANY error -// (even if the key exists, but is of the wrong type). - import ( "encoding/json" "fmt" @@ -33,12 +30,6 @@ func ToJSON() (string, error) { return util.ToJSONIndent(configValues); } -func init() { - LoadLoacalConfig(); - LoadEnv(); - InitLogging(); -} - // A mode intended for running unit tests. func MustEnableUnitTestingMode() { err := EnableUnitTestingMode(); @@ -82,8 +73,8 @@ func EnableTestingMode() { InitLogging(); } -func LoadLoacalConfig() error { - path := filepath.Join(GetConfigDir(), CONFIG_FILENAME); +func LoadConfigFromDir(dir string) error { + path := filepath.Join(dir, CONFIG_FILENAME); if (util.PathExists(path)) { err := LoadFile(path); if (err != nil) { @@ -91,7 +82,7 @@ func LoadLoacalConfig() error { } } - path = filepath.Join(GetConfigDir(), SECRETS_FILENAME); + path = filepath.Join(dir, SECRETS_FILENAME); if (util.PathExists(path)) { err := LoadFile(path); if (err != nil) { @@ -112,7 +103,7 @@ func LoadFile(path string) error { err = LoadReader(file); if (err != nil) { - return fmt.Errorf("Unable to decode config file (%s): %w.", path, err); + return fmt.Errorf("Unable to get config from file (%s): %w.", path, err); } return nil; @@ -122,7 +113,7 @@ func LoadFile(path string) error { func LoadString(text string) error { err := LoadReader(strings.NewReader(text)); if (err != nil) { - return fmt.Errorf("Unable to decode config from string: %w.", err); + return fmt.Errorf("Unable to get config from string: %w.", err); } return nil; @@ -143,6 +134,10 @@ func LoadReader(reader io.Reader) error { } for key, value := range fileOptions { + if (key == BASE_DIR.Key) { + return fmt.Errorf("Cannot set key '%s' in config files, use the command-line.", key); + } + // encoding/json uses float64 as its default numeric type. // Check if it is actually an integer. floatValue, ok := value.(float64); diff --git a/config/options.go b/config/options.go index c04e5767..000bc48e 100644 --- a/config/options.go +++ b/config/options.go @@ -2,10 +2,14 @@ package config var ( // Base - NAME = MustNewStringOption("instance.name", "autograder", "A name to identify this autograder instance. Should only contain alphanumerics and underscores."); + NAME = MustNewStringOption("instance.name", "autograder", + "A name to identify this autograder instance." + + " Should only contain alphanumerics and underscores."); // Directories - BASE_DIR = MustNewStringOption("dirs.base", GetDefaultBaseDir(), "The base dir for autograder to store data."); + BASE_DIR = MustNewStringOption("dirs.base", GetDefaultBaseDir(), + "The base dir for autograder to store data." + + " SHOULD NOT be set in config files (to prevent cycles), only on the command-line."); // Debug / Testing DEBUG = MustNewBoolOption("debug", false, "Enable general debugging."); From 173d5dfaa9507c79d6dc6fe5ce9445529feeb7e5 Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Sun, 24 Dec 2023 22:44:54 -0800 Subject: [PATCH 05/16] Config arg test now ignored env variables. --- config/args.go | 8 +++++--- config/args_test.go | 2 +- 2 files changed, 6 insertions(+), 4 deletions(-) diff --git a/config/args.go b/config/args.go index 6a38d3db..61a72c9f 100644 --- a/config/args.go +++ b/config/args.go @@ -31,10 +31,10 @@ type ConfigArgs struct { } func HandleConfigArgs(args ConfigArgs) error { - return HandleConfigArgsFull(args, ""); + return HandleConfigArgsFull(args, "", false); } -func HandleConfigArgsFull(args ConfigArgs, cwd string) error { +func HandleConfigArgsFull(args ConfigArgs, cwd string, skipEnv bool) error { defer InitLogging(); if (cwd == "") { @@ -48,7 +48,9 @@ func HandleConfigArgsFull(args ConfigArgs, cwd string) error { } // Step 1: Load options from environmental variables. - LoadEnv(); + if (!skipEnv) { + LoadEnv(); + } // Step 2: Load options from WORK_DIR. dir := GetConfigDir(); diff --git a/config/args_test.go b/config/args_test.go index 33911f3a..c83ed80e 100644 --- a/config/args_test.go +++ b/config/args_test.go @@ -122,7 +122,7 @@ func TestConfigArgs(test *testing.T) { Reset(); BASE_DIR.Set(testCase.baseDir); - err := HandleConfigArgsFull(testCase.args, testCase.cwd); + err := HandleConfigArgsFull(testCase.args, testCase.cwd, true); if ((err == nil) && (testCase.hasError)) { test.Errorf("Case %d: Found no error when there should be one.", i); From e03b78b7f43d04e3a1df6cb24ea1a5b6d9341bad Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Mon, 25 Dec 2023 18:33:35 -0800 Subject: [PATCH 06/16] Fixed a bug where the opposite condition was checked for a scoring task validation. --- model/tasks/scoring.go | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/model/tasks/scoring.go b/model/tasks/scoring.go index 3e01f5a3..c86d9d5c 100644 --- a/model/tasks/scoring.go +++ b/model/tasks/scoring.go @@ -20,7 +20,7 @@ func (this *ScoringUploadTask) Validate(course TaskCourse) error { return err; } - if (course.HasLMSAdapter()) { + if (!course.HasLMSAdapter()) { return fmt.Errorf("Score and Upload task course must have an LMS adapter."); } From dc83c14d04263dc20198824f0ac96ab3e51eaeca Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Tue, 26 Dec 2023 12:19:03 -0800 Subject: [PATCH 07/16] Moved http functions from util to common package and added the ability to store requests/responses. --- api/core/routing_test.go | 3 +- api/core/test.go | 5 +-- common/filespec.go | 2 +- {util => common}/http.go | 55 +++++++++++++++++++++++++++++++- config/options.go | 7 ++-- lms/backend/canvas/assignment.go | 3 +- lms/backend/canvas/comments.go | 4 +-- lms/backend/canvas/scores.go | 5 +-- lms/backend/canvas/users.go | 5 +-- 9 files changed, 74 insertions(+), 15 deletions(-) rename {util => common}/http.go (79%) diff --git a/api/core/routing_test.go b/api/core/routing_test.go index 3be059c5..96c7fdf5 100644 --- a/api/core/routing_test.go +++ b/api/core/routing_test.go @@ -5,6 +5,7 @@ import ( "math" "testing" + "github.com/eriq-augustine/autograder/common" "github.com/eriq-augustine/autograder/util" ) @@ -82,7 +83,7 @@ func TestBadRequestEmptyContent(test *testing.T) { url := serverURL + endpoint; for i, testCase := range testCases { - responseText, err := util.PostNoCheck(url, testCase.form); + responseText, err := common.PostNoCheck(url, testCase.form); if (err != nil) { test.Errorf("Case %d -- POST returned an error: '%v'.", i, err); continue; diff --git a/api/core/test.go b/api/core/test.go index f5e47c25..d4b37a84 100644 --- a/api/core/test.go +++ b/api/core/test.go @@ -5,6 +5,7 @@ import ( "os" "testing" + "github.com/eriq-augustine/autograder/common" "github.com/eriq-augustine/autograder/config" "github.com/eriq-augustine/autograder/db" "github.com/eriq-augustine/autograder/model" @@ -83,9 +84,9 @@ func SendTestAPIRequestFull(test *testing.T, endpoint string, fields map[string] var err error; if (len(paths) == 0) { - responseText, err = util.PostNoCheck(url, form); + responseText, err = common.PostNoCheck(url, form); } else { - responseText, err = util.PostFiles(url, form, paths, false); + responseText, err = common.PostFiles(url, form, paths, false); } if (err != nil) { diff --git a/common/filespec.go b/common/filespec.go index 6073cbc8..7dc7cf41 100644 --- a/common/filespec.go +++ b/common/filespec.go @@ -293,7 +293,7 @@ func (this *FileSpec) downloadURL(destDir string) error { return fmt.Errorf("Failed to make dir for URL FileSpec ('%s'): '%w'.", destPath, err); } - content, err := util.RawGet(this.Path); + content, err := RawGet(this.Path); if (err != nil) { return err; } diff --git a/util/http.go b/common/http.go similarity index 79% rename from util/http.go rename to common/http.go index 751d8fa1..57739716 100644 --- a/util/http.go +++ b/common/http.go @@ -1,4 +1,4 @@ -package util +package common // Unless marked as "raw", all functions here assume that the respone body it textual. @@ -14,8 +14,22 @@ import ( "strings" "github.com/rs/zerolog/log" + + "github.com/eriq-augustine/autograder/config" + "github.com/eriq-augustine/autograder/util" ) +// A representation of an HTTP request. +type SavedHTTPRequest struct { + URL string + Method string + RequestHeaders map[string][]string + + ResponseCode int + ResponseHeaders map[string][]string + ResponseBody string +} + // Get a binary response. func RawGet(uri string) ([]byte, error) { response, err := http.Get(uri); @@ -169,6 +183,22 @@ func doRequest(uri string, request *http.Request, verb string, checkResult bool) } body := string(rawBody); + if (config.STORE_HTTP.Get() != "") { + request := SavedHTTPRequest{ + URL: uri, + Method: request.Method, + RequestHeaders: request.Header, + ResponseCode: response.StatusCode, + ResponseHeaders: response.Header, + ResponseBody: body, + }; + + err = writeRequest(&request); + if (err != nil) { + return "", nil, fmt.Errorf("Failed to save HTTP request '%s': '%w'.", uri, err); + } + } + if (checkResult && (response.StatusCode != http.StatusOK)) { log.Error().Int("code", response.StatusCode).Str("body", body).Any("headers", response.Header).Str("url", uri).Msg("Got a non-OK status."); return "", nil, fmt.Errorf("Got a non-OK status code '%d' from %s on URL '%s': '%w'.", response.StatusCode, verb, uri, err); @@ -176,3 +206,26 @@ func doRequest(uri string, request *http.Request, verb string, checkResult bool) return body, response.Header, nil; } + +func writeRequest(request *SavedHTTPRequest) error { + baseDir := config.STORE_HTTP.Get(); + if (baseDir == "") { + return fmt.Errorf("No base dir provided."); + } + + err := util.MkDir(baseDir); + if (err != nil) { + return fmt.Errorf("Failed to create dir to store HTTP requests '%s': '%w'.", baseDir, err); + } + + filename := fmt.Sprintf("%s.json", util.UUID()); + path := filepath.Join(baseDir, filename); + + err = util.ToJSONFileIndent(request, path); + if (err != nil) { + return fmt.Errorf("Failed to write JSON file '%s': '%w'.", path, err); + } + + log.Debug().Str("uri", request.URL).Str("path", path).Msg("Saved HTTP request."); + return nil; +} diff --git a/config/options.go b/config/options.go index 000bc48e..f73e7dc4 100644 --- a/config/options.go +++ b/config/options.go @@ -13,9 +13,10 @@ var ( // Debug / Testing DEBUG = MustNewBoolOption("debug", false, "Enable general debugging."); - NO_STORE = MustNewBoolOption("grader.nostore", false, "Do not store grading outout (submissions)."); + NO_STORE = MustNewBoolOption("grader.nostore", false, "Do not store grading output (submissions)."); TESTING_MODE = MustNewBoolOption("testing", false, "Assume tests are being run, which may alter some operations."); - NO_AUTH = MustNewBoolOption("api.noauth", false, "Disbale authentication on the API."); + NO_AUTH = MustNewBoolOption("api.noauth", false, "Disable authentication on the API."); + STORE_HTTP = MustNewStringOption("http.store", "", "Store HTTP requests made by the server to the specified directory."); // Logging LOG_LEVEL = MustNewStringOption("log.level", "INFO", "The default logging level."); @@ -34,7 +35,7 @@ var ( // Tasks NO_TASKS = MustNewBoolOption("tasks.disable", false, "Disable all scheduled tasks."); TASK_MIN_REST_SECS = MustNewIntOption("tasks.minrest", 5 * 60, - "The minimum time (in seconds) betwen invocations of the same task." + + "The minimum time (in seconds) between invocations of the same task." + " A task instance that tries to run too quickly will be skipped."); TASK_BACKUP_DIR = MustNewStringOption("tasks.backup.dir", "", "Path to where backups are made. Defaults to inside BASE_DIR."); diff --git a/lms/backend/canvas/assignment.go b/lms/backend/canvas/assignment.go index cfd96314..192be988 100644 --- a/lms/backend/canvas/assignment.go +++ b/lms/backend/canvas/assignment.go @@ -3,6 +3,7 @@ package canvas import ( "fmt" + "github.com/eriq-augustine/autograder/common" "github.com/eriq-augustine/autograder/lms/lmstypes" "github.com/eriq-augustine/autograder/util" ) @@ -17,7 +18,7 @@ func (this *CanvasBackend) FetchAssignment(assignmentID string) (*lmstypes.Assig url := this.BaseURL + apiEndpoint; headers := this.standardHeaders(); - body, _, err := util.GetWithHeaders(url, headers); + body, _, err := common.GetWithHeaders(url, headers); if (err != nil) { return nil, fmt.Errorf("Failed to fetch assignment: '%w'.", err); diff --git a/lms/backend/canvas/comments.go b/lms/backend/canvas/comments.go index ee56e334..48ad8218 100644 --- a/lms/backend/canvas/comments.go +++ b/lms/backend/canvas/comments.go @@ -4,8 +4,8 @@ import ( "fmt" "time" + "github.com/eriq-augustine/autograder/common" "github.com/eriq-augustine/autograder/lms/lmstypes" - "github.com/eriq-augustine/autograder/util" ) func (this *CanvasBackend) UpdateComments(assignmentID string, comments []*lmstypes.SubmissionComment) error { @@ -36,7 +36,7 @@ func (this *CanvasBackend) UpdateComment(assignmentID string, comment *lmstypes. form["comment"] = comment.Text; headers := this.standardHeaders(); - _, _, err := util.PutWithHeaders(url, form, headers); + _, _, err := common.PutWithHeaders(url, form, headers); if (err != nil) { return fmt.Errorf("Failed to update comments: '%w'.", err); diff --git a/lms/backend/canvas/scores.go b/lms/backend/canvas/scores.go index e3e535ed..d263832e 100644 --- a/lms/backend/canvas/scores.go +++ b/lms/backend/canvas/scores.go @@ -4,6 +4,7 @@ import ( "fmt" "time" + "github.com/eriq-augustine/autograder/common" "github.com/eriq-augustine/autograder/lms/lmstypes" "github.com/eriq-augustine/autograder/util" ) @@ -22,7 +23,7 @@ func (this *CanvasBackend) FetchAssignmentScores(assignmentID string) ([]*lmstyp scores := make([]*lmstypes.SubmissionScore, 0); for (url != "") { - body, responseHeaders, err := util.GetWithHeaders(url, headers); + body, responseHeaders, err := common.GetWithHeaders(url, headers); if (err != nil) { return nil, fmt.Errorf("Failed to fetch scores."); @@ -95,7 +96,7 @@ func (this *CanvasBackend) updateAssignmentScores(assignmentID string, scores [] } } - _, _, err := util.PostWithHeaders(url, form, headers); + _, _, err := common.PostWithHeaders(url, form, headers); if (err != nil) { return fmt.Errorf("Failed to upload scores: '%w'.", err); } diff --git a/lms/backend/canvas/users.go b/lms/backend/canvas/users.go index bd8006cf..c1578bb8 100644 --- a/lms/backend/canvas/users.go +++ b/lms/backend/canvas/users.go @@ -6,6 +6,7 @@ import ( "github.com/rs/zerolog/log" + "github.com/eriq-augustine/autograder/common" "github.com/eriq-augustine/autograder/lms/lmstypes" "github.com/eriq-augustine/autograder/util" ) @@ -24,7 +25,7 @@ func (this *CanvasBackend) FetchUsers() ([]*lmstypes.User, error) { users := make([]*lmstypes.User, 0); for (url != "") { - body, responseHeaders, err := util.GetWithHeaders(url, headers); + body, responseHeaders, err := common.GetWithHeaders(url, headers); if (err != nil) { return nil, fmt.Errorf("Failed to fetch users: '%w'.", err); @@ -60,7 +61,7 @@ func (this *CanvasBackend) FetchUser(email string) (*lmstypes.User, error) { url := this.BaseURL + apiEndpoint; headers := this.standardHeaders(); - body, _, err := util.GetWithHeaders(url, headers); + body, _, err := common.GetWithHeaders(url, headers); if (err != nil) { return nil, fmt.Errorf("Failed to fetch user '%s': '%w'.", email, err); From 8e130d7fa4012f2058f519d9692d100b60f55232 Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Tue, 26 Dec 2023 12:19:21 -0800 Subject: [PATCH 08/16] Added a test for lms user fetching. --- lms/backend/canvas/main_test.go | 154 ++++++++++++++++++ .../testdata/http/get-user-base-admin.json | 28 ++++ .../testdata/http/get-user-base-owner.json | 28 ++++ .../testdata/http/get-user-base-student.json | 28 ++++ lms/backend/canvas/users_test.go | 53 ++++++ lms/lmsusers/sync_test.go | 2 +- 6 files changed, 292 insertions(+), 1 deletion(-) create mode 100644 lms/backend/canvas/main_test.go create mode 100644 lms/backend/canvas/testdata/http/get-user-base-admin.json create mode 100644 lms/backend/canvas/testdata/http/get-user-base-owner.json create mode 100644 lms/backend/canvas/testdata/http/get-user-base-student.json create mode 100644 lms/backend/canvas/users_test.go diff --git a/lms/backend/canvas/main_test.go b/lms/backend/canvas/main_test.go new file mode 100644 index 00000000..607c3ee6 --- /dev/null +++ b/lms/backend/canvas/main_test.go @@ -0,0 +1,154 @@ +package canvas + +import ( + "embed" + "fmt" + "io/fs" + "net/http" + "net/http/httptest" + "net/url" + "os" + "strings" + "testing" + + "github.com/eriq-augustine/autograder/common" + "github.com/eriq-augustine/autograder/db" + "github.com/eriq-augustine/autograder/util" +) + +// TEST: Anonymoize +const ( + TEST_COURSE_ID = "12345" + TEST_TOKEN = "ABC123" +) + +var server *httptest.Server; +var serverURL string; + +//go:embed testdata/http +var httpDataDir embed.FS; + +var testBackend *CanvasBackend; + +func TestMain(suite *testing.M) { + var err error; + + // Run inside a func so defers will run before os.Exit(). + code := func() int { + db.PrepForTestingMain(); + defer db.CleanupTestingMain(); + + err = startTestServer(); + if (err != nil) { + panic(err); + } + defer stopTestServer(); + + testBackend, err = NewBackend(TEST_COURSE_ID, TEST_TOKEN, serverURL); + if (err != nil) { + panic(err); + } + + return suite.Run(); + }(); + + os.Exit(code); +} + +func startTestServer() error { + if (server != nil) { + return fmt.Errorf("Test server already started."); + } + + requests, err := loadRequests(); + if (err != nil) { + return err; + } + + server = httptest.NewServer(makeHandler(requests)); + serverURL = server.URL; + + return nil; +} + +func makeHandler(requests map[string]*common.SavedHTTPRequest) http.Handler { + return &testCanvasHandler{requests}; +} + +type testCanvasHandler struct { + requests map[string]*common.SavedHTTPRequest +} + +func (this *testCanvasHandler) ServeHTTP(response http.ResponseWriter, request *http.Request) { + key := fmt.Sprintf("%s::%s?%s", request.Method, request.URL.Path, request.URL.RawQuery); + savedRequest := this.requests[key]; + if (savedRequest == nil) { + fmt.Printf("ERROR 404: '%s'.\n", key); + http.NotFound(response, request); + return; + } + + for key, value := range savedRequest.RequestHeaders { + response.Header()[key] = value; + } + + response.WriteHeader(savedRequest.ResponseCode); + _, err := response.Write([]byte(savedRequest.ResponseBody)); + if (err != nil) { + panic(err); + } +} + +func loadRequests() (map[string]*common.SavedHTTPRequest, error) { + requests := make(map[string]*common.SavedHTTPRequest); + + err := fs.WalkDir(httpDataDir, ".", func(path string, info fs.DirEntry, err error) error { + if (err != nil) { + return err; + } + + if (info.IsDir()) { + return nil; + } + + if (!strings.HasSuffix(info.Name(), ".json")) { + return nil; + } + + data, err := httpDataDir.ReadFile(path); + if (err != nil) { + return fmt.Errorf("Failed to read embedded test file '%s': '%w'.", path, err); + } + + var request common.SavedHTTPRequest; + err = util.JSONFromString(string(data), &request); + if (err != nil) { + return fmt.Errorf("Failed to JSON parse test file '%s': '%w'.", path, err); + } + + uri, err := url.Parse(request.URL); + if (err != nil) { + return fmt.Errorf("Failed to parse test URL '%s': '%w'.", request.URL, err); + } + + key := fmt.Sprintf("%s::%s?%s", request.Method, uri.Path, uri.RawQuery); + requests[key] = &request; + + return nil; + }); + + if (err != nil) { + return nil, fmt.Errorf("Failed to walk embeded test dir: '%w'.", err); + } + + return requests, nil; +} + +func stopTestServer() { + if (server != nil) { + server.Close(); + + server = nil; + serverURL = ""; + } +} diff --git a/lms/backend/canvas/testdata/http/get-user-base-admin.json b/lms/backend/canvas/testdata/http/get-user-base-admin.json new file mode 100644 index 00000000..325e5566 --- /dev/null +++ b/lms/backend/canvas/testdata/http/get-user-base-admin.json @@ -0,0 +1,28 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/search_users?include[]=enrollments&search_term=admin%40test.com", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=admin%40test.com&page=1&per_page=10\u003e; rel=\"current\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=admin%40test.com&page=1&per_page=10\u003e; rel=\"first\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=admin%40test.com&page=1&per_page=10\u003e; rel=\"last\"" + ], + "Status": [ + "200 OK" + ], + "Vary": [ + "Accept-Encoding" + ] + }, + "ResponseBody": "[{\"id\":\"00020\",\"name\":\"admin\",\"created_at\":\"2017-03-02T20:52:50-08:00\",\"sortable_name\":\"admin\",\"short_name\":\"admin\",\"sis_user_id\":\"1116667\",\"integration_id\":null,\"root_account\":\"canvas.test.com\",\"login_id\":\"admin@test.com\",\"enrollments\":[{\"id\":\"3116135\",\"user_id\":\"00020\",\"course_id\":\"12345\",\"type\":\"TaEnrollment\",\"created_at\":\"2023-09-25T08:48:00Z\",\"updated_at\":\"2023-09-28T02:41:42Z\",\"associated_user_id\":null,\"start_at\":null,\"end_at\":null,\"course_section_id\":\"153833\",\"root_account_id\":\"1\",\"limit_privileges_to_course_section\":false,\"enrollment_state\":\"active\",\"role\":\"TA - Site Manager\",\"role_id\":\"13\",\"last_activity_at\":\"2023-12-26T02:52:53Z\",\"last_attended_at\":null,\"total_activity_time\":448948,\"sis_account_id\":\"CSE\",\"sis_course_id\":\"2238-124092-1-01\",\"course_integration_id\":null,\"sis_section_id\":\"2238-124092-1-1-CSE-40-01-LEC-12676\",\"section_integration_id\":null,\"sis_user_id\":\"1116667\",\"html_url\":\"https://canvas.test.com/courses/12345/users/00020\"}],\"email\":\"admin@test.com\"}]" +} diff --git a/lms/backend/canvas/testdata/http/get-user-base-owner.json b/lms/backend/canvas/testdata/http/get-user-base-owner.json new file mode 100644 index 00000000..a050941c --- /dev/null +++ b/lms/backend/canvas/testdata/http/get-user-base-owner.json @@ -0,0 +1,28 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/search_users?include[]=enrollments&search_term=owner%40test.com", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=owner%40test.com&page=1&per_page=10\u003e; rel=\"current\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=owner%40test.com&page=1&per_page=10\u003e; rel=\"first\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=owner%40test.com&page=1&per_page=10\u003e; rel=\"last\"" + ], + "Status": [ + "200 OK" + ], + "Vary": [ + "Accept-Encoding" + ] + }, + "ResponseBody": "[{\"id\":\"00010\",\"name\":\"owner\",\"created_at\":\"2017-03-02T20:40:44-08:00\",\"sortable_name\":\"owner\",\"short_name\":\"owner\",\"sis_user_id\":\"1394707\",\"integration_id\":null,\"root_account\":\"canvas.test.com\",\"login_id\":\"owner@test.com\",\"enrollments\":[{\"id\":\"2917828\",\"user_id\":\"00010\",\"course_id\":\"12345\",\"type\":\"TeacherEnrollment\",\"created_at\":\"2023-04-03T17:16:06Z\",\"updated_at\":\"2023-09-28T02:41:42Z\",\"associated_user_id\":null,\"start_at\":null,\"end_at\":null,\"course_section_id\":\"153926\",\"root_account_id\":\"1\",\"limit_privileges_to_course_section\":false,\"enrollment_state\":\"active\",\"role\":\"TeacherEnrollment\",\"role_id\":\"4\",\"last_activity_at\":\"2023-12-19T00:18:07Z\",\"last_attended_at\":null,\"total_activity_time\":368227,\"sis_account_id\":\"CSE\",\"sis_course_id\":\"2238-124092-1-01\",\"course_integration_id\":null,\"sis_section_id\":null,\"section_integration_id\":null,\"sis_user_id\":\"1394707\",\"html_url\":\"https://canvas.test.com/courses/12345/users/00010\"}],\"email\":\"owner@test.com\"}]" +} diff --git a/lms/backend/canvas/testdata/http/get-user-base-student.json b/lms/backend/canvas/testdata/http/get-user-base-student.json new file mode 100644 index 00000000..41619c84 --- /dev/null +++ b/lms/backend/canvas/testdata/http/get-user-base-student.json @@ -0,0 +1,28 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/search_users?include[]=enrollments&search_term=student%40test.com", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=student%40test.com&page=1&per_page=10\u003e; rel=\"current\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=student%40test.com&page=1&per_page=10\u003e; rel=\"first\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=student%40test.com&page=1&per_page=10\u003e; rel=\"last\"" + ], + "Status": [ + "200 OK" + ], + "Vary": [ + "Accept-Encoding" + ] + }, + "ResponseBody": "[{\"id\":\"00040\",\"name\":\"student\",\"created_at\":\"2022-05-09T10:09:29-07:00\",\"sortable_name\":\"student\",\"short_name\":\"student\",\"sis_user_id\":\"1994048\",\"integration_id\":null,\"root_account\":\"canvas.test.com\",\"login_id\":\"student@test.com\",\"enrollments\":[{\"id\":\"2986419\",\"user_id\":\"00040\",\"course_id\":\"12345\",\"type\":\"StudentEnrollment\",\"created_at\":\"2023-05-24T21:23:45Z\",\"updated_at\":\"2023-09-28T02:41:42Z\",\"associated_user_id\":null,\"start_at\":null,\"end_at\":null,\"course_section_id\":\"153833\",\"root_account_id\":\"1\",\"limit_privileges_to_course_section\":false,\"enrollment_state\":\"active\",\"role\":\"StudentEnrollment\",\"role_id\":\"3\",\"last_activity_at\":\"2023-12-17T09:27:39Z\",\"last_attended_at\":null,\"total_activity_time\":46549,\"grades\":{\"html_url\":\"https://canvas.test.com/courses/12345/grades/00040\",\"current_grade\":\"A\",\"current_score\":90.66,\"final_grade\":\"A\",\"final_score\":90.66,\"unposted_current_score\":90.66,\"unposted_current_grade\":\"A\",\"unposted_final_score\":90.66,\"unposted_final_grade\":\"A\"},\"sis_account_id\":\"CSE\",\"sis_course_id\":\"2238-124092-1-01\",\"course_integration_id\":null,\"sis_section_id\":\"2238-124092-1-1-CSE-40-01-LEC-12676\",\"section_integration_id\":null,\"sis_user_id\":\"1994048\",\"html_url\":\"https://canvas.test.com/courses/12345/users/00040\"},{\"id\":\"2991908\",\"user_id\":\"00040\",\"course_id\":\"12345\",\"type\":\"StudentEnrollment\",\"created_at\":\"2023-05-25T03:28:18Z\",\"updated_at\":\"2023-09-28T02:41:42Z\",\"associated_user_id\":null,\"start_at\":null,\"end_at\":null,\"course_section_id\":\"153234\",\"root_account_id\":\"1\",\"limit_privileges_to_course_section\":false,\"enrollment_state\":\"active\",\"role\":\"StudentEnrollment\",\"role_id\":\"3\",\"last_activity_at\":\"2023-12-17T09:27:39Z\",\"last_attended_at\":null,\"total_activity_time\":46549,\"grades\":{\"html_url\":\"https://canvas.test.com/courses/12345/grades/00040\",\"current_grade\":\"A\",\"current_score\":90.66,\"final_grade\":\"A\",\"final_score\":90.66,\"unposted_current_score\":90.66,\"unposted_current_grade\":\"A\",\"unposted_final_score\":90.66,\"unposted_final_grade\":\"A\"},\"sis_account_id\":\"CSE\",\"sis_course_id\":\"2238-124092-1-01\",\"course_integration_id\":null,\"sis_section_id\":\"2238-124092-1-1-CSE-40-01B-DIS-12678\",\"section_integration_id\":null,\"sis_user_id\":\"1994048\",\"html_url\":\"https://canvas.test.com/courses/12345/users/00040\"}],\"email\":\"student@test.com\",\"analytics_url\":\"/courses/12345/analytics/users/00040\"}]" +} diff --git a/lms/backend/canvas/users_test.go b/lms/backend/canvas/users_test.go new file mode 100644 index 00000000..664debcb --- /dev/null +++ b/lms/backend/canvas/users_test.go @@ -0,0 +1,53 @@ +package canvas + +import ( + "testing" + + "github.com/eriq-augustine/autograder/lms/lmstypes" + "github.com/eriq-augustine/autograder/model" +) + +func TestCanvasUserGetBase(test *testing.T) { + testCases := []struct{email string; expected *lmstypes.User}{ + { + "owner@test.com", + &lmstypes.User{ + ID: "00010", + Name: "owner", + Email: "owner@test.com", + Role: model.RoleOwner, + }, + }, + { + "admin@test.com", + &lmstypes.User{ + ID: "00020", + Name: "admin", + Email: "admin@test.com", + Role: model.RoleAdmin, + }, + }, + { + "student@test.com", + &lmstypes.User{ + ID: "00040", + Name: "student", + Email: "student@test.com", + Role: model.RoleStudent, + }, + }, + }; + + for i, testCase := range testCases { + user, err := testBackend.FetchUser(testCase.email); + if (err != nil) { + test.Errorf("Case %d: Failed to fetch user: '%v'.", i, err); + continue; + } + + if (*testCase.expected != *user) { + test.Errorf("Case %d: User not as expected. Expected: '%+v', Actual: '%+v'.", i, testCase.expected, user); + continue; + } + } +} diff --git a/lms/lmsusers/sync_test.go b/lms/lmsusers/sync_test.go index 91f647aa..770267db 100644 --- a/lms/lmsusers/sync_test.go +++ b/lms/lmsusers/sync_test.go @@ -25,7 +25,7 @@ func reset() { lmstest.ClearUsersModifier(); } -func TestCourseSyncLMSUisers(test *testing.T) { +func TestCourseSyncLMSUsers(test *testing.T) { // Leave the db in a good state after the test. defer reset(); From e8e8be8f0b8dcc1c82dccff77e072aa1bade7a43 Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Tue, 26 Dec 2023 22:42:09 -0800 Subject: [PATCH 09/16] Added a test for LMS users fetching. --- lms/backend/canvas/main_test.go | 3 +- .../testdata/http/get-user-base-admin.json | 2 +- .../testdata/http/get-user-base-owner.json | 2 +- .../testdata/http/get-user-base-student.json | 2 +- .../testdata/http/get-users-base-01.json | 25 +++++++++++++ .../testdata/http/get-users-base-02.json | 25 +++++++++++++ .../testdata/http/get-users-base-03.json | 25 +++++++++++++ lms/backend/canvas/users.go | 17 +++++++-- lms/backend/canvas/users_test.go | 35 +++++++++++++++++++ 9 files changed, 129 insertions(+), 7 deletions(-) create mode 100644 lms/backend/canvas/testdata/http/get-users-base-01.json create mode 100644 lms/backend/canvas/testdata/http/get-users-base-02.json create mode 100644 lms/backend/canvas/testdata/http/get-users-base-03.json diff --git a/lms/backend/canvas/main_test.go b/lms/backend/canvas/main_test.go index 607c3ee6..429b3593 100644 --- a/lms/backend/canvas/main_test.go +++ b/lms/backend/canvas/main_test.go @@ -16,7 +16,6 @@ import ( "github.com/eriq-augustine/autograder/util" ) -// TEST: Anonymoize const ( TEST_COURSE_ID = "12345" TEST_TOKEN = "ABC123" @@ -88,7 +87,7 @@ func (this *testCanvasHandler) ServeHTTP(response http.ResponseWriter, request * return; } - for key, value := range savedRequest.RequestHeaders { + for key, value := range savedRequest.ResponseHeaders { response.Header()[key] = value; } diff --git a/lms/backend/canvas/testdata/http/get-user-base-admin.json b/lms/backend/canvas/testdata/http/get-user-base-admin.json index 325e5566..494d8078 100644 --- a/lms/backend/canvas/testdata/http/get-user-base-admin.json +++ b/lms/backend/canvas/testdata/http/get-user-base-admin.json @@ -15,7 +15,7 @@ "application/json; charset=utf-8" ], "Link": [ - "\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=admin%40test.com&page=1&per_page=10\u003e; rel=\"current\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=admin%40test.com&page=1&per_page=10\u003e; rel=\"first\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=admin%40test.com&page=1&per_page=10\u003e; rel=\"last\"" + "; rel=\"current\",; rel=\"first\",; rel=\"last\"" ], "Status": [ "200 OK" diff --git a/lms/backend/canvas/testdata/http/get-user-base-owner.json b/lms/backend/canvas/testdata/http/get-user-base-owner.json index a050941c..c0210b8b 100644 --- a/lms/backend/canvas/testdata/http/get-user-base-owner.json +++ b/lms/backend/canvas/testdata/http/get-user-base-owner.json @@ -15,7 +15,7 @@ "application/json; charset=utf-8" ], "Link": [ - "\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=owner%40test.com&page=1&per_page=10\u003e; rel=\"current\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=owner%40test.com&page=1&per_page=10\u003e; rel=\"first\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=owner%40test.com&page=1&per_page=10\u003e; rel=\"last\"" + "; rel=\"current\",; rel=\"first\",; rel=\"last\"" ], "Status": [ "200 OK" diff --git a/lms/backend/canvas/testdata/http/get-user-base-student.json b/lms/backend/canvas/testdata/http/get-user-base-student.json index 41619c84..e17535e6 100644 --- a/lms/backend/canvas/testdata/http/get-user-base-student.json +++ b/lms/backend/canvas/testdata/http/get-user-base-student.json @@ -15,7 +15,7 @@ "application/json; charset=utf-8" ], "Link": [ - "\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=student%40test.com&page=1&per_page=10\u003e; rel=\"current\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=student%40test.com&page=1&per_page=10\u003e; rel=\"first\",\u003chttps://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&search_term=student%40test.com&page=1&per_page=10\u003e; rel=\"last\"" + "; rel=\"current\",; rel=\"first\",; rel=\"last\"" ], "Status": [ "200 OK" diff --git a/lms/backend/canvas/testdata/http/get-users-base-01.json b/lms/backend/canvas/testdata/http/get-users-base-01.json new file mode 100644 index 00000000..4b2bdd5f --- /dev/null +++ b/lms/backend/canvas/testdata/http/get-users-base-01.json @@ -0,0 +1,25 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/users?include[]=enrollments&per_page=75", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "; rel=\"current\",; rel=\"next\",; rel=\"first\"" + ], + "Status": [ + "200 OK" + ] + }, + "ResponseBody": "[{\"id\":\"00040\",\"name\":\"student\",\"created_at\":\"2022-05-09T10:09:29-07:00\",\"sortable_name\":\"student\",\"short_name\":\"student\",\"sis_user_id\":\"1994048\",\"integration_id\":null,\"root_account\":\"canvas.test.com\",\"login_id\":\"student@test.com\",\"enrollments\":[{\"id\":\"2986419\",\"user_id\":\"00040\",\"course_id\":\"12345\",\"type\":\"StudentEnrollment\",\"created_at\":\"2023-05-24T21:23:45Z\",\"updated_at\":\"2023-09-28T02:41:42Z\",\"associated_user_id\":null,\"start_at\":null,\"end_at\":null,\"course_section_id\":\"153833\",\"root_account_id\":\"1\",\"limit_privileges_to_course_section\":false,\"enrollment_state\":\"active\",\"role\":\"StudentEnrollment\",\"role_id\":\"3\",\"last_activity_at\":\"2023-12-17T09:27:39Z\",\"last_attended_at\":null,\"total_activity_time\":46549,\"grades\":{\"html_url\":\"https://canvas.test.com/courses/12345/grades/00040\",\"current_grade\":\"A\",\"current_score\":90.66,\"final_grade\":\"A\",\"final_score\":90.66,\"unposted_current_score\":90.66,\"unposted_current_grade\":\"A\",\"unposted_final_score\":90.66,\"unposted_final_grade\":\"A\"},\"sis_account_id\":\"CSE\",\"sis_course_id\":\"2238-124092-1-01\",\"course_integration_id\":null,\"sis_section_id\":\"2238-124092-1-1-CSE-40-01-LEC-12676\",\"section_integration_id\":null,\"sis_user_id\":\"1994048\",\"html_url\":\"https://canvas.test.com/courses/12345/users/00040\"},{\"id\":\"2991908\",\"user_id\":\"00040\",\"course_id\":\"12345\",\"type\":\"StudentEnrollment\",\"created_at\":\"2023-05-25T03:28:18Z\",\"updated_at\":\"2023-09-28T02:41:42Z\",\"associated_user_id\":null,\"start_at\":null,\"end_at\":null,\"course_section_id\":\"153234\",\"root_account_id\":\"1\",\"limit_privileges_to_course_section\":false,\"enrollment_state\":\"active\",\"role\":\"StudentEnrollment\",\"role_id\":\"3\",\"last_activity_at\":\"2023-12-17T09:27:39Z\",\"last_attended_at\":null,\"total_activity_time\":46549,\"grades\":{\"html_url\":\"https://canvas.test.com/courses/12345/grades/00040\",\"current_grade\":\"A\",\"current_score\":90.66,\"final_grade\":\"A\",\"final_score\":90.66,\"unposted_current_score\":90.66,\"unposted_current_grade\":\"A\",\"unposted_final_score\":90.66,\"unposted_final_grade\":\"A\"},\"sis_account_id\":\"CSE\",\"sis_course_id\":\"2238-124092-1-01\",\"course_integration_id\":null,\"sis_section_id\":\"2238-124092-1-1-CSE-40-01B-DIS-12678\",\"section_integration_id\":null,\"sis_user_id\":\"1994048\",\"html_url\":\"https://canvas.test.com/courses/12345/users/00040\"}],\"email\":\"student@test.com\",\"analytics_url\":\"/courses/12345/analytics/users/00040\"}]" +} diff --git a/lms/backend/canvas/testdata/http/get-users-base-02.json b/lms/backend/canvas/testdata/http/get-users-base-02.json new file mode 100644 index 00000000..c4b19623 --- /dev/null +++ b/lms/backend/canvas/testdata/http/get-users-base-02.json @@ -0,0 +1,25 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&page=2&per_page=75", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "; rel=\"current\",; rel=\"next\",; rel=\"prev\",; rel=\"first\"" + ], + "Status": [ + "200 OK" + ] + }, + "ResponseBody": "[{\"id\":\"00020\",\"name\":\"admin\",\"created_at\":\"2017-03-02T20:52:50-08:00\",\"sortable_name\":\"admin\",\"short_name\":\"admin\",\"sis_user_id\":\"1116667\",\"integration_id\":null,\"root_account\":\"canvas.test.com\",\"login_id\":\"admin@test.com\",\"enrollments\":[{\"id\":\"3116135\",\"user_id\":\"00020\",\"course_id\":\"12345\",\"type\":\"TaEnrollment\",\"created_at\":\"2023-09-25T08:48:00Z\",\"updated_at\":\"2023-09-28T02:41:42Z\",\"associated_user_id\":null,\"start_at\":null,\"end_at\":null,\"course_section_id\":\"153833\",\"root_account_id\":\"1\",\"limit_privileges_to_course_section\":false,\"enrollment_state\":\"active\",\"role\":\"TA - Site Manager\",\"role_id\":\"13\",\"last_activity_at\":\"2023-12-26T02:52:53Z\",\"last_attended_at\":null,\"total_activity_time\":448948,\"sis_account_id\":\"CSE\",\"sis_course_id\":\"2238-124092-1-01\",\"course_integration_id\":null,\"sis_section_id\":\"2238-124092-1-1-CSE-40-01-LEC-12676\",\"section_integration_id\":null,\"sis_user_id\":\"1116667\",\"html_url\":\"https://canvas.test.com/courses/12345/users/00020\"}],\"email\":\"admin@test.com\"}]" +} diff --git a/lms/backend/canvas/testdata/http/get-users-base-03.json b/lms/backend/canvas/testdata/http/get-users-base-03.json new file mode 100644 index 00000000..7a8fb5ef --- /dev/null +++ b/lms/backend/canvas/testdata/http/get-users-base-03.json @@ -0,0 +1,25 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/users?include%5B%5D=enrollments&page=3&per_page=75", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "; rel=\"current\",; rel=\"prev\",; rel=\"first\",; rel=\"last\"" + ], + "Status": [ + "200 OK" + ] + }, + "ResponseBody": "[{\"id\":\"00010\",\"name\":\"owner\",\"created_at\":\"2017-03-02T20:40:44-08:00\",\"sortable_name\":\"owner\",\"short_name\":\"owner\",\"sis_user_id\":\"1394707\",\"integration_id\":null,\"root_account\":\"canvas.test.com\",\"login_id\":\"owner@test.com\",\"enrollments\":[{\"id\":\"2917828\",\"user_id\":\"00010\",\"course_id\":\"12345\",\"type\":\"TeacherEnrollment\",\"created_at\":\"2023-04-03T17:16:06Z\",\"updated_at\":\"2023-09-28T02:41:42Z\",\"associated_user_id\":null,\"start_at\":null,\"end_at\":null,\"course_section_id\":\"153926\",\"root_account_id\":\"1\",\"limit_privileges_to_course_section\":false,\"enrollment_state\":\"active\",\"role\":\"TeacherEnrollment\",\"role_id\":\"4\",\"last_activity_at\":\"2023-12-19T00:18:07Z\",\"last_attended_at\":null,\"total_activity_time\":368227,\"sis_account_id\":\"CSE\",\"sis_course_id\":\"2238-124092-1-01\",\"course_integration_id\":null,\"sis_section_id\":null,\"section_integration_id\":null,\"sis_user_id\":\"1394707\",\"html_url\":\"https://canvas.test.com/courses/12345/users/00010\"}],\"email\":\"owner@test.com\"}]" +} diff --git a/lms/backend/canvas/users.go b/lms/backend/canvas/users.go index c1578bb8..eddbf615 100644 --- a/lms/backend/canvas/users.go +++ b/lms/backend/canvas/users.go @@ -2,7 +2,7 @@ package canvas import ( "fmt" - "net/url" + neturl "net/url" "github.com/rs/zerolog/log" @@ -12,6 +12,10 @@ import ( ) func (this *CanvasBackend) FetchUsers() ([]*lmstypes.User, error) { + return this.fetchUsers(false); +} + +func (this *CanvasBackend) fetchUsers(rewriteLinks bool) ([]*lmstypes.User, error) { this.getAPILock(); defer this.releaseAPILock(); @@ -25,6 +29,15 @@ func (this *CanvasBackend) FetchUsers() ([]*lmstypes.User, error) { users := make([]*lmstypes.User, 0); for (url != "") { + if (rewriteLinks) { + parsed, err := neturl.Parse(url); + if (err != nil) { + return nil, fmt.Errorf("Failed to parse URL '%s': '%w'.", url, err); + } + + url = fmt.Sprintf("%s%s?%s", this.BaseURL, parsed.Path, parsed.RawQuery); + } + body, responseHeaders, err := common.GetWithHeaders(url, headers); if (err != nil) { @@ -57,7 +70,7 @@ func (this *CanvasBackend) FetchUser(email string) (*lmstypes.User, error) { apiEndpoint := fmt.Sprintf( "/api/v1/courses/%s/search_users?include[]=enrollments&search_term=%s", - this.CourseID, url.QueryEscape(email)); + this.CourseID, neturl.QueryEscape(email)); url := this.BaseURL + apiEndpoint; headers := this.standardHeaders(); diff --git a/lms/backend/canvas/users_test.go b/lms/backend/canvas/users_test.go index 664debcb..f25a606f 100644 --- a/lms/backend/canvas/users_test.go +++ b/lms/backend/canvas/users_test.go @@ -1,10 +1,12 @@ package canvas import ( + "reflect" "testing" "github.com/eriq-augustine/autograder/lms/lmstypes" "github.com/eriq-augustine/autograder/model" + "github.com/eriq-augustine/autograder/util" ) func TestCanvasUserGetBase(test *testing.T) { @@ -51,3 +53,36 @@ func TestCanvasUserGetBase(test *testing.T) { } } } + +func TestCanvasUsersGetBase(test *testing.T) { + expected := []*lmstypes.User{ + &lmstypes.User{ + ID: "00040", + Name: "student", + Email: "student@test.com", + Role: model.RoleStudent, + }, + &lmstypes.User{ + ID: "00020", + Name: "admin", + Email: "admin@test.com", + Role: model.RoleAdmin, + }, + &lmstypes.User{ + ID: "00010", + Name: "owner", + Email: "owner@test.com", + Role: model.RoleOwner, + }, + }; + + users, err := testBackend.fetchUsers(true); + if (err != nil) { + test.Fatalf("Failed to fetch user: '%v'.", err); + } + + if (!reflect.DeepEqual(expected, users)) { + test.Fatalf("Users not as expected. Expected: '%s', Actual: '%s'.", + util.MustToJSONIndent(expected), util.MustToJSONIndent(users)); + } +} From 268274a5b6b7076ee1c8c8709c5b260778fa14cc Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Tue, 26 Dec 2023 23:24:26 -0800 Subject: [PATCH 10/16] Added a test for LMS fetch assignment. --- lms/backend/canvas/assignment_test.go | 45 +++++++++++++++++++ .../testdata/http/fetch-assignment-base.json | 22 +++++++++ ...-admin.json => fetch-user-base-admin.json} | 0 ...-owner.json => fetch-user-base-owner.json} | 0 ...dent.json => fetch-user-base-student.json} | 0 ...-base-01.json => fetch-users-base-01.json} | 0 ...-base-02.json => fetch-users-base-02.json} | 0 ...-base-03.json => fetch-users-base-03.json} | 0 8 files changed, 67 insertions(+) create mode 100644 lms/backend/canvas/assignment_test.go create mode 100644 lms/backend/canvas/testdata/http/fetch-assignment-base.json rename lms/backend/canvas/testdata/http/{get-user-base-admin.json => fetch-user-base-admin.json} (100%) rename lms/backend/canvas/testdata/http/{get-user-base-owner.json => fetch-user-base-owner.json} (100%) rename lms/backend/canvas/testdata/http/{get-user-base-student.json => fetch-user-base-student.json} (100%) rename lms/backend/canvas/testdata/http/{get-users-base-01.json => fetch-users-base-01.json} (100%) rename lms/backend/canvas/testdata/http/{get-users-base-02.json => fetch-users-base-02.json} (100%) rename lms/backend/canvas/testdata/http/{get-users-base-03.json => fetch-users-base-03.json} (100%) diff --git a/lms/backend/canvas/assignment_test.go b/lms/backend/canvas/assignment_test.go new file mode 100644 index 00000000..70760b00 --- /dev/null +++ b/lms/backend/canvas/assignment_test.go @@ -0,0 +1,45 @@ +package canvas + +import ( + "testing" + "time" + + "github.com/eriq-augustine/autograder/lms/lmstypes" + "github.com/eriq-augustine/autograder/util" +) + +const TEST_ASSIGNMENT_ID = "98765"; + +func TestFetchAssignmentBase(test *testing.T) { + expected := lmstypes.Assignment{ + ID: TEST_ASSIGNMENT_ID, + Name: "Assignment 0", + LMSCourseID: "12345", + DueDate: mustParseTime(test, "2023-10-06T06:59:59Z"), + MaxPoints: 100.0, + }; + + assignment, err := testBackend.FetchAssignment(TEST_ASSIGNMENT_ID); + if (err != nil) { + test.Fatalf("Failed tp fetch assignment: '%v'.", err); + } + + // Can't compare directly because of time.Time. + // Use JSON instead. + expectedJSON := util.MustToJSONIndent(expected); + actualJSON := util.MustToJSONIndent(assignment); + + if (expectedJSON != actualJSON) { + test.Fatalf("Assignment not as expected. Expected: '%s', Actual: '%s'.", + expectedJSON, actualJSON); + } +} + +func mustParseTime(test *testing.T, text string) *time.Time { + instance, err := time.Parse(time.RFC3339, text); + if (err != nil) { + test.Fatalf("Failed to parse time '%s': '%v'.", text, err); + } + + return &instance; +} diff --git a/lms/backend/canvas/testdata/http/fetch-assignment-base.json b/lms/backend/canvas/testdata/http/fetch-assignment-base.json new file mode 100644 index 00000000..c334cbe8 --- /dev/null +++ b/lms/backend/canvas/testdata/http/fetch-assignment-base.json @@ -0,0 +1,22 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/assignments/98765", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Status": [ + "200 OK" + ] + }, + "ResponseBody": "{\"id\":\"98765\",\"description\":\"desc\",\"due_at\":\"2023-10-06T06:59:59Z\",\"unlock_at\":null,\"lock_at\":null,\"points_possible\":100.0,\"grading_type\":\"points\",\"assignment_group_id\":\"124050\",\"grading_standard_id\":null,\"created_at\":\"2023-09-16T05:04:23Z\",\"updated_at\":\"2023-10-11T17:56:12Z\",\"peer_reviews\":false,\"automatic_peer_reviews\":false,\"position\":2,\"grade_group_students_individually\":false,\"anonymous_peer_reviews\":false,\"group_category_id\":null,\"post_to_sis\":false,\"moderated_grading\":false,\"omit_from_final_grade\":false,\"intra_group_peer_reviews\":false,\"anonymous_instructor_annotations\":false,\"anonymous_grading\":false,\"graders_anonymous_to_graders\":false,\"grader_count\":0,\"grader_comments_visible_to_graders\":true,\"final_grader_id\":null,\"grader_names_visible_to_final_grader\":true,\"allowed_attempts\":-1,\"annotatable_attachment_id\":null,\"hide_in_gradebook\":false,\"secure_params\":\"ZZZ\",\"lti_context_id\":\"YYY\",\"course_id\":\"12345\",\"name\":\"Assignment 0\",\"submission_types\":[\"none\"],\"has_submitted_submissions\":false,\"due_date_required\":false,\"max_name_length\":255,\"in_closed_grading_period\":false,\"graded_submissions_exist\":true,\"is_quiz_assignment\":false,\"can_duplicate\":true,\"original_course_id\":null,\"original_assignment_id\":null,\"original_lti_resource_link_id\":null,\"original_assignment_name\":null,\"original_quiz_id\":null,\"workflow_state\":\"published\",\"important_dates\":false,\"muted\":false,\"html_url\":\"https://canvas.test.com/courses/12345/assignments/98765\",\"has_overrides\":false,\"needs_grading_count\":0,\"sis_assignment_id\":null,\"integration_id\":null,\"integration_data\":{},\"published\":true,\"unpublishable\":true,\"only_visible_to_overrides\":false,\"locked_for_user\":false,\"submissions_download_url\":\"https://canvas.test.com/courses/12345/assignments/98765/submissions?zip=1\",\"post_manually\":true,\"anonymize_students\":false,\"require_lockdown_browser\":false,\"restrict_quantitative_data\":false}" +} diff --git a/lms/backend/canvas/testdata/http/get-user-base-admin.json b/lms/backend/canvas/testdata/http/fetch-user-base-admin.json similarity index 100% rename from lms/backend/canvas/testdata/http/get-user-base-admin.json rename to lms/backend/canvas/testdata/http/fetch-user-base-admin.json diff --git a/lms/backend/canvas/testdata/http/get-user-base-owner.json b/lms/backend/canvas/testdata/http/fetch-user-base-owner.json similarity index 100% rename from lms/backend/canvas/testdata/http/get-user-base-owner.json rename to lms/backend/canvas/testdata/http/fetch-user-base-owner.json diff --git a/lms/backend/canvas/testdata/http/get-user-base-student.json b/lms/backend/canvas/testdata/http/fetch-user-base-student.json similarity index 100% rename from lms/backend/canvas/testdata/http/get-user-base-student.json rename to lms/backend/canvas/testdata/http/fetch-user-base-student.json diff --git a/lms/backend/canvas/testdata/http/get-users-base-01.json b/lms/backend/canvas/testdata/http/fetch-users-base-01.json similarity index 100% rename from lms/backend/canvas/testdata/http/get-users-base-01.json rename to lms/backend/canvas/testdata/http/fetch-users-base-01.json diff --git a/lms/backend/canvas/testdata/http/get-users-base-02.json b/lms/backend/canvas/testdata/http/fetch-users-base-02.json similarity index 100% rename from lms/backend/canvas/testdata/http/get-users-base-02.json rename to lms/backend/canvas/testdata/http/fetch-users-base-02.json diff --git a/lms/backend/canvas/testdata/http/get-users-base-03.json b/lms/backend/canvas/testdata/http/fetch-users-base-03.json similarity index 100% rename from lms/backend/canvas/testdata/http/get-users-base-03.json rename to lms/backend/canvas/testdata/http/fetch-users-base-03.json From e486a254159fec5dfeb2d68c0c98c26fc8e7d46b Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Wed, 27 Dec 2023 11:16:33 -0800 Subject: [PATCH 11/16] Added the ability to fetch a single LMS score and associated tests. --- cmd/lms-fetch-assignment-grade/main.go | 69 +++++++++++++++++++ cmd/lms-fetch-assignment-grades/main.go | 3 +- lms/backend/canvas/common.go | 12 ++++ lms/backend/canvas/scores.go | 37 ++++++++++ lms/backend/canvas/scores_test.go | 63 +++++++++++++++++ .../http/fetch-assignment-score-base.json | 22 ++++++ .../http/fetch-assignment-scores-base-01.json | 25 +++++++ .../http/fetch-assignment-scores-base-02.json | 25 +++++++ .../http/fetch-assignment-scores-base-03.json | 25 +++++++ lms/backend/canvas/users.go | 9 ++- lms/backend/test/backend.go | 4 ++ lms/lms.go | 10 +++ 12 files changed, 297 insertions(+), 7 deletions(-) create mode 100644 cmd/lms-fetch-assignment-grade/main.go create mode 100644 lms/backend/canvas/scores_test.go create mode 100644 lms/backend/canvas/testdata/http/fetch-assignment-score-base.json create mode 100644 lms/backend/canvas/testdata/http/fetch-assignment-scores-base-01.json create mode 100644 lms/backend/canvas/testdata/http/fetch-assignment-scores-base-02.json create mode 100644 lms/backend/canvas/testdata/http/fetch-assignment-scores-base-03.json diff --git a/cmd/lms-fetch-assignment-grade/main.go b/cmd/lms-fetch-assignment-grade/main.go new file mode 100644 index 00000000..fa377905 --- /dev/null +++ b/cmd/lms-fetch-assignment-grade/main.go @@ -0,0 +1,69 @@ +package main + +import ( + "fmt" + "strings" + + "github.com/alecthomas/kong" + "github.com/rs/zerolog/log" + + "github.com/eriq-augustine/autograder/config" + "github.com/eriq-augustine/autograder/db" + "github.com/eriq-augustine/autograder/lms" + "github.com/eriq-augustine/autograder/util" +) + +var args struct { + config.ConfigArgs + Course string `help:"ID of the course." arg:""` + Assignment string `help:"ID of the assignment." arg:""` + Email string `help:"Email of the user to fetch." arg:""` +} + +func main() { + kong.Parse(&args, + kong.Description("Fetch the grade for a specific assignment and user from an LMS."), + ); + + err := config.HandleConfigArgs(args.ConfigArgs); + if (err != nil) { + log.Fatal().Err(err).Msg("Could not load config options."); + } + + db.MustOpen(); + defer db.MustClose(); + + assignment := db.MustGetAssignment(args.Course, args.Assignment); + course := assignment.GetCourse(); + + if (assignment.GetLMSID() == "") { + log.Fatal().Str("assignment", assignment.FullID()).Msg("Assignment has no LMS ID."); + } + + user, err := db.GetUser(course, args.Email); + if (err != nil) { + log.Fatal().Err(err).Str("course", course.GetID()).Str("user", args.Email).Msg("Failed to fetch user."); + } + + if (user == nil) { + log.Fatal().Err(err).Str("course", course.GetID()).Str("user", args.Email).Msg("Could not find user."); + } + + if (user.LMSID == "") { + log.Fatal().Err(err).Str("course", course.GetID()).Str("user", args.Email).Msg("User does not have an LMS ID."); + } + + score, err := lms.FetchAssignmentScore(course, assignment.GetLMSID(), user.LMSID); + if (err != nil) { + log.Fatal().Err(err).Str("course", course.GetID()).Str("assignment", assignment.GetID()).Str("user", args.Email).Msg("User does not have an LMS ID."); + } + + textComments := make([]string, 0, len(score.Comments)); + for _, comment := range score.Comments { + textComments = append(textComments, comment.Text); + } + comments := strings.Join(textComments, ";"); + + fmt.Println("lms_user_id\tscore\ttime\tcomments"); + fmt.Printf("%s\t%s\t%s\t%s\n", score.UserID, util.FloatToStr(score.Score), score.Time, comments); +} diff --git a/cmd/lms-fetch-assignment-grades/main.go b/cmd/lms-fetch-assignment-grades/main.go index 9bc7d425..7119e461 100644 --- a/cmd/lms-fetch-assignment-grades/main.go +++ b/cmd/lms-fetch-assignment-grades/main.go @@ -21,8 +21,7 @@ var args struct { func main() { kong.Parse(&args, - kong.Description("Fetch the grades for a specific assignment from an LMS." + - " Either --assignment-path or (--course-path and --assignment-id) are required."), + kong.Description("Fetch the grades for a specific assignment from an LMS."), ); err := config.HandleConfigArgs(args.ConfigArgs); diff --git a/lms/backend/canvas/common.go b/lms/backend/canvas/common.go index 5c31366a..7ac65815 100644 --- a/lms/backend/canvas/common.go +++ b/lms/backend/canvas/common.go @@ -2,6 +2,7 @@ package canvas import ( "fmt" + neturl "net/url" "strings" "sync" "time" @@ -66,3 +67,14 @@ func fetchNextCanvasLink(headers map[string][]string) string { return ""; } + +// Rewrite a URL that appears in a LINK header for testing. +func (this *CanvasBackend) rewriteLink(url string) (string, error) { + parsed, err := neturl.Parse(url); + if (err != nil) { + return "", fmt.Errorf("Failed to parse URL '%s': '%w'.", url, err); + } + + url = fmt.Sprintf("%s%s?%s", this.BaseURL, parsed.Path, parsed.RawQuery); + return url, nil; +} diff --git a/lms/backend/canvas/scores.go b/lms/backend/canvas/scores.go index d263832e..26f255d5 100644 --- a/lms/backend/canvas/scores.go +++ b/lms/backend/canvas/scores.go @@ -9,7 +9,35 @@ import ( "github.com/eriq-augustine/autograder/util" ) +func (this *CanvasBackend) FetchAssignmentScore(assignmentID string, userID string) (*lmstypes.SubmissionScore, error) { + this.getAPILock(); + defer this.releaseAPILock(); + + apiEndpoint := fmt.Sprintf( + "/api/v1/courses/%s/assignments/%s/submissions/%s?include[]=submission_comments", + this.CourseID, assignmentID, userID); + url := this.BaseURL + apiEndpoint; + + headers := this.standardHeaders(); + body, _, err := common.GetWithHeaders(url, headers); + if (err != nil) { + return nil, fmt.Errorf("Failed to fetch score."); + } + + var rawScore SubmissionScore; + err = util.JSONFromString(body, &rawScore); + if (err != nil) { + return nil, fmt.Errorf("Failed to unmarshal raw scores: '%w'.", err); + } + + return rawScore.ToLMSType(), nil; +} + func (this *CanvasBackend) FetchAssignmentScores(assignmentID string) ([]*lmstypes.SubmissionScore, error) { + return this.fetchAssignmentScores(assignmentID, false); +} + +func (this *CanvasBackend) fetchAssignmentScores(assignmentID string, rewriteLinks bool) ([]*lmstypes.SubmissionScore, error) { this.getAPILock(); defer this.releaseAPILock(); @@ -23,6 +51,15 @@ func (this *CanvasBackend) FetchAssignmentScores(assignmentID string) ([]*lmstyp scores := make([]*lmstypes.SubmissionScore, 0); for (url != "") { + var err error; + + if (rewriteLinks) { + url, err = this.rewriteLink(url); + if (err != nil) { + return nil, err; + } + } + body, responseHeaders, err := common.GetWithHeaders(url, headers); if (err != nil) { diff --git a/lms/backend/canvas/scores_test.go b/lms/backend/canvas/scores_test.go new file mode 100644 index 00000000..9cda3a69 --- /dev/null +++ b/lms/backend/canvas/scores_test.go @@ -0,0 +1,63 @@ +package canvas + +import ( + "testing" + "time" + + "github.com/eriq-augustine/autograder/lms/lmstypes" + "github.com/eriq-augustine/autograder/util" +) + +var testScore lmstypes.SubmissionScore = lmstypes.SubmissionScore{ + UserID: "00040", + Score: 100.0, + Time: time.Time{}, + Comments: []*lmstypes.SubmissionComment{ + &lmstypes.SubmissionComment{ + ID: "0987654", + Author: "7827", + Text: "{\n\"id\": \"course101::hw0::student@test.com::1696364768\",\n\"submission-time\": \"2023-10-03T20:26:08.951546Z\",\n\"upload-time\": \"2023-10-07T13:04:54.412979316-05:00\",\n\"raw-score\": 100,\n\"score\": 100,\n\"lock\": false,\n\"late-date-usage\": 0,\n\"num-days-late\": 0,\n\"reject\": false,\n\"__autograder__v01__\": 0\n}", + Time: "", + }, + }, +}; + +func TestFetchAssignmentSccoreBase(test *testing.T) { + score, err := testBackend.FetchAssignmentScore(TEST_ASSIGNMENT_ID, "00040"); + if (err != nil) { + test.Fatalf("Failed to fetch assignment score: '%v'.", err); + } + + // Can't compare directly because of time.Time. + // Use JSON instead. + expectedJSON := util.MustToJSONIndent(testScore); + actualJSON := util.MustToJSONIndent(score); + + if (expectedJSON != actualJSON) { + test.Fatalf("Score not as expected. Expected: '%s', Actual: '%s'.", + expectedJSON, actualJSON); + } +} + +func TestFetchAssignmentSccoresBase(test *testing.T) { + scores, err := testBackend.fetchAssignmentScores(TEST_ASSIGNMENT_ID, true); + if (err != nil) { + test.Fatalf("Failed to fetch assignment scores: '%v'.", err); + } + + expected := []*lmstypes.SubmissionScore{ + &testScore, + &testScore, + &testScore, + } + + // Can't compare directly because of time.Time. + // Use JSON instead. + expectedJSON := util.MustToJSONIndent(expected); + actualJSON := util.MustToJSONIndent(scores); + + if (expectedJSON != actualJSON) { + test.Fatalf("Scores not as expected. Expected: '%s', Actual: '%s'.", + expectedJSON, actualJSON); + } +} diff --git a/lms/backend/canvas/testdata/http/fetch-assignment-score-base.json b/lms/backend/canvas/testdata/http/fetch-assignment-score-base.json new file mode 100644 index 00000000..08afb200 --- /dev/null +++ b/lms/backend/canvas/testdata/http/fetch-assignment-score-base.json @@ -0,0 +1,22 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/assignments/98765/submissions/00040?include[]=submission_comments", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Status": [ + "200 OK" + ] + }, + "ResponseBody": "{\"id\":\"12345678\",\"body\":null,\"url\":null,\"grade\":\"100\",\"score\":100.0,\"submitted_at\":null,\"assignment_id\":\"98765\",\"user_id\":\"00040\",\"submission_type\":null,\"workflow_state\":\"graded\",\"grade_matches_current_submission\":true,\"graded_at\":\"2023-10-07T18:05:35Z\",\"grader_id\":\"7827\",\"attempt\":null,\"cached_due_date\":\"2023-10-06T06:59:59Z\",\"excused\":false,\"late_policy_status\":null,\"points_deducted\":null,\"grading_period_id\":null,\"extra_attempts\":null,\"posted_at\":\"2023-12-16T20:42:37Z\",\"redo_request\":false,\"custom_grade_status_id\":null,\"sticker\":null,\"late\":false,\"missing\":false,\"seconds_late\":7127012,\"entered_grade\":\"100\",\"entered_score\":100.0,\"preview_url\":\"https://canvas.test.com/courses/12345/assignments/98765/submissions/00040?preview=1\\u0026version=1\",\"submission_comments\":[{\"id\":\"0987654\",\"comment\":\"{\\n\\\"id\\\": \\\"course101::hw0::student@test.com::1696364768\\\",\\n\\\"submission-time\\\": \\\"2023-10-03T20:26:08.951546Z\\\",\\n\\\"upload-time\\\": \\\"2023-10-07T13:04:54.412979316-05:00\\\",\\n\\\"raw-score\\\": 100,\\n\\\"score\\\": 100,\\n\\\"lock\\\": false,\\n\\\"late-date-usage\\\": 0,\\n\\\"num-days-late\\\": 0,\\n\\\"reject\\\": false,\\n\\\"__autograder__v01__\\\": 0\\n}\",\"author_id\":\"7827\",\"author_name\":\"Eriq Augustine\",\"created_at\":\"2023-10-07T18:05:35Z\",\"edited_at\":null,\"attempt\":null,\"avatar_path\":\"/images/users/7827-b1af3557c8\",\"author\":{\"id\":\"7827\",\"anonymous_id\":\"61f\",\"display_name\":\"Eriq Augustine\",\"avatar_image_url\":\"https://canvas.test.com/images/messages/avatar-50.png\",\"html_url\":\"https://canvas.test.com/courses/12345/users/7827\",\"pronouns\":null}}]}" +} diff --git a/lms/backend/canvas/testdata/http/fetch-assignment-scores-base-01.json b/lms/backend/canvas/testdata/http/fetch-assignment-scores-base-01.json new file mode 100644 index 00000000..7c4ee01e --- /dev/null +++ b/lms/backend/canvas/testdata/http/fetch-assignment-scores-base-01.json @@ -0,0 +1,25 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/assignments/98765/submissions?per_page=75\u0026include[]=submission_comments", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "; rel=\"current\",; rel=\"next\",; rel=\"first\",; rel=\"last\"" + ], + "Status": [ + "200 OK" + ] + }, + "ResponseBody": "[{\"id\":\"12345678\",\"body\":null,\"url\":null,\"grade\":\"100\",\"score\":100.0,\"submitted_at\":null,\"assignment_id\":\"98765\",\"user_id\":\"00040\",\"submission_type\":null,\"workflow_state\":\"graded\",\"grade_matches_current_submission\":true,\"graded_at\":\"2023-10-07T18:05:35Z\",\"grader_id\":\"7827\",\"attempt\":null,\"cached_due_date\":\"2023-10-06T06:59:59Z\",\"excused\":false,\"late_policy_status\":null,\"points_deducted\":null,\"grading_period_id\":null,\"extra_attempts\":null,\"posted_at\":\"2023-12-16T20:42:37Z\",\"redo_request\":false,\"custom_grade_status_id\":null,\"sticker\":null,\"late\":false,\"missing\":false,\"seconds_late\":7127012,\"entered_grade\":\"100\",\"entered_score\":100.0,\"preview_url\":\"https://canvas.test.com/courses/12345/assignments/98765/submissions/00040?preview=1\\u0026version=1\",\"submission_comments\":[{\"id\":\"0987654\",\"comment\":\"{\\n\\\"id\\\": \\\"course101::hw0::student@test.com::1696364768\\\",\\n\\\"submission-time\\\": \\\"2023-10-03T20:26:08.951546Z\\\",\\n\\\"upload-time\\\": \\\"2023-10-07T13:04:54.412979316-05:00\\\",\\n\\\"raw-score\\\": 100,\\n\\\"score\\\": 100,\\n\\\"lock\\\": false,\\n\\\"late-date-usage\\\": 0,\\n\\\"num-days-late\\\": 0,\\n\\\"reject\\\": false,\\n\\\"__autograder__v01__\\\": 0\\n}\",\"author_id\":\"7827\",\"author_name\":\"Eriq Augustine\",\"created_at\":\"2023-10-07T18:05:35Z\",\"edited_at\":null,\"attempt\":null,\"avatar_path\":\"/images/users/7827-b1af3557c8\",\"author\":{\"id\":\"7827\",\"anonymous_id\":\"61f\",\"display_name\":\"Eriq Augustine\",\"avatar_image_url\":\"https://canvas.test.com/images/messages/avatar-50.png\",\"html_url\":\"https://canvas.test.com/courses/12345/users/7827\",\"pronouns\":null}}]}]" +} diff --git a/lms/backend/canvas/testdata/http/fetch-assignment-scores-base-02.json b/lms/backend/canvas/testdata/http/fetch-assignment-scores-base-02.json new file mode 100644 index 00000000..7d50933a --- /dev/null +++ b/lms/backend/canvas/testdata/http/fetch-assignment-scores-base-02.json @@ -0,0 +1,25 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/assignments/98765/submissions?include%5B%5D=submission_comments\u0026page=2\u0026per_page=75", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "; rel=\"current\",; rel=\"next\",; rel=\"prev\",; rel=\"first\",; rel=\"last\"" + ], + "Status": [ + "200 OK" + ] + }, + "ResponseBody": "[{\"id\":\"12345678\",\"body\":null,\"url\":null,\"grade\":\"100\",\"score\":100.0,\"submitted_at\":null,\"assignment_id\":\"98765\",\"user_id\":\"00040\",\"submission_type\":null,\"workflow_state\":\"graded\",\"grade_matches_current_submission\":true,\"graded_at\":\"2023-10-07T18:05:35Z\",\"grader_id\":\"7827\",\"attempt\":null,\"cached_due_date\":\"2023-10-06T06:59:59Z\",\"excused\":false,\"late_policy_status\":null,\"points_deducted\":null,\"grading_period_id\":null,\"extra_attempts\":null,\"posted_at\":\"2023-12-16T20:42:37Z\",\"redo_request\":false,\"custom_grade_status_id\":null,\"sticker\":null,\"late\":false,\"missing\":false,\"seconds_late\":7127012,\"entered_grade\":\"100\",\"entered_score\":100.0,\"preview_url\":\"https://canvas.test.com/courses/12345/assignments/98765/submissions/00040?preview=1\\u0026version=1\",\"submission_comments\":[{\"id\":\"0987654\",\"comment\":\"{\\n\\\"id\\\": \\\"course101::hw0::student@test.com::1696364768\\\",\\n\\\"submission-time\\\": \\\"2023-10-03T20:26:08.951546Z\\\",\\n\\\"upload-time\\\": \\\"2023-10-07T13:04:54.412979316-05:00\\\",\\n\\\"raw-score\\\": 100,\\n\\\"score\\\": 100,\\n\\\"lock\\\": false,\\n\\\"late-date-usage\\\": 0,\\n\\\"num-days-late\\\": 0,\\n\\\"reject\\\": false,\\n\\\"__autograder__v01__\\\": 0\\n}\",\"author_id\":\"7827\",\"author_name\":\"Eriq Augustine\",\"created_at\":\"2023-10-07T18:05:35Z\",\"edited_at\":null,\"attempt\":null,\"avatar_path\":\"/images/users/7827-b1af3557c8\",\"author\":{\"id\":\"7827\",\"anonymous_id\":\"61f\",\"display_name\":\"Eriq Augustine\",\"avatar_image_url\":\"https://canvas.test.com/images/messages/avatar-50.png\",\"html_url\":\"https://canvas.test.com/courses/12345/users/7827\",\"pronouns\":null}}]}]" +} diff --git a/lms/backend/canvas/testdata/http/fetch-assignment-scores-base-03.json b/lms/backend/canvas/testdata/http/fetch-assignment-scores-base-03.json new file mode 100644 index 00000000..5608a2cf --- /dev/null +++ b/lms/backend/canvas/testdata/http/fetch-assignment-scores-base-03.json @@ -0,0 +1,25 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/assignments/98765/submissions?include%5B%5D=submission_comments\u0026page=3\u0026per_page=75", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "; rel=\"current\",; rel=\"prev\",; rel=\"first\",; rel=\"last\"" + ], + "Status": [ + "200 OK" + ] + }, + "ResponseBody": "[{\"id\":\"12345678\",\"body\":null,\"url\":null,\"grade\":\"100\",\"score\":100.0,\"submitted_at\":null,\"assignment_id\":\"98765\",\"user_id\":\"00040\",\"submission_type\":null,\"workflow_state\":\"graded\",\"grade_matches_current_submission\":true,\"graded_at\":\"2023-10-07T18:05:35Z\",\"grader_id\":\"7827\",\"attempt\":null,\"cached_due_date\":\"2023-10-06T06:59:59Z\",\"excused\":false,\"late_policy_status\":null,\"points_deducted\":null,\"grading_period_id\":null,\"extra_attempts\":null,\"posted_at\":\"2023-12-16T20:42:37Z\",\"redo_request\":false,\"custom_grade_status_id\":null,\"sticker\":null,\"late\":false,\"missing\":false,\"seconds_late\":7127012,\"entered_grade\":\"100\",\"entered_score\":100.0,\"preview_url\":\"https://canvas.test.com/courses/12345/assignments/98765/submissions/00040?preview=1\\u0026version=1\",\"submission_comments\":[{\"id\":\"0987654\",\"comment\":\"{\\n\\\"id\\\": \\\"course101::hw0::student@test.com::1696364768\\\",\\n\\\"submission-time\\\": \\\"2023-10-03T20:26:08.951546Z\\\",\\n\\\"upload-time\\\": \\\"2023-10-07T13:04:54.412979316-05:00\\\",\\n\\\"raw-score\\\": 100,\\n\\\"score\\\": 100,\\n\\\"lock\\\": false,\\n\\\"late-date-usage\\\": 0,\\n\\\"num-days-late\\\": 0,\\n\\\"reject\\\": false,\\n\\\"__autograder__v01__\\\": 0\\n}\",\"author_id\":\"7827\",\"author_name\":\"Eriq Augustine\",\"created_at\":\"2023-10-07T18:05:35Z\",\"edited_at\":null,\"attempt\":null,\"avatar_path\":\"/images/users/7827-b1af3557c8\",\"author\":{\"id\":\"7827\",\"anonymous_id\":\"61f\",\"display_name\":\"Eriq Augustine\",\"avatar_image_url\":\"https://canvas.test.com/images/messages/avatar-50.png\",\"html_url\":\"https://canvas.test.com/courses/12345/users/7827\",\"pronouns\":null}}]}]" +} diff --git a/lms/backend/canvas/users.go b/lms/backend/canvas/users.go index eddbf615..ff4dc853 100644 --- a/lms/backend/canvas/users.go +++ b/lms/backend/canvas/users.go @@ -29,13 +29,13 @@ func (this *CanvasBackend) fetchUsers(rewriteLinks bool) ([]*lmstypes.User, erro users := make([]*lmstypes.User, 0); for (url != "") { + var err error; + if (rewriteLinks) { - parsed, err := neturl.Parse(url); + url, err = this.rewriteLink(url); if (err != nil) { - return nil, fmt.Errorf("Failed to parse URL '%s': '%w'.", url, err); + return nil, err; } - - url = fmt.Sprintf("%s%s?%s", this.BaseURL, parsed.Path, parsed.RawQuery); } body, responseHeaders, err := common.GetWithHeaders(url, headers); @@ -75,7 +75,6 @@ func (this *CanvasBackend) FetchUser(email string) (*lmstypes.User, error) { headers := this.standardHeaders(); body, _, err := common.GetWithHeaders(url, headers); - if (err != nil) { return nil, fmt.Errorf("Failed to fetch user '%s': '%w'.", email, err); } diff --git a/lms/backend/test/backend.go b/lms/backend/test/backend.go index 0327d6c1..7463181e 100644 --- a/lms/backend/test/backend.go +++ b/lms/backend/test/backend.go @@ -57,3 +57,7 @@ func (this *TestLMSBackend) UpdateComment(assignmentID string, comment *lmstypes func (this *TestLMSBackend) FetchAssignmentScores(assignmentID string) ([]*lmstypes.SubmissionScore, error) { return nil, nil; } + +func (this *TestLMSBackend) FetchAssignmentScore(assignmentID string, userID string) (*lmstypes.SubmissionScore, error) { + return nil, nil; +} diff --git a/lms/lms.go b/lms/lms.go index 8c887441..b84251a7 100644 --- a/lms/lms.go +++ b/lms/lms.go @@ -16,6 +16,7 @@ type lmsBackend interface { UpdateComment(assignmentID string, comment *lmstypes.SubmissionComment) error FetchAssignmentScores(assignmentID string) ([]*lmstypes.SubmissionScore, error) + FetchAssignmentScore(assignmentID string, userID string) (*lmstypes.SubmissionScore, error) UpdateAssignmentScores(assignmentID string, scores []*lmstypes.SubmissionScore) error FetchUsers() ([]*lmstypes.User, error) @@ -84,6 +85,15 @@ func FetchAssignmentScores(course *model.Course, assignmentID string) ([]*lmstyp return backend.FetchAssignmentScores(assignmentID); } +func FetchAssignmentScore(course *model.Course, assignmentID string, userID string) (*lmstypes.SubmissionScore, error) { + backend, err := getBackend(course); + if (err != nil) { + return nil, err; + } + + return backend.FetchAssignmentScore(assignmentID, userID); +} + func UpdateAssignmentScores(course *model.Course, assignmentID string, scores []*lmstypes.SubmissionScore) error { backend, err := getBackend(course); if (err != nil) { From a96e480ee5dc501cb7c00802e5cba485410763fa Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Wed, 27 Dec 2023 14:10:54 -0800 Subject: [PATCH 12/16] Move LMS user syncing to the procedures package and added the ability to only sync a few users (versus one or all users). --- api/lms/sync_users.go | 4 +-- api/lms/sync_users_test.go | 1 - api/user/add.go | 6 ++-- api/user/add_test.go | 6 ++-- cmd/lms-sync-users/main.go | 4 +-- cmd/users/main.go | 11 ++++++-- .../sync.go => procedures/lms_sync.go | 28 +++++++++++-------- .../lms_sync_test.go | 4 +-- {lms/lmsusers => procedures}/main_test.go | 2 +- 9 files changed, 39 insertions(+), 27 deletions(-) rename lms/lmsusers/sync.go => procedures/lms_sync.go (87%) rename lms/lmsusers/sync_test.go => procedures/lms_sync_test.go (98%) rename {lms/lmsusers => procedures}/main_test.go (95%) diff --git a/api/lms/sync_users.go b/api/lms/sync_users.go index fca25139..a3894acf 100644 --- a/api/lms/sync_users.go +++ b/api/lms/sync_users.go @@ -2,7 +2,7 @@ package lms import ( "github.com/eriq-augustine/autograder/api/core" - "github.com/eriq-augustine/autograder/lms/lmsusers" + "github.com/eriq-augustine/autograder/procedures" ) type SyncUsersRequest struct { @@ -23,7 +23,7 @@ func HandleSyncUsers(request *SyncUsersRequest) (*SyncUsersResponse, *core.APIEr Add("course", request.Course.GetID()); } - result, err := lmsusers.SyncLMSUsers(request.Course, request.DryRun, !request.SkipEmails); + result, err := procedures.SyncAllLMSUsers(request.Course, request.DryRun, !request.SkipEmails); if (err != nil) { return nil, core.NewInternalError("-504", &request.APIRequestCourseUserContext, "Failed to sync LMS users.").Err(err); diff --git a/api/lms/sync_users_test.go b/api/lms/sync_users_test.go index 5c5e567b..79616591 100644 --- a/api/lms/sync_users_test.go +++ b/api/lms/sync_users_test.go @@ -8,7 +8,6 @@ import ( "github.com/eriq-augustine/autograder/util" ) -// lmsusers.SyncLMSUsers() is already heavily tested, focus on API-specific functionality. func TestAPISyncUsers(test *testing.T) { testCases := []struct{ role model.UserRole; dryRun bool; skipEmails bool; permError bool }{ {model.RoleOther, false, true, true}, diff --git a/api/user/add.go b/api/user/add.go index 7e4958b6..ca5afcdf 100644 --- a/api/user/add.go +++ b/api/user/add.go @@ -6,8 +6,8 @@ import ( "github.com/eriq-augustine/autograder/api/core" "github.com/eriq-augustine/autograder/db" - "github.com/eriq-augustine/autograder/lms/lmsusers" "github.com/eriq-augustine/autograder/model" + "github.com/eriq-augustine/autograder/procedures" ) type AddRequest struct { @@ -41,6 +41,7 @@ func HandleAdd(request *AddRequest) (*AddResponse, *core.APIError) { }; newUsers := make(map[string]*model.User, len(request.NewUsers)); + emails := make([]string, 0, len(request.NewUsers)); for i, apiUser := range request.NewUsers { user, err := apiUser.ToUsr(); @@ -65,6 +66,7 @@ func HandleAdd(request *AddRequest) (*AddResponse, *core.APIError) { } newUsers[apiUser.Email] = user; + emails = append(emails, apiUser.Email); } result, err := db.SyncUsers(request.Course, newUsers, request.Force, request.DryRun, !request.SkipEmails); @@ -74,7 +76,7 @@ func HandleAdd(request *AddRequest) (*AddResponse, *core.APIError) { } if (!request.SkipLMSSync) { - lmsResult, err := lmsusers.SyncLMSUsers(request.Course, request.DryRun, !request.SkipEmails); + lmsResult, err := procedures.SyncLMSUserEmails(request.Course, emails, request.DryRun, !request.SkipEmails); if (err != nil) { log.Error().Err(err).Str("api-request", request.RequestID).Msg("Failed to sync LMS users."); } else { diff --git a/api/user/add_test.go b/api/user/add_test.go index f2f980b5..eba2246e 100644 --- a/api/user/add_test.go +++ b/api/user/add_test.go @@ -44,7 +44,7 @@ func TestUserAdd(test *testing.T) { Unchanged: []*core.UserInfo{}, }, Errors: []AddError{}, - LMSSyncCount: 6, + LMSSyncCount: 2, }, }, @@ -70,7 +70,7 @@ func TestUserAdd(test *testing.T) { AddError{1, "owner@test.com", "Cannot create a user with a higher role (owner) than your role (admin)."}, AddError{2, "owner@test.com", "Cannot modify a user with a higher role (owner) than your role (admin)."}, }, - LMSSyncCount: 5, + LMSSyncCount: 0, }, }, @@ -121,7 +121,7 @@ func TestUserAdd(test *testing.T) { Unchanged: []*core.UserInfo{}, }, Errors: []AddError{}, - LMSSyncCount: 5, + LMSSyncCount: 0, }, }, }; diff --git a/cmd/lms-sync-users/main.go b/cmd/lms-sync-users/main.go index d6c5c4b0..7fc20e8e 100644 --- a/cmd/lms-sync-users/main.go +++ b/cmd/lms-sync-users/main.go @@ -6,7 +6,7 @@ import ( "github.com/eriq-augustine/autograder/config" "github.com/eriq-augustine/autograder/db" - "github.com/eriq-augustine/autograder/lms/lmsusers" + "github.com/eriq-augustine/autograder/procedures" ) var args struct { @@ -31,7 +31,7 @@ func main() { course := db.MustGetCourse(args.Course); - result, err := lmsusers.SyncLMSUsers(course, args.DryRun, !args.SkipEmails); + result, err := procedures.SyncAllLMSUsers(course, args.DryRun, !args.SkipEmails); if (err != nil) { log.Fatal().Err(err).Msg("Failed to sync LMS users."); } diff --git a/cmd/users/main.go b/cmd/users/main.go index 2e99eaa5..551b9f5b 100644 --- a/cmd/users/main.go +++ b/cmd/users/main.go @@ -12,8 +12,8 @@ import ( "github.com/eriq-augustine/autograder/config" "github.com/eriq-augustine/autograder/db" - "github.com/eriq-augustine/autograder/lms/lmsusers" "github.com/eriq-augustine/autograder/model" + "github.com/eriq-augustine/autograder/procedures" "github.com/eriq-augustine/autograder/util" ) @@ -55,7 +55,7 @@ func (this *AddUser) Run(course *model.Course) error { result.PrintReport(); if (this.SyncLMS) { - result, err = lmsusers.SyncLMSUser(course, this.Email, this.DryRun, this.SendEmail); + result, err = procedures.SyncLMSUserEmail(course, this.Email, this.DryRun, this.SendEmail); if (err != nil) { return err; } @@ -93,7 +93,12 @@ func (this *AddTSV) Run(course *model.Course) error { } if (this.SyncLMS) { - result, err = lmsusers.SyncLMSUsers(course, this.DryRun, this.SendEmail); + emails := make([]string, 0, len(newUsers)); + for _, newUser := range newUsers { + emails = append(emails, newUser.Email); + } + + result, err = procedures.SyncLMSUserEmails(course, emails, this.DryRun, this.SendEmail); if (err != nil) { return err; } diff --git a/lms/lmsusers/sync.go b/procedures/lms_sync.go similarity index 87% rename from lms/lmsusers/sync.go rename to procedures/lms_sync.go index 73619200..c835e591 100644 --- a/lms/lmsusers/sync.go +++ b/procedures/lms_sync.go @@ -1,4 +1,4 @@ -package lmsusers +package procedures import ( "errors" @@ -12,7 +12,7 @@ import ( ) // Sync users with the provided LMS. -func SyncLMSUsers(course *model.Course, dryRun bool, sendEmails bool) (*model.UserSyncResult, error) { +func SyncAllLMSUsers(course *model.Course, dryRun bool, sendEmails bool) (*model.UserSyncResult, error) { lmsUsersSlice, err := lms.FetchUsers(course); if (err != nil) { return nil, fmt.Errorf("Failed to fetch LMS users: '%w'.", err); @@ -26,17 +26,23 @@ func SyncLMSUsers(course *model.Course, dryRun bool, sendEmails bool) (*model.Us return syncLMSUsers(course, dryRun, sendEmails, lmsUsers, nil); } -func SyncLMSUser(course *model.Course, email string, dryRun bool, sendEmails bool) (*model.UserSyncResult, error) { - lmsUser, err := lms.FetchUser(course, email); - if (err != nil) { - return nil, err; - } +func SyncLMSUserEmail(course *model.Course, email string, dryRun bool, sendEmails bool) (*model.UserSyncResult, error) { + return SyncLMSUserEmails(course, []string{email}, dryRun, sendEmails); +} + +func SyncLMSUserEmails(course *model.Course, emails []string, dryRun bool, sendEmails bool) (*model.UserSyncResult, error) { + lmsUsers := make(map[string]*lmstypes.User); + + for _, email := range emails { + lmsUser, err := lms.FetchUser(course, email); + if (err != nil) { + return nil, err; + } - lmsUsers := map[string]*lmstypes.User{ - lmsUser.Email: lmsUser, - }; + lmsUsers[lmsUser.Email] = lmsUser; + } - return syncLMSUsers(course, dryRun, sendEmails, lmsUsers, []string{email}); + return syncLMSUsers(course, dryRun, sendEmails, lmsUsers, emails); } // Sync users. diff --git a/lms/lmsusers/sync_test.go b/procedures/lms_sync_test.go similarity index 98% rename from lms/lmsusers/sync_test.go rename to procedures/lms_sync_test.go index 770267db..8f7f5a83 100644 --- a/lms/lmsusers/sync_test.go +++ b/procedures/lms_sync_test.go @@ -1,4 +1,4 @@ -package lmsusers +package procedures import ( "slices" @@ -44,7 +44,7 @@ func TestCourseSyncLMSUsers(test *testing.T) { email.ClearTestMessages(); - result, err := SyncLMSUsers(course, testCase.dryRun, testCase.sendEmails); + result, err := SyncAllLMSUsers(course, testCase.dryRun, testCase.sendEmails); if (err != nil) { test.Errorf("Case %d (%+v): User sync failed: '%v'.", i, testCase, err); continue; diff --git a/lms/lmsusers/main_test.go b/procedures/main_test.go similarity index 95% rename from lms/lmsusers/main_test.go rename to procedures/main_test.go index 0d85b80f..fd6ff42c 100644 --- a/lms/lmsusers/main_test.go +++ b/procedures/main_test.go @@ -1,4 +1,4 @@ -package lmsusers +package procedures import ( "os" From 6c444c476b437d4af6be1f3e8b87fbeaf5d50980 Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Wed, 27 Dec 2023 23:04:41 -0800 Subject: [PATCH 13/16] Added the ability to get all assignments for a course. --- cmd/lms-fetch-assignment/main.go | 3 +- cmd/lms-fetch-assignments/main.go | 41 +++++++++++++++ cmd/lms-upload-assignment-grades/main.go | 3 +- lms/backend/canvas/assignment.go | 52 +++++++++++++++++++ lms/backend/canvas/assignment_test.go | 35 +++++++------ lms/backend/canvas/main_test.go | 11 ++++ .../testdata/http/fetch-assignments-base.json | 25 +++++++++ lms/backend/canvas/users.go | 1 - lms/backend/test/backend.go | 4 ++ lms/lms.go | 10 ++++ model/lms.go | 7 ++- model/sync.go | 4 ++ procedures/lms_sync.go | 4 +- procedures/lms_sync_test.go | 4 +- 14 files changed, 178 insertions(+), 26 deletions(-) create mode 100644 cmd/lms-fetch-assignments/main.go create mode 100644 lms/backend/canvas/testdata/http/fetch-assignments-base.json diff --git a/cmd/lms-fetch-assignment/main.go b/cmd/lms-fetch-assignment/main.go index e6473d6a..011f03d6 100644 --- a/cmd/lms-fetch-assignment/main.go +++ b/cmd/lms-fetch-assignment/main.go @@ -20,8 +20,7 @@ var args struct { func main() { kong.Parse(&args, - kong.Description("Fetch the grades for a specific assignment from an LMS." + - " Either --assignment-path or (--course-path and --assignment-id) are required."), + kong.Description("Fetch information about a specific assignment from an LMS."), ); err := config.HandleConfigArgs(args.ConfigArgs); diff --git a/cmd/lms-fetch-assignments/main.go b/cmd/lms-fetch-assignments/main.go new file mode 100644 index 00000000..41796734 --- /dev/null +++ b/cmd/lms-fetch-assignments/main.go @@ -0,0 +1,41 @@ +package main + +import ( + "fmt" + + "github.com/alecthomas/kong" + "github.com/rs/zerolog/log" + + "github.com/eriq-augustine/autograder/config" + "github.com/eriq-augustine/autograder/db" + "github.com/eriq-augustine/autograder/lms" + "github.com/eriq-augustine/autograder/util" +) + +var args struct { + config.ConfigArgs + Course string `help:"ID of the course." arg:""` +} + +func main() { + kong.Parse(&args, + kong.Description("Fetch information about all assignments from an LMS."), + ); + + err := config.HandleConfigArgs(args.ConfigArgs); + if (err != nil) { + log.Fatal().Err(err).Msg("Could not load config options."); + } + + db.MustOpen(); + defer db.MustClose(); + + course := db.MustGetCourse(args.Course); + + lmsAssignments, err := lms.FetchAssignments(course); + if (err != nil) { + log.Fatal().Err(err).Msg("Failed to fetch assignments."); + } + + fmt.Println(util.MustToJSONIndent(lmsAssignments)); +} diff --git a/cmd/lms-upload-assignment-grades/main.go b/cmd/lms-upload-assignment-grades/main.go index 00de9ab2..38757ba8 100644 --- a/cmd/lms-upload-assignment-grades/main.go +++ b/cmd/lms-upload-assignment-grades/main.go @@ -25,8 +25,7 @@ var args struct { func main() { kong.Parse(&args, - kong.Description("Upload grades for an assignment to the coure's LMS from a TSV file." + - " Either --assignment-path or (--course-path and --assignment-id) are required."), + kong.Description("Upload grades for an assignment to the coure's LMS from a TSV file."), ); err := config.HandleConfigArgs(args.ConfigArgs); diff --git a/lms/backend/canvas/assignment.go b/lms/backend/canvas/assignment.go index 192be988..62f7c212 100644 --- a/lms/backend/canvas/assignment.go +++ b/lms/backend/canvas/assignment.go @@ -32,3 +32,55 @@ func (this *CanvasBackend) FetchAssignment(assignmentID string) (*lmstypes.Assig return assignment.ToLMSType(), nil; } + +func (this *CanvasBackend) FetchAssignments() ([]*lmstypes.Assignment, error) { + return this.fetchAssignments(false); +} + +func (this *CanvasBackend) fetchAssignments(rewriteLinks bool) ([]*lmstypes.Assignment, error) { + this.getAPILock(); + defer this.releaseAPILock(); + + apiEndpoint := fmt.Sprintf( + "/api/v1/courses/%s/assignments?per_page=%d", + this.CourseID, PAGE_SIZE); + url := this.BaseURL + apiEndpoint; + + headers := this.standardHeaders(); + + assignments := make([]*lmstypes.Assignment, 0); + + for (url != "") { + var err error; + + if (rewriteLinks) { + url, err = this.rewriteLink(url); + if (err != nil) { + return nil, err; + } + } + + body, responseHeaders, err := common.GetWithHeaders(url, headers); + if (err != nil) { + return nil, fmt.Errorf("Failed to fetch users: '%w'.", err); + } + + var pageAssignments []*Assignment; + err = util.JSONFromString(body, &pageAssignments); + if (err != nil) { + return nil, fmt.Errorf("Failed to unmarshal assignments page: '%w'.", err); + } + + for _, assignment := range pageAssignments { + if (assignment == nil) { + continue; + } + + assignments = append(assignments, assignment.ToLMSType()); + } + + url = fetchNextCanvasLink(responseHeaders); + } + + return assignments, nil; +} diff --git a/lms/backend/canvas/assignment_test.go b/lms/backend/canvas/assignment_test.go index 70760b00..45bfb8e4 100644 --- a/lms/backend/canvas/assignment_test.go +++ b/lms/backend/canvas/assignment_test.go @@ -2,23 +2,20 @@ package canvas import ( "testing" - "time" "github.com/eriq-augustine/autograder/lms/lmstypes" "github.com/eriq-augustine/autograder/util" ) -const TEST_ASSIGNMENT_ID = "98765"; +var expectedAssignment lmstypes.Assignment = lmstypes.Assignment{ + ID: TEST_ASSIGNMENT_ID, + Name: "Assignment 0", + LMSCourseID: "12345", + DueDate: mustParseTime("2023-10-06T06:59:59Z"), + MaxPoints: 100.0, +}; func TestFetchAssignmentBase(test *testing.T) { - expected := lmstypes.Assignment{ - ID: TEST_ASSIGNMENT_ID, - Name: "Assignment 0", - LMSCourseID: "12345", - DueDate: mustParseTime(test, "2023-10-06T06:59:59Z"), - MaxPoints: 100.0, - }; - assignment, err := testBackend.FetchAssignment(TEST_ASSIGNMENT_ID); if (err != nil) { test.Fatalf("Failed tp fetch assignment: '%v'.", err); @@ -26,7 +23,7 @@ func TestFetchAssignmentBase(test *testing.T) { // Can't compare directly because of time.Time. // Use JSON instead. - expectedJSON := util.MustToJSONIndent(expected); + expectedJSON := util.MustToJSONIndent(expectedAssignment); actualJSON := util.MustToJSONIndent(assignment); if (expectedJSON != actualJSON) { @@ -35,11 +32,19 @@ func TestFetchAssignmentBase(test *testing.T) { } } -func mustParseTime(test *testing.T, text string) *time.Time { - instance, err := time.Parse(time.RFC3339, text); +func TestFetchAssignmentsBase(test *testing.T) { + assignments, err := testBackend.FetchAssignments(); if (err != nil) { - test.Fatalf("Failed to parse time '%s': '%v'.", text, err); + test.Fatalf("Failed tp fetch assignments: '%v'.", err); } - return &instance; + // Can't compare directly because of time.Time. + // Use JSON instead. + expectedJSON := util.MustToJSONIndent([]*lmstypes.Assignment{&expectedAssignment}); + actualJSON := util.MustToJSONIndent(assignments); + + if (expectedJSON != actualJSON) { + test.Fatalf("Assignment not as expected. Expected: '%s', Actual: '%s'.", + expectedJSON, actualJSON); + } } diff --git a/lms/backend/canvas/main_test.go b/lms/backend/canvas/main_test.go index 429b3593..3b13ed37 100644 --- a/lms/backend/canvas/main_test.go +++ b/lms/backend/canvas/main_test.go @@ -10,6 +10,7 @@ import ( "os" "strings" "testing" + "time" "github.com/eriq-augustine/autograder/common" "github.com/eriq-augustine/autograder/db" @@ -18,6 +19,7 @@ import ( const ( TEST_COURSE_ID = "12345" + TEST_ASSIGNMENT_ID = "98765" TEST_TOKEN = "ABC123" ) @@ -151,3 +153,12 @@ func stopTestServer() { serverURL = ""; } } + +func mustParseTime(text string) *time.Time { + instance, err := time.Parse(time.RFC3339, text); + if (err != nil) { + panic(fmt.Sprintf("Failed to parse time '%s': '%v'.", text, err)); + } + + return &instance; +} diff --git a/lms/backend/canvas/testdata/http/fetch-assignments-base.json b/lms/backend/canvas/testdata/http/fetch-assignments-base.json new file mode 100644 index 00000000..942ca35b --- /dev/null +++ b/lms/backend/canvas/testdata/http/fetch-assignments-base.json @@ -0,0 +1,25 @@ +{ + "URL": "https://canvas.test.com/api/v1/courses/12345/assignments?per_page=75", + "Method": "GET", + "RequestHeaders": { + "Accept": [ + "application/json+canvas-string-ids" + ], + "Authorization": [ + "Bearer ABC123" + ] + }, + "ResponseCode": 200, + "ResponseHeaders": { + "Content-Type": [ + "application/json; charset=utf-8" + ], + "Link": [ + "; rel=\"current\",; rel=\"first\",; rel=\"last\"" + ], + "Status": [ + "200 OK" + ] + }, + "ResponseBody": "[{\"id\":\"98765\",\"description\":\"desc\",\"due_at\":\"2023-10-06T06:59:59Z\",\"unlock_at\":null,\"lock_at\":null,\"points_possible\":100.0,\"grading_type\":\"points\",\"assignment_group_id\":\"124050\",\"grading_standard_id\":null,\"created_at\":\"2023-09-16T05:04:23Z\",\"updated_at\":\"2023-10-11T17:56:12Z\",\"peer_reviews\":false,\"automatic_peer_reviews\":false,\"position\":2,\"grade_group_students_individually\":false,\"anonymous_peer_reviews\":false,\"group_category_id\":null,\"post_to_sis\":false,\"moderated_grading\":false,\"omit_from_final_grade\":false,\"intra_group_peer_reviews\":false,\"anonymous_instructor_annotations\":false,\"anonymous_grading\":false,\"graders_anonymous_to_graders\":false,\"grader_count\":0,\"grader_comments_visible_to_graders\":true,\"final_grader_id\":null,\"grader_names_visible_to_final_grader\":true,\"allowed_attempts\":-1,\"annotatable_attachment_id\":null,\"hide_in_gradebook\":false,\"secure_params\":\"ZZZ\",\"lti_context_id\":\"YYY\",\"course_id\":\"12345\",\"name\":\"Assignment 0\",\"submission_types\":[\"none\"],\"has_submitted_submissions\":false,\"due_date_required\":false,\"max_name_length\":255,\"in_closed_grading_period\":false,\"graded_submissions_exist\":true,\"is_quiz_assignment\":false,\"can_duplicate\":true,\"original_course_id\":null,\"original_assignment_id\":null,\"original_lti_resource_link_id\":null,\"original_assignment_name\":null,\"original_quiz_id\":null,\"workflow_state\":\"published\",\"important_dates\":false,\"muted\":false,\"html_url\":\"https://canvas.test.com/courses/12345/assignments/98765\",\"has_overrides\":false,\"needs_grading_count\":0,\"sis_assignment_id\":null,\"integration_id\":null,\"integration_data\":{},\"published\":true,\"unpublishable\":true,\"only_visible_to_overrides\":false,\"locked_for_user\":false,\"submissions_download_url\":\"https://canvas.test.com/courses/12345/assignments/98765/submissions?zip=1\",\"post_manually\":true,\"anonymize_students\":false,\"require_lockdown_browser\":false,\"restrict_quantitative_data\":false}]" +} diff --git a/lms/backend/canvas/users.go b/lms/backend/canvas/users.go index ff4dc853..3e370ede 100644 --- a/lms/backend/canvas/users.go +++ b/lms/backend/canvas/users.go @@ -39,7 +39,6 @@ func (this *CanvasBackend) fetchUsers(rewriteLinks bool) ([]*lmstypes.User, erro } body, responseHeaders, err := common.GetWithHeaders(url, headers); - if (err != nil) { return nil, fmt.Errorf("Failed to fetch users: '%w'.", err); } diff --git a/lms/backend/test/backend.go b/lms/backend/test/backend.go index 7463181e..d5712f51 100644 --- a/lms/backend/test/backend.go +++ b/lms/backend/test/backend.go @@ -42,6 +42,10 @@ func ClearUsersModifier() { usersModifier = nil; } +func (this *TestLMSBackend) FetchAssignments() ([]*lmstypes.Assignment, error) { + return nil, nil; +} + func (this *TestLMSBackend) FetchAssignment(assignmentID string) (*lmstypes.Assignment, error) { return nil, nil; } diff --git a/lms/lms.go b/lms/lms.go index b84251a7..5f8c2cb5 100644 --- a/lms/lms.go +++ b/lms/lms.go @@ -10,6 +10,7 @@ import ( ) type lmsBackend interface { + FetchAssignments() ([]*lmstypes.Assignment, error) FetchAssignment(assignmentID string) (*lmstypes.Assignment, error) UpdateComments(assignmentID string, comments []*lmstypes.SubmissionComment) error @@ -58,6 +59,15 @@ func FetchAssignment(course *model.Course, assignmentID string) (*lmstypes.Assig return backend.FetchAssignment(assignmentID); } +func FetchAssignments(course *model.Course) ([]*lmstypes.Assignment, error) { + backend, err := getBackend(course); + if (err != nil) { + return nil, err; + } + + return backend.FetchAssignments(); +} + func UpdateComments(course *model.Course, assignmentID string, comments []*lmstypes.SubmissionComment) error { backend, err := getBackend(course); if (err != nil) { diff --git a/model/lms.go b/model/lms.go index 60a46a24..b140513f 100644 --- a/model/lms.go +++ b/model/lms.go @@ -19,9 +19,12 @@ type LMSAdapter struct { BaseURL string `json:"base-url,omitempty"` // Behavior options. + SyncUserAttributes bool `json:"sync-user-attributes,omitempty"` - SyncAddUsers bool `json:"sync-add-users,omitempty"` - SyncRemoveUsers bool `json:"sync-remove-users,omitempty"` + SyncUserAdds bool `json:"sync-user-adds,omitempty"` + SyncUserRemoves bool `json:"sync-user-removes,omitempty"` + + SyncAssignments bool `json:"sync-assignments,omitempty"` } func (this *LMSAdapter) Validate() error { diff --git a/model/sync.go b/model/sync.go index 730686b0..f06d10e4 100644 --- a/model/sync.go +++ b/model/sync.go @@ -4,6 +4,10 @@ import ( "fmt" ) +type LMSSyncResult struct { + UserSync *UserSyncResult `json:"user-sync"` +} + type UserSyncResult struct { Add []*User Mod []*User diff --git a/procedures/lms_sync.go b/procedures/lms_sync.go index c835e591..ea4dc64e 100644 --- a/procedures/lms_sync.go +++ b/procedures/lms_sync.go @@ -138,7 +138,7 @@ func resolveUserSync(course *model.Course, localUsers map[string]*model.User, // Add. if (localUser == nil) { - if (!adapter.SyncAddUsers) { + if (!adapter.SyncUserAdds) { return nil, nil; } @@ -167,7 +167,7 @@ func resolveUserSync(course *model.Course, localUsers map[string]*model.User, // Del. if (lmsUser == nil) { - if (!adapter.SyncRemoveUsers) { + if (!adapter.SyncUserRemoves) { return &model.UserResolveResult{Unchanged: localUser}, nil; } diff --git a/procedures/lms_sync_test.go b/procedures/lms_sync_test.go index 8f7f5a83..438df80c 100644 --- a/procedures/lms_sync_test.go +++ b/procedures/lms_sync_test.go @@ -37,8 +37,8 @@ func TestCourseSyncLMSUsers(test *testing.T) { course := db.MustGetTestCourse(); course.GetLMSAdapter().SyncUserAttributes = testCase.syncAttributes; - course.GetLMSAdapter().SyncAddUsers = testCase.syncAdd; - course.GetLMSAdapter().SyncRemoveUsers = testCase.syncDel; + course.GetLMSAdapter().SyncUserAdds = testCase.syncAdd; + course.GetLMSAdapter().SyncUserRemoves = testCase.syncDel; localUsers := db.MustGetUsers(course); From fde7b07057ab7716301edf5d607fc42030740ef6 Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Sat, 30 Dec 2023 16:09:45 +0700 Subject: [PATCH 14/16] Changed the submission response so that the reject reason is now a general message. --- api/submission/submit.go | 4 ++-- api/submission/submit_test.go | 8 ++++---- 2 files changed, 6 insertions(+), 6 deletions(-) diff --git a/api/submission/submit.go b/api/submission/submit.go index e68a08de..17aa4a02 100644 --- a/api/submission/submit.go +++ b/api/submission/submit.go @@ -18,7 +18,7 @@ type SubmitRequest struct { type SubmitResponse struct { Rejected bool `json:"rejected"` - RejectReason string `json:"reject-reason"` + Message string `json:"message"` GradingSucess bool `json:"grading-success"` GradingInfo *model.GradingInfo `json:"result"` @@ -44,7 +44,7 @@ func HandleSubmit(request *SubmitRequest) (*SubmitResponse, *core.APIError) { if (reject != nil) { response.Rejected = true; - response.RejectReason = reject.String(); + response.Message = reject.String(); return &response, nil; } diff --git a/api/submission/submit_test.go b/api/submission/submit_test.go index 5d0b41dc..56ce47c8 100644 --- a/api/submission/submit_test.go +++ b/api/submission/submit_test.go @@ -45,7 +45,7 @@ func TestSubmit(test *testing.T) { continue; } - if (responseContent.RejectReason != "") { + if (responseContent.Message != "") { test.Errorf("Case %d: Response has a reject reason when it should not: '%v'.", i, responseContent); continue; } @@ -100,13 +100,13 @@ func TestRejectSubmissionMaxAttempts(test *testing.T) { test.Fatalf("Response is not rejected when it should be: '%v'.", responseContent); } - if (responseContent.RejectReason == "") { + if (responseContent.Message == "") { test.Fatalf("Response does not have a reject reason when it should: '%v'.", responseContent); } expected := (&grader.RejectMaxAttempts{0}).String(); - if (expected != responseContent.RejectReason) { + if (expected != responseContent.Message) { test.Fatalf("Did not get the expected rejection reason. Expected: '%s', Actual: '%s'.", - expected, responseContent.RejectReason); + expected, responseContent.Message); } } From 7767da99fbed8f043b77312d3302927c459299ce Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Sat, 30 Dec 2023 16:43:28 +0700 Subject: [PATCH 15/16] Reset the API locator numbers (now that the API has stabalized). --- api/admin/update_course.go | 8 ++--- api/core/auth.go | 6 ++-- api/core/auth_test.go | 22 ++++++------- api/core/request.go | 24 +++++++------- api/core/request_fields.go | 18 +++++----- api/core/request_fields_test.go | 30 ++++++++--------- api/core/routing.go | 22 ++++++------- api/core/routing_test.go | 42 ++++++++++++------------ api/lms/sync_users.go | 4 +-- api/lms/sync_users_test.go | 2 +- api/lms/upload_scores.go | 4 +-- api/lms/upload_scores_test.go | 4 +-- api/lms/user_get.go | 4 +-- api/lms/user_get_test.go | 4 +-- api/submission/fetch_scores.go | 2 +- api/submission/fetch_scores_test.go | 2 +- api/submission/fetch_submission.go | 2 +- api/submission/fetch_submission_test.go | 2 +- api/submission/fetch_submissions.go | 2 +- api/submission/fetch_submissions_test.go | 2 +- api/submission/history.go | 2 +- api/submission/history_test.go | 2 +- api/submission/peek.go | 2 +- api/submission/peek_test.go | 2 +- api/submission/remove_submission.go | 2 +- api/submission/remove_submission_test.go | 2 +- api/user/add.go | 4 +-- api/user/add_test.go | 2 +- api/user/change_password.go | 8 ++--- api/user/change_password_test.go | 4 +-- api/user/get_test.go | 4 +-- api/user/remove.go | 4 +-- api/user/remove_test.go | 4 +-- 33 files changed, 124 insertions(+), 124 deletions(-) diff --git a/api/admin/update_course.go b/api/admin/update_course.go index 358f78f2..5563d22d 100644 --- a/api/admin/update_course.go +++ b/api/admin/update_course.go @@ -23,7 +23,7 @@ func HandleUpdateCourse(request *UpdateCourseRequest) (*UpdateCourseResponse, *c if (request.Clear) { err := db.ClearCourse(request.Course); if (err != nil) { - return nil, core.NewInternalError("-701", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-201", &request.APIRequestCourseUserContext, "Failed to clear course.").Err(err); } } @@ -31,7 +31,7 @@ func HandleUpdateCourse(request *UpdateCourseRequest) (*UpdateCourseResponse, *c if (request.Source != "") { spec, err := common.ParseFileSpec(request.Source); if (err != nil) { - return nil, core.NewBadCourseRequestError("-702", &request.APIRequestCourseUserContext, + return nil, core.NewBadCourseRequestError("-202", &request.APIRequestCourseUserContext, "Source FileSpec is not formatted properly.").Err(err); } @@ -39,14 +39,14 @@ func HandleUpdateCourse(request *UpdateCourseRequest) (*UpdateCourseResponse, *c err = db.SaveCourse(request.Course); if (err != nil) { - return nil, core.NewInternalError("-703", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-203", &request.APIRequestCourseUserContext, "Failed to save course.").Err(err); } } updated, err := procedures.UpdateCourse(request.Course); if (err != nil) { - return nil, core.NewInternalError("-704", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-204", &request.APIRequestCourseUserContext, "Failed to update course.").Err(err); } diff --git a/api/core/auth.go b/api/core/auth.go index 86d4b267..15a2762e 100644 --- a/api/core/auth.go +++ b/api/core/auth.go @@ -14,11 +14,11 @@ import ( func (this *APIRequestCourseUserContext) Auth() (*model.User, *APIError) { user, err := db.GetUser(this.Course, this.UserEmail); if (err != nil) { - return nil, NewAuthBadRequestError("-201", this, "Cannot Get User").Err(err); + return nil, NewAuthBadRequestError("-012", this, "Cannot Get User").Err(err); } if (user == nil) { - return nil, NewAuthBadRequestError("-202", this, "Unknown User"); + return nil, NewAuthBadRequestError("-013", this, "Unknown User"); } if (config.NO_AUTH.Get()) { @@ -27,7 +27,7 @@ func (this *APIRequestCourseUserContext) Auth() (*model.User, *APIError) { } if (!user.CheckPassword(this.UserPass)) { - return nil, NewAuthBadRequestError("-203", this, "Bad Password"); + return nil, NewAuthBadRequestError("-014", this, "Bad Password"); } return user, nil; diff --git a/api/core/auth_test.go b/api/core/auth_test.go index d8c517e5..9725ad7d 100644 --- a/api/core/auth_test.go +++ b/api/core/auth_test.go @@ -20,14 +20,14 @@ func TestAuth(test *testing.T) { {"student@test.com", "student", false, ""}, {"other@test.com", "other", false, ""}, - {"Z", "student", false, "-202"}, - {"Zstudent@test.com", "student", false, "-202"}, - {"student@test.comZ", "student", false, "-202"}, - {"student", "student", false, "-202"}, + {"Z", "student", false, "-013"}, + {"Zstudent@test.com", "student", false, "-013"}, + {"student@test.comZ", "student", false, "-013"}, + {"student", "student", false, "-013"}, - {"student@test.com", "", false, "-203"}, - {"student@test.com", "Zstudent", false, "-203"}, - {"student@test.com", "studentZ", false, "-203"}, + {"student@test.com", "", false, "-014"}, + {"student@test.com", "Zstudent", false, "-014"}, + {"student@test.com", "studentZ", false, "-014"}, {"owner@test.com", "owner", true, ""}, {"admin@test.com", "admin", true, ""}, @@ -35,10 +35,10 @@ func TestAuth(test *testing.T) { {"student@test.com", "student", true, ""}, {"other@test.com", "other", true, ""}, - {"Z", "student", true, "-202"}, - {"Zstudent@test.com", "student", true, "-202"}, - {"student@test.comZ", "student", true, "-202"}, - {"student", "student", true, "-202"}, + {"Z", "student", true, "-013"}, + {"Zstudent@test.com", "student", true, "-013"}, + {"student@test.comZ", "student", true, "-013"}, + {"student", "student", true, "-013"}, {"student@test.com", "", true, ""}, {"student@test.com", "Zstudent", true, ""}, diff --git a/api/core/request.go b/api/core/request.go index 6a0de2f1..dded6cc3 100644 --- a/api/core/request.go +++ b/api/core/request.go @@ -71,25 +71,25 @@ func (this *APIRequestCourseUserContext) Validate(request any, endpoint string) } if (this.CourseID == "") { - return NewBadRequestError("-301", &this.APIRequest, "No course ID specified."); + return NewBadRequestError("-015", &this.APIRequest, "No course ID specified."); } if (this.UserEmail == "") { - return NewBadRequestError("-302", &this.APIRequest, "No user email specified."); + return NewBadRequestError("-016", &this.APIRequest, "No user email specified."); } if (this.UserPass == "") { - return NewBadRequestError("-303", &this.APIRequest, "No user password specified."); + return NewBadRequestError("-017", &this.APIRequest, "No user password specified."); } var err error; this.Course, err = db.GetCourse(this.CourseID); if (err != nil) { - return NewInternalError("-318", this, "Unable to get course").Err(err); + return NewInternalError("-032", this, "Unable to get course").Err(err); } if (this.Course == nil) { - return NewBadRequestError("-304", &this.APIRequest, fmt.Sprintf("Could not find course: '%s'.", this.CourseID)). + return NewBadRequestError("-018", &this.APIRequest, fmt.Sprintf("Could not find course: '%s'.", this.CourseID)). Add("course-id", this.CourseID); } @@ -100,11 +100,11 @@ func (this *APIRequestCourseUserContext) Validate(request any, endpoint string) minRole, foundRole := getMaxRole(request); if (!foundRole) { - return NewInternalError("-305", this, "No role found for request. All request structs require a minimum role."); + return NewInternalError("-019", this, "No role found for request. All request structs require a minimum role."); } if (this.User.Role < minRole) { - return NewBadPermissionsError("-306", this, minRole, "Base API Request"); + return NewBadPermissionsError("-020", this, minRole, "Base API Request"); } return nil; @@ -118,12 +118,12 @@ func (this *APIRequestAssignmentContext) Validate(request any, endpoint string) } if (this.AssignmentID == "") { - return NewBadRequestError("-307", &this.APIRequest, "No assignment ID specified."); + return NewBadRequestError("-021", &this.APIRequest, "No assignment ID specified."); } this.Assignment = this.Course.GetAssignment(this.AssignmentID); if (this.Assignment == nil) { - return NewBadRequestError("-308", &this.APIRequest, fmt.Sprintf("Could not find assignment: '%s'.", this.AssignmentID)). + return NewBadRequestError("-022", &this.APIRequest, fmt.Sprintf("Could not find assignment: '%s'.", this.AssignmentID)). Add("course-id", this.CourseID).Add("assignment-id", this.AssignmentID); } @@ -135,7 +135,7 @@ func (this *APIRequestAssignmentContext) Validate(request any, endpoint string) func ValidateAPIRequest(request *http.Request, apiRequest any, endpoint string) *APIError { reflectPointer := reflect.ValueOf(apiRequest); if (reflectPointer.Kind() != reflect.Pointer) { - return NewBareInternalError("-309", endpoint, "ValidateAPIRequest() must be called with a pointer."). + return NewBareInternalError("-023", endpoint, "ValidateAPIRequest() must be called with a pointer."). Add("kind", reflectPointer.Kind().String()); } @@ -146,7 +146,7 @@ func ValidateAPIRequest(request *http.Request, apiRequest any, endpoint string) } if (!foundRequestStruct) { - return NewBareInternalError("-310", endpoint, "Request is not any kind of known API request."); + return NewBareInternalError("-024", endpoint, "Request is not any kind of known API request."); } // Check for any special field types that we know how to populate. @@ -185,7 +185,7 @@ func validateRequestStruct(request any, endpoint string) (bool, *APIError) { reflectValue := reflect.ValueOf(request).Elem(); if (reflectValue.Kind() != reflect.Struct) { - return false, NewBareInternalError("-317", endpoint, "Request's type must be a struct."). + return false, NewBareInternalError("-031", endpoint, "Request's type must be a struct."). Add("kind", reflectValue.Kind().String()); } diff --git a/api/core/request_fields.go b/api/core/request_fields.go index 704f220f..b6f2e6ab 100644 --- a/api/core/request_fields.go +++ b/api/core/request_fields.go @@ -131,7 +131,7 @@ func checkRequestTargetUser(endpoint string, apiRequest any, fieldIndex int) *AP jsonName := util.JSONFieldName(fieldType); if (field.Email == "") { - return NewBadRequestError("-320", &courseContext.APIRequest, + return NewBadRequestError("-034", &courseContext.APIRequest, fmt.Sprintf("Field '%s' requires a non-empty string, empty or null provided.", jsonName)). Add("struct-name", structName).Add("field-name", fieldType.Name).Add("json-name", jsonName); } @@ -173,7 +173,7 @@ func checkRequestTargetUserSelfOrRole(endpoint string, apiRequest any, fieldInde // Operations not on self require higher permissions. if ((field.Email != courseContext.User.Email) && (courseContext.User.Role < minRole)) { - return NewBadPermissionsError("-319", courseContext, minRole, "Non-Self Target User"); + return NewBadPermissionsError("-033", courseContext, minRole, "Non-Self Target User"); } user := users[field.Email]; @@ -198,19 +198,19 @@ func checkRequestPostFiles(request *http.Request, endpoint string, apiRequest an fieldType := reflectValue.Type().Field(fieldIndex); if (!fieldType.IsExported()) { - return NewBareInternalError("-314", endpoint, "A POSTFiles field must be exported."). + return NewBareInternalError("-028", endpoint, "A POSTFiles field must be exported."). Add("struct-name", structName).Add("field-name", fieldType.Name); } postFiles, err := storeRequestFiles(request); if (err != nil) { - return NewBareInternalError("-315", endpoint, "Failed to store files from POST.").Err(err). + return NewBareInternalError("-029", endpoint, "Failed to store files from POST.").Err(err). Add("struct-name", structName).Add("field-name", fieldType.Name); } if (postFiles == nil) { - return NewBareBadRequestError("-316", endpoint, "Endpoint requires files to be provided in POST body, no files found."). + return NewBareBadRequestError("-030", endpoint, "Endpoint requires files to be provided in POST body, no files found."). Add("struct-name", structName).Add("field-name", fieldType.Name); } @@ -230,7 +230,7 @@ func checkRequestNonEmptyString(endpoint string, apiRequest any, fieldIndex int) value := fieldValue.Interface().(NonEmptyString); if (value == "") { - return NewBareBadRequestError("-318", endpoint, + return NewBareBadRequestError("-032", endpoint, fmt.Sprintf("Field '%s' requires a non-empty string, empty or null provided.", jsonName)). Add("struct-name", structName).Add("field-name", fieldType.Name).Add("json-name", jsonName); } @@ -327,7 +327,7 @@ func baseCheckRequestUsersField(endpoint string, apiRequest any, fieldIndex int) courseContextValue := reflectValue.FieldByName("APIRequestCourseUserContext"); if (!courseContextValue.IsValid() || courseContextValue.IsZero()) { return nil, nil, - NewBareInternalError("-311", endpoint, "A request with type requiring users must embed APIRequestCourseUserContext"). + NewBareInternalError("-025", endpoint, "A request with type requiring users must embed APIRequestCourseUserContext"). Add("request", apiRequest). Add("struct-name", structName).Add("field-name", fieldType.Name).Add("field-type", fieldName); } @@ -335,14 +335,14 @@ func baseCheckRequestUsersField(endpoint string, apiRequest any, fieldIndex int) if (!fieldType.IsExported()) { return nil, nil, - NewInternalError("-312", &courseContext, "Field must be exported."). + NewInternalError("-026", &courseContext, "Field must be exported."). Add("struct-name", structName).Add("field-name", fieldType.Name).Add("field-type", fieldName); } users, err := db.GetUsers(courseContext.Course); if (err != nil) { return nil, nil, - NewInternalError("-313", &courseContext, "Failed to fetch embeded users.").Err(err). + NewInternalError("-027", &courseContext, "Failed to fetch embeded users.").Err(err). Add("struct-name", structName).Add("field-name", fieldType.Name).Add("field-type", fieldName); } diff --git a/api/core/request_fields_test.go b/api/core/request_fields_test.go index fb3aa66d..f029af46 100644 --- a/api/core/request_fields_test.go +++ b/api/core/request_fields_test.go @@ -31,8 +31,8 @@ func TestBadUsersFieldNoContext(test *testing.T) { i, testCase.request); } - if (apiErr.Locator != "-311") { - test.Fatalf("Case %d: Struct with no course context does not return an error with locator '-311', found '%s': '%+v.", + if (apiErr.Locator != "-025") { + test.Fatalf("Case %d: Struct with no course context does not return an error with locator '-025', found '%s': '%+v.", i, apiErr.Locator, testCase.request); } } @@ -78,8 +78,8 @@ func TestBadUsersFieldNotExported(test *testing.T) { i, testCase.request); } - if (apiErr.Locator != "-312") { - test.Fatalf("Case %d: Struct with non-exported field does not return an error with locator '-312', found '%s': '%v.", + if (apiErr.Locator != "-026") { + test.Fatalf("Case %d: Struct with non-exported field does not return an error with locator '-026', found '%s': '%v.", i, apiErr.Locator, apiErr); } } @@ -91,13 +91,13 @@ func TestNonEmptyStringField(test *testing.T) { {&struct{ APIRequest; Text NonEmptyString }{Text: "ZZZ"}, "", "Text"}, - {&struct{ APIRequest; Text NonEmptyString }{}, "-318", "Text"}, - {&struct{ APIRequest; Text NonEmptyString }{Text: ""}, "-318", "Text"}, + {&struct{ APIRequest; Text NonEmptyString }{}, "-032", "Text"}, + {&struct{ APIRequest; Text NonEmptyString }{Text: ""}, "-032", "Text"}, - {&struct{ APIRequest; Text NonEmptyString `json:"text"`}{}, "-318", "text"}, - {&struct{ APIRequest; Text NonEmptyString `json:"text,omitempty"`}{}, "-318", "text"}, - {&struct{ APIRequest; Text NonEmptyString `json:"foo-bar"`}{}, "-318", "foo-bar"}, - {&struct{ APIRequest; Text NonEmptyString `json:"foo-bar,omitempty"`}{}, "-318", "foo-bar"}, + {&struct{ APIRequest; Text NonEmptyString `json:"text"`}{}, "-032", "text"}, + {&struct{ APIRequest; Text NonEmptyString `json:"text,omitempty"`}{}, "-032", "text"}, + {&struct{ APIRequest; Text NonEmptyString `json:"foo-bar"`}{}, "-032", "foo-bar"}, + {&struct{ APIRequest; Text NonEmptyString `json:"foo-bar,omitempty"`}{}, "-032", "foo-bar"}, }; for i, testCase := range testCases { @@ -197,7 +197,7 @@ func TestBadPostFilesFieldNotExported(test *testing.T) { test.Fatalf("Struct with non-exported files does not return an error,"); } - expectedLocator := "-314"; + expectedLocator := "-028"; if (apiErr.Locator != expectedLocator) { test.Fatalf("Struct with non-exported files does not return an error with the correct locator. Expcted '%s', found '%s'.", expectedLocator, apiErr.Locator); @@ -227,7 +227,7 @@ func TestBadPostFilesNoFiles(test *testing.T) { test.Fatalf("Request did not generate an error: '%v'.", response); } - expectedLocator := "-316"; + expectedLocator := "-030"; if (response.Locator != expectedLocator) { test.Fatalf("Error does not have the correct locator. Expcted '%s', found '%s'.", expectedLocator, response.Locator); @@ -263,7 +263,7 @@ func TestBadPostFilesStoreFail(test *testing.T) { test.Fatalf("Request did not generate an error: '%v'.", response); } - expectedLocator := "-315"; + expectedLocator := "-029"; if (response.Locator != expectedLocator) { test.Fatalf("Error does not have the correct locator. Expcted '%s', found '%s'.", expectedLocator, response.Locator); @@ -453,7 +453,7 @@ func testTargetUser[T comparable, V userGetter](test *testing.T, apiErr := ValidateAPIRequest(nil, request, ""); if (apiErr != nil) { if (testCase.permError) { - expectedLocator := "-319"; + expectedLocator := "-033"; if (expectedLocator != apiErr.Locator) { test.Errorf("Case %d: Incorrect error returned on permissions error. Expcted '%s', found '%s'.", i, expectedLocator, apiErr.Locator); @@ -517,7 +517,7 @@ func TestTargetUser(test *testing.T) { apiErr := ValidateAPIRequest(nil, &request, ""); if (apiErr != nil) { if (testCase.target == "") { - expectedLocator := "-320"; + expectedLocator := "-034"; if (expectedLocator != apiErr.Locator) { test.Errorf("Case %d: Incorrect error returned on empty string. Expcted '%s', found '%s'.", i, expectedLocator, apiErr.Locator); diff --git a/api/core/routing.go b/api/core/routing.go index 36af6c58..ba13c88d 100644 --- a/api/core/routing.go +++ b/api/core/routing.go @@ -112,7 +112,7 @@ func NewAPIRoute(pattern string, apiHandler any) *Route { log.Error().Any("value", value).Str("endpoint", request.URL.Path). Msg("Recovered from a panic when handling an API endpoint."); - apiErr := NewBareInternalError("-101", request.URL.Path, "Recovered from a panic when handling an API endpoint."). + apiErr := NewBareInternalError("-001", request.URL.Path, "Recovered from a panic when handling an API endpoint."). Add("value", value); err = sendAPIResponse(nil, response, nil, apiErr, false); @@ -171,7 +171,7 @@ func sendAPIResponse(apiRequest ValidAPIRequest, response http.ResponseWriter, payload, err := util.ToJSON(apiResponse); if (err != nil) { - apiErr = NewBareInternalError("-102", "", "Could not serialize API response.").Err(err); + apiErr = NewBareInternalError("-002", "", "Could not serialize API response.").Err(err); apiResponse = apiErr.ToResponse(); if (hardFail) { @@ -210,7 +210,7 @@ func createAPIRequest(request *http.Request, apiHandler ValidAPIHandler) (ValidA if (strings.Contains(strings.Join(request.Header["Content-Type"], " "), "multipart/form-data")) { err := request.ParseMultipartForm(MAX_FORM_MEM_SIZE_BYTES); if (err != nil) { - return nil, NewBareBadRequestError("-103", endpoint, + return nil, NewBareBadRequestError("-003", endpoint, fmt.Sprintf("POST request is improperly formatted.")). Err(err); } @@ -219,14 +219,14 @@ func createAPIRequest(request *http.Request, apiHandler ValidAPIHandler) (ValidA // Get the text from the POST. textContent := request.PostFormValue(API_REQUEST_CONTENT_KEY); if (textContent == "") { - return nil, NewBareBadRequestError("-104", endpoint, + return nil, NewBareBadRequestError("-004", endpoint, fmt.Sprintf("JSON payload for POST form key '%s' is empty.", API_REQUEST_CONTENT_KEY)); } // Unmarshal the JSON. err := util.JSONFromString(textContent, apiRequest); if (err != nil) { - return nil, NewBareBadRequestError("-105", endpoint, + return nil, NewBareBadRequestError("-005", endpoint, fmt.Sprintf("JSON payload for POST form key '%s' is not valid JSON.", API_REQUEST_CONTENT_KEY)). Err(err); } @@ -267,39 +267,39 @@ func validateAPIHandler(endpoint string, apiHandler any) (ValidAPIHandler, *APIE reflectType := reflect.TypeOf(apiHandler); if (reflectValue.Kind() != reflect.Func) { - return nil, NewBareInternalError("-106", endpoint, "API handler is not a function."). + return nil, NewBareInternalError("-006", endpoint, "API handler is not a function."). Add("kind", reflectValue.Kind().String()); } funcInfo := getFuncInfo(apiHandler); if (reflectType.NumIn() != 1) { - return nil, NewBareInternalError("-107", endpoint, "API handler does not have exactly 1 argument."). + return nil, NewBareInternalError("-007", endpoint, "API handler does not have exactly 1 argument."). Add("num-in", reflectType.NumIn()). Add("function-info", funcInfo); } argumentType := reflectType.In(0); if (argumentType.Kind() != reflect.Pointer) { - return nil, NewBareInternalError("-108", endpoint, "API handler's argument is not a pointer."). + return nil, NewBareInternalError("-008", endpoint, "API handler's argument is not a pointer."). Add("kind", argumentType.Kind().String()). Add("function-info", funcInfo); } if (reflectType.NumOut() != 2) { - return nil, NewBareInternalError("-109", endpoint, "API handler does not return exactly 2 arguments."). + return nil, NewBareInternalError("-009", endpoint, "API handler does not return exactly 2 arguments."). Add("num-out", reflectType.NumOut()). Add("function-info", funcInfo); } if (reflectType.Out(0).Kind() != reflect.Pointer) { - return nil, NewBareInternalError("-110", endpoint, "API handler's first return value is not a pointer."). + return nil, NewBareInternalError("-010", endpoint, "API handler's first return value is not a pointer."). Add("kind", reflectType.Out(0).Kind().String()). Add("function-info", funcInfo); } if (reflectType.Out(1) != reflect.TypeOf((*APIError)(nil))) { - return nil, NewBareInternalError("-111", endpoint, "API handler's second return value is a *APIError."). + return nil, NewBareInternalError("-011", endpoint, "API handler's second return value is a *APIError."). Add("type", reflectType.Out(1).String()). Add("function-info", funcInfo); } diff --git a/api/core/routing_test.go b/api/core/routing_test.go index 96c7fdf5..4735983f 100644 --- a/api/core/routing_test.go +++ b/api/core/routing_test.go @@ -27,7 +27,7 @@ func TestAPIPanic(test *testing.T) { routes = append(routes, NewAPIRoute(endpoint, handler)); response := SendTestAPIRequest(test, endpoint, nil); - if (response.Locator != "-101") { + if (response.Locator != "-001") { test.Fatalf("Response does not have panic locator of '-501', actual locator: '%s'.", response.Locator); } } @@ -37,20 +37,20 @@ func TestAPIPanic(test *testing.T) { func TestMalformedHandlers(test *testing.T) { // Define all the handlers. testCases := []struct{handler any; locator string}{ - {"", "-106"}, - {nil, "-106"}, - {0, "-106"}, - {func() (*any, *APIError) { return nil, nil }, "-107"}, - {func(request *BaseTestRequest, testarg int) (*any, *APIError) { return nil, nil }, "-107"}, - {func(request BaseTestRequest) (*any, *APIError) { return nil, nil }, "-108"}, - {func(request int) (*any, *APIError) { return nil, nil }, "-108"}, - {func(request *BaseTestRequest) (*any) { return nil }, "-109"}, - {func(request *BaseTestRequest) (int, *any, *APIError) { return 0, nil, nil }, "-109"}, - {func(request *BaseTestRequest) (any, *APIError) { return nil, nil }, "-110"}, - {func(request *BaseTestRequest) (int, *APIError) { return 0, nil }, "-110"}, - {func(request *BaseTestRequest) (*any, APIError) { return nil, APIError{} }, "-111"}, - {func(request *BaseTestRequest) (*any, any) { return nil, nil }, "-111"}, - {func(request *BaseTestRequest) (*any, int) { return nil, 0}, "-111"}, + {"", "-006"}, + {nil, "-006"}, + {0, "-006"}, + {func() (*any, *APIError) { return nil, nil }, "-007"}, + {func(request *BaseTestRequest, testarg int) (*any, *APIError) { return nil, nil }, "-007"}, + {func(request BaseTestRequest) (*any, *APIError) { return nil, nil }, "-008"}, + {func(request int) (*any, *APIError) { return nil, nil }, "-008"}, + {func(request *BaseTestRequest) (*any) { return nil }, "-009"}, + {func(request *BaseTestRequest) (int, *any, *APIError) { return 0, nil, nil }, "-009"}, + {func(request *BaseTestRequest) (any, *APIError) { return nil, nil }, "-010"}, + {func(request *BaseTestRequest) (int, *APIError) { return 0, nil }, "-010"}, + {func(request *BaseTestRequest) (*any, APIError) { return nil, APIError{} }, "-011"}, + {func(request *BaseTestRequest) (*any, any) { return nil, nil }, "-011"}, + {func(request *BaseTestRequest) (*any, int) { return nil, 0}, "-011"}, }; for i, testCase := range testCases { @@ -69,11 +69,11 @@ func TestMalformedHandlers(test *testing.T) { func TestBadRequestEmptyContent(test *testing.T) { // Define all the content that will go in the post form. testCases := []struct{form map[string]string; locator string}{ - {map[string]string{}, "-104"}, - {map[string]string{API_REQUEST_CONTENT_KEY: ``}, "-104"}, - {map[string]string{API_REQUEST_CONTENT_KEY: `Z`}, "-105"}, - {map[string]string{API_REQUEST_CONTENT_KEY: `1`}, "-105"}, - {map[string]string{API_REQUEST_CONTENT_KEY: `[]`}, "-105"}, + {map[string]string{}, "-004"}, + {map[string]string{API_REQUEST_CONTENT_KEY: ``}, "-004"}, + {map[string]string{API_REQUEST_CONTENT_KEY: `Z`}, "-005"}, + {map[string]string{API_REQUEST_CONTENT_KEY: `1`}, "-005"}, + {map[string]string{API_REQUEST_CONTENT_KEY: `[]`}, "-005"}, }; endpoint := `/test/api/bad-request/empty-content`; @@ -121,7 +121,7 @@ func TestNonMarshalableResponse(test *testing.T) { routes = append(routes, NewAPIRoute(endpoint, handler)); response := SendTestAPIRequest(test, endpoint, nil); - if (response.Locator != "-102") { + if (response.Locator != "-002") { test.Fatalf("Response does not locator of '-531', actual locator: '%s'.", response.Locator); } } diff --git a/api/lms/sync_users.go b/api/lms/sync_users.go index a3894acf..90b96cda 100644 --- a/api/lms/sync_users.go +++ b/api/lms/sync_users.go @@ -19,13 +19,13 @@ type SyncUsersResponse struct { func HandleSyncUsers(request *SyncUsersRequest) (*SyncUsersResponse, *core.APIError) { if (request.Course.GetLMSAdapter() == nil) { - return nil, core.NewBadRequestError("-503", &request.APIRequest, "Course is not linked to an LMS."). + return nil, core.NewBadRequestError("-403", &request.APIRequest, "Course is not linked to an LMS."). Add("course", request.Course.GetID()); } result, err := procedures.SyncAllLMSUsers(request.Course, request.DryRun, !request.SkipEmails); if (err != nil) { - return nil, core.NewInternalError("-504", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-404", &request.APIRequestCourseUserContext, "Failed to sync LMS users.").Err(err); } diff --git a/api/lms/sync_users_test.go b/api/lms/sync_users_test.go index 79616591..a312cf61 100644 --- a/api/lms/sync_users_test.go +++ b/api/lms/sync_users_test.go @@ -31,7 +31,7 @@ func TestAPISyncUsers(test *testing.T) { response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`lms/sync/users`), fields, nil, testCase.role); if (!response.Success) { if (testCase.permError) { - expectedLocator := "-306"; + expectedLocator := "-020"; if (response.Locator != expectedLocator) { test.Errorf("Case %d: Incorrect error returned on permissions error. Expcted '%s', found '%s'.", i, expectedLocator, response.Locator); diff --git a/api/lms/upload_scores.go b/api/lms/upload_scores.go index bfe5e01a..0d4b9d72 100644 --- a/api/lms/upload_scores.go +++ b/api/lms/upload_scores.go @@ -35,7 +35,7 @@ type RowEntry struct { func HandleUploadScores(request *UploadScoresRequest) (*UploadScoresResponse, *core.APIError) { if (request.Course.GetLMSAdapter() == nil) { - return nil, core.NewBadRequestError("-505", &request.APIRequest, "Course is not linked to an LMS."). + return nil, core.NewBadRequestError("-405", &request.APIRequest, "Course is not linked to an LMS."). Add("course", request.Course.GetID()); } @@ -48,7 +48,7 @@ func HandleUploadScores(request *UploadScoresRequest) (*UploadScoresResponse, *c err := lms.UpdateAssignmentScores(request.Course, string(request.AssignmentLMSID), scores); if (err != nil) { - return nil, core.NewInternalError("-506", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-406", &request.APIRequestCourseUserContext, "Failed to upload LMS scores.").Err(err); } diff --git a/api/lms/upload_scores_test.go b/api/lms/upload_scores_test.go index 8183e292..32cf3cc9 100644 --- a/api/lms/upload_scores_test.go +++ b/api/lms/upload_scores_test.go @@ -128,9 +128,9 @@ func TestUploadScores(test *testing.T) { if (!response.Success) { expectedLocator := ""; if (testCase.permError) { - expectedLocator = "-306"; + expectedLocator = "-020"; } else if (testCase.failUpdate) { - expectedLocator = "-506"; + expectedLocator = "-406"; } if (expectedLocator == "") { diff --git a/api/lms/user_get.go b/api/lms/user_get.go index 6478dd69..4bf92860 100644 --- a/api/lms/user_get.go +++ b/api/lms/user_get.go @@ -20,7 +20,7 @@ type UserGetResponse struct { func HandleUserGet(request *UserGetRequest) (*UserGetResponse, *core.APIError) { if (request.Course.GetLMSAdapter() == nil) { - return nil, core.NewBadRequestError("-501", &request.APIRequest, "Course is not linked to an LMS."). + return nil, core.NewBadRequestError("-401", &request.APIRequest, "Course is not linked to an LMS."). Add("course", request.Course.GetID()); } @@ -35,7 +35,7 @@ func HandleUserGet(request *UserGetRequest) (*UserGetResponse, *core.APIError) { lmsUser, err := lms.FetchUser(request.Course, string(request.TargetUser.Email)); if (err != nil) { - return nil, core.NewInternalError("-502", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-402", &request.APIRequestCourseUserContext, "Failed to fetch LMS user.").Err(err).Add("email", string(request.TargetUser.Email)); } diff --git a/api/lms/user_get_test.go b/api/lms/user_get_test.go index a0c267f0..19312bff 100644 --- a/api/lms/user_get_test.go +++ b/api/lms/user_get_test.go @@ -33,9 +33,9 @@ func TestUserGet(test *testing.T) { if (!response.Success) { expectedLocator := ""; if (testCase.permError) { - expectedLocator = "-306"; + expectedLocator = "-020"; } else if (testCase.target == "") { - expectedLocator = "-320"; + expectedLocator = "-034"; } if (expectedLocator == "") { diff --git a/api/submission/fetch_scores.go b/api/submission/fetch_scores.go index c758f33f..8c595a9b 100644 --- a/api/submission/fetch_scores.go +++ b/api/submission/fetch_scores.go @@ -21,7 +21,7 @@ type FetchScoresResponse struct { func HandleFetchScores(request *FetchScoresRequest) (*FetchScoresResponse, *core.APIError) { submissionInfos, err := db.GetRecentSubmissionSurvey(request.Assignment, request.FilterRole); if (err != nil) { - return nil, core.NewInternalError("-404", &request.APIRequestCourseUserContext, "Failed to get submission summaries.").Err(err); + return nil, core.NewInternalError("-602", &request.APIRequestCourseUserContext, "Failed to get submission summaries.").Err(err); } return &FetchScoresResponse{submissionInfos}, nil; diff --git a/api/submission/fetch_scores_test.go b/api/submission/fetch_scores_test.go index 5ff47470..b8b9d58d 100644 --- a/api/submission/fetch_scores_test.go +++ b/api/submission/fetch_scores_test.go @@ -45,7 +45,7 @@ func TestFetchScores(test *testing.T) { response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`submission/fetch/scores`), fields, nil, testCase.role); if (!response.Success) { if (testCase.permError) { - expectedLocator := "-306"; + expectedLocator := "-020"; if (response.Locator != expectedLocator) { test.Errorf("Case %d: Incorrect error returned on permissions error. Expcted '%s', found '%s'.", i, expectedLocator, response.Locator); diff --git a/api/submission/fetch_submission.go b/api/submission/fetch_submission.go index a10f2cb5..bbd0cfc3 100644 --- a/api/submission/fetch_submission.go +++ b/api/submission/fetch_submission.go @@ -31,7 +31,7 @@ func HandleFetchSubmission(request *FetchSubmissionRequest) (*FetchSubmissionRes gradingResult, err := db.GetSubmissionContents(request.Assignment, request.TargetUser.Email, request.TargetSubmission); if (err != nil) { - return nil, core.NewInternalError("-409", &request.APIRequestCourseUserContext, "Failed to get submission contents."). + return nil, core.NewInternalError("-604", &request.APIRequestCourseUserContext, "Failed to get submission contents."). Err(err).Add("email", request.TargetUser.Email).Add("submission", request.TargetSubmission); } diff --git a/api/submission/fetch_submission_test.go b/api/submission/fetch_submission_test.go index b0593b6d..e3ddab8a 100644 --- a/api/submission/fetch_submission_test.go +++ b/api/submission/fetch_submission_test.go @@ -79,7 +79,7 @@ func TestFetchSubmission(test *testing.T) { response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`submission/fetch/submission`), fields, nil, testCase.role); if (!response.Success) { if (testCase.permError) { - expectedLocator := "-319"; + expectedLocator := "-033"; if (response.Locator != expectedLocator) { test.Errorf("Case %d: Incorrect error returned on permissions error. Expcted '%s', found '%s'.", i, expectedLocator, response.Locator); diff --git a/api/submission/fetch_submissions.go b/api/submission/fetch_submissions.go index 5a8f24a2..e7ecbe10 100644 --- a/api/submission/fetch_submissions.go +++ b/api/submission/fetch_submissions.go @@ -20,7 +20,7 @@ type FetchSubmissionsResponse struct { func HandleFetchSubmissions(request *FetchSubmissionsRequest) (*FetchSubmissionsResponse, *core.APIError) { results, err := db.GetRecentSubmissionContents(request.Assignment, request.FilterRole); if (err != nil) { - return nil, core.NewInternalError("-412", &request.APIRequestCourseUserContext, "Failed to get submissions."). + return nil, core.NewInternalError("-605", &request.APIRequestCourseUserContext, "Failed to get submissions."). Err(err); } diff --git a/api/submission/fetch_submissions_test.go b/api/submission/fetch_submissions_test.go index a74e0835..e0a16323 100644 --- a/api/submission/fetch_submissions_test.go +++ b/api/submission/fetch_submissions_test.go @@ -33,7 +33,7 @@ func TestFetchSubmissions(test *testing.T) { response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`submission/fetch/submissions`), nil, nil, testCase.role); if (!response.Success) { if (testCase.permError) { - expectedLocator := "-306"; + expectedLocator := "-020"; if (response.Locator != expectedLocator) { test.Errorf("Case %d: Incorrect error returned on permissions error. Expcted '%s', found '%s'.", i, expectedLocator, response.Locator); diff --git a/api/submission/history.go b/api/submission/history.go index 34a65418..bbc48396 100644 --- a/api/submission/history.go +++ b/api/submission/history.go @@ -32,7 +32,7 @@ func HandleHistory(request *HistoryRequest) (*HistoryResponse, *core.APIError) { history, err := db.GetSubmissionHistory(request.Assignment, request.TargetUser.Email); if (err != nil) { - return nil, core.NewInternalError("-407", &request.APIRequestCourseUserContext, "Failed to get submission history."). + return nil, core.NewInternalError("-603", &request.APIRequestCourseUserContext, "Failed to get submission history."). Err(err).Add("user", request.TargetUser.Email); } diff --git a/api/submission/history_test.go b/api/submission/history_test.go index 9731b7d5..cfd1ade3 100644 --- a/api/submission/history_test.go +++ b/api/submission/history_test.go @@ -33,7 +33,7 @@ func TestHistory(test *testing.T) { response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`submission/history`), fields, nil, testCase.role); if (!response.Success) { if (testCase.permError) { - expectedLocator := "-319"; + expectedLocator := "-033"; if (response.Locator != expectedLocator) { test.Errorf("Case %d: Incorrect error returned on permissions error. Expcted '%s', found '%s'.", i, expectedLocator, response.Locator); diff --git a/api/submission/peek.go b/api/submission/peek.go index 6fc8e97f..0d4518fc 100644 --- a/api/submission/peek.go +++ b/api/submission/peek.go @@ -31,7 +31,7 @@ func HandlePeek(request *PeekRequest) (*PeekResponse, *core.APIError) { submissionResult, err := db.GetSubmissionResult(request.Assignment, request.TargetUser.Email, request.TargetSubmission); if (err != nil) { - return nil, core.NewInternalError("-402", &request.APIRequestCourseUserContext, "Failed to get submission result."). + return nil, core.NewInternalError("-601", &request.APIRequestCourseUserContext, "Failed to get submission result."). Err(err).Add("user", request.TargetUser.Email).Add("submission", request.TargetSubmission); } diff --git a/api/submission/peek_test.go b/api/submission/peek_test.go index 420a4518..718e95eb 100644 --- a/api/submission/peek_test.go +++ b/api/submission/peek_test.go @@ -62,7 +62,7 @@ func TestPeek(test *testing.T) { response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`submission/peek`), fields, nil, testCase.role); if (!response.Success) { if (testCase.permError) { - expectedLocator := "-319"; + expectedLocator := "-033"; if (response.Locator != expectedLocator) { test.Errorf("Case %d: Incorrect error returned on permissions error. Expcted '%s', found '%s'.", i, expectedLocator, response.Locator); diff --git a/api/submission/remove_submission.go b/api/submission/remove_submission.go index aeacde3e..1eec5ecc 100644 --- a/api/submission/remove_submission.go +++ b/api/submission/remove_submission.go @@ -29,7 +29,7 @@ func HandleRemoveSubmission(request *RemoveSubmissionRequest) (*RemoveSubmission doesExist, err := db.RemoveSubmission(request.Assignment, request.TargetUser.Email, request.TargetSubmission); if (err != nil) { - return nil, core.NewInternalError("-418", &request.APIRequestCourseUserContext, "Failed to remove the submission."). + return nil, core.NewInternalError("-606", &request.APIRequestCourseUserContext, "Failed to remove the submission."). Err(err).Add("user", request.TargetUser.Email).Add("submission", request.TargetSubmission); } diff --git a/api/submission/remove_submission_test.go b/api/submission/remove_submission_test.go index 644338a4..84551771 100644 --- a/api/submission/remove_submission_test.go +++ b/api/submission/remove_submission_test.go @@ -63,7 +63,7 @@ func TestRemoveSubmission(test *testing.T) { if (!response.Success) { if (testCase.permError) { - expectedLocator := "-306"; + expectedLocator := "-020"; if (response.Locator != expectedLocator) { test.Errorf("Case %d: Incorrect error returned on permissions error. Expected '%s', found '%s'.", i, expectedLocator, response.Locator); diff --git a/api/user/add.go b/api/user/add.go index ca5afcdf..8a45ce85 100644 --- a/api/user/add.go +++ b/api/user/add.go @@ -71,7 +71,7 @@ func HandleAdd(request *AddRequest) (*AddResponse, *core.APIError) { result, err := db.SyncUsers(request.Course, newUsers, request.Force, request.DryRun, !request.SkipEmails); if (err != nil) { - return nil, core.NewInternalError("-603", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-803", &request.APIRequestCourseUserContext, "Failed to sync new users.").Err(err); } @@ -87,7 +87,7 @@ func HandleAdd(request *AddRequest) (*AddResponse, *core.APIError) { users, err := db.GetUsers(request.Course); if (err != nil) { - return nil, core.NewInternalError("-604", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-804", &request.APIRequestCourseUserContext, "Failed to fetch users").Err(err); } diff --git a/api/user/add_test.go b/api/user/add_test.go index eba2246e..cf30d265 100644 --- a/api/user/add_test.go +++ b/api/user/add_test.go @@ -140,7 +140,7 @@ func TestUserAdd(test *testing.T) { response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`user/add`), fields, nil, testCase.role); if (!response.Success) { if (testCase.permError) { - expectedLocator := "-306"; + expectedLocator := "-020"; if (response.Locator != expectedLocator) { test.Errorf("Case %d: Incorrect error returned. Expcted '%s', found '%s'.", i, expectedLocator, response.Locator); diff --git a/api/user/change_password.go b/api/user/change_password.go index 4037595c..d6512180 100644 --- a/api/user/change_password.go +++ b/api/user/change_password.go @@ -28,7 +28,7 @@ func HandleChangePassword(request *ChangePasswordRequest) (*ChangePasswordRespon response.FoundUser = true; if (request.TargetUser.User.Role > request.User.Role) { - return nil, core.NewBadPermissionsError("-605", &request.APIRequestCourseUserContext, request.TargetUser.User.Role, + return nil, core.NewBadPermissionsError("-805", &request.APIRequestCourseUserContext, request.TargetUser.User.Role, "Cannot modify a user with a higher role.").Add("target-user", request.TargetUser.User.Email); } @@ -42,20 +42,20 @@ func HandleChangePassword(request *ChangePasswordRequest) (*ChangePasswordRespon } if (err != nil) { - return nil, core.NewInternalError("-606", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-806", &request.APIRequestCourseUserContext, "Failed to set password.").Err(err).Add("email", request.TargetUser.Email); } err = db.SaveUser(request.Course, request.TargetUser.User); if (err != nil) { - return nil, core.NewInternalError("-607", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-807", &request.APIRequestCourseUserContext, "Failed to save user.").Err(err).Add("email", request.TargetUser.Email); } if (pass != "") { err = model.SendUserAddEmail(request.TargetUser.User, pass, true, true, false, false); if (err != nil) { - return nil, core.NewInternalError("-608", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-808", &request.APIRequestCourseUserContext, "Failed to send user email.").Err(err).Add("email", request.TargetUser.Email); } } diff --git a/api/user/change_password_test.go b/api/user/change_password_test.go index f0642c78..391165a3 100644 --- a/api/user/change_password_test.go +++ b/api/user/change_password_test.go @@ -70,9 +70,9 @@ func TestChangePassword(test *testing.T) { if (!response.Success) { expectedLocator := ""; if (testCase.permError) { - expectedLocator = "-319"; + expectedLocator = "-033"; } else if (testCase.advPermError) { - expectedLocator = "-605"; + expectedLocator = "-805"; } if (expectedLocator == "") { diff --git a/api/user/get_test.go b/api/user/get_test.go index 873bf458..2e143b6d 100644 --- a/api/user/get_test.go +++ b/api/user/get_test.go @@ -33,9 +33,9 @@ func TestUserGet(test *testing.T) { if (!response.Success) { expectedLocator := ""; if (testCase.permError) { - expectedLocator = "-306"; + expectedLocator = "-020"; } else if (testCase.target == "") { - expectedLocator = "-320"; + expectedLocator = "-034"; } if (expectedLocator == "") { diff --git a/api/user/remove.go b/api/user/remove.go index cc6254c3..22aa3207 100644 --- a/api/user/remove.go +++ b/api/user/remove.go @@ -26,13 +26,13 @@ func HandleRemove(request *RemoveRequest) (*RemoveResponse, *core.APIError) { response.FoundUser = true; if (request.TargetUser.User.Role > request.User.Role) { - return nil, core.NewBadPermissionsError("-601", &request.APIRequestCourseUserContext, request.TargetUser.User.Role, + return nil, core.NewBadPermissionsError("-801", &request.APIRequestCourseUserContext, request.TargetUser.User.Role, "Cannot remove a user with a higher role.").Add("target-user", request.TargetUser.User.Email); } _, err := db.RemoveUser(request.Course, request.TargetUser.Email); if (err != nil) { - return nil, core.NewInternalError("-602", &request.APIRequestCourseUserContext, + return nil, core.NewInternalError("-802", &request.APIRequestCourseUserContext, "Failed to remove user.").Err(err).Add("email", request.TargetUser.Email); } diff --git a/api/user/remove_test.go b/api/user/remove_test.go index a1712431..a737957e 100644 --- a/api/user/remove_test.go +++ b/api/user/remove_test.go @@ -45,9 +45,9 @@ func TestUserRemove(test *testing.T) { if (!response.Success) { expectedLocator := ""; if (testCase.basicPermError) { - expectedLocator = "-306"; + expectedLocator = "-020"; } else if (testCase.advPermError) { - expectedLocator = "-601"; + expectedLocator = "-801"; } if (expectedLocator == "") { From 12534b536bc2377472b2dbdf666ce653190f8d09 Mon Sep 17 00:00:00 2001 From: Eriq Augustine Date: Sun, 31 Dec 2023 18:20:07 +0700 Subject: [PATCH 16/16] Added assignment syncing to the LMS user syncing procedure. --- _docs/assignments.md | 14 +++ api/lms/routes.go | 2 +- api/lms/{sync_users.go => sync.go} | 19 +-- api/lms/{sync_users_test.go => sync_test.go} | 6 +- cmd/{lms-sync-users => lms-sync}/main.go | 11 +- cmd/users/main.go | 8 +- common/timestamp.go | 13 ++ db/database.go | 6 +- model/assignment.go | 16 ++- model/assignment_load.go | 9 +- model/course.go | 25 ++++ model/sync.go | 53 ++++---- procedures/lms_sync.go | 121 +++++++++++++++++++ util/dirent.go | 49 ++++++-- util/path.go | 18 +++ 15 files changed, 294 insertions(+), 76 deletions(-) create mode 100644 _docs/assignments.md rename api/lms/{sync_users.go => sync.go} (53%) rename api/lms/{sync_users_test.go => sync_test.go} (92%) rename cmd/{lms-sync-users => lms-sync}/main.go (73%) diff --git a/_docs/assignments.md b/_docs/assignments.md new file mode 100644 index 00000000..23776012 --- /dev/null +++ b/_docs/assignments.md @@ -0,0 +1,14 @@ +# Autograder Assignments + +## LMS Syncing Mechanics + +When syncing assignment information from an LMS, +only fields not set in the assignment config will be brought over from the LMS. + +Matching autograder assignments with LMS assignments are first done via the LMS id set in the assignment's config. +If no LMS is is set, then an attempt is made to match assignments via their name (if the name is not empty). +A name match is made only if an autograder assignment matches one and only one LMS assignment. + +## Duplicate Assignments + +Assignments in the same course may not share the same ID, name, or LMS ID. diff --git a/api/lms/routes.go b/api/lms/routes.go index d7e6d343..0172aece 100644 --- a/api/lms/routes.go +++ b/api/lms/routes.go @@ -8,7 +8,7 @@ import ( var routes []*core.Route = []*core.Route{ core.NewAPIRoute(core.NewEndpoint(`lms/user/get`), HandleUserGet), - core.NewAPIRoute(core.NewEndpoint(`lms/sync/users`), HandleSyncUsers), + core.NewAPIRoute(core.NewEndpoint(`lms/sync`), HandleSync), core.NewAPIRoute(core.NewEndpoint(`lms/upload/scores`), HandleUploadScores), }; diff --git a/api/lms/sync_users.go b/api/lms/sync.go similarity index 53% rename from api/lms/sync_users.go rename to api/lms/sync.go index 90b96cda..4d95ae5e 100644 --- a/api/lms/sync_users.go +++ b/api/lms/sync.go @@ -2,10 +2,11 @@ package lms import ( "github.com/eriq-augustine/autograder/api/core" + "github.com/eriq-augustine/autograder/model" "github.com/eriq-augustine/autograder/procedures" ) -type SyncUsersRequest struct { +type SyncRequest struct { core.APIRequestCourseUserContext core.MinRoleAdmin @@ -13,24 +14,26 @@ type SyncUsersRequest struct { SkipEmails bool `json:"skip-emails"` } -type SyncUsersResponse struct { - core.SyncUsersInfo +type SyncResponse struct { + Users *core.SyncUsersInfo `json:"users"` + Assignments *model.AssignmentSyncResult `json:"assignments"` } -func HandleSyncUsers(request *SyncUsersRequest) (*SyncUsersResponse, *core.APIError) { +func HandleSync(request *SyncRequest) (*SyncResponse, *core.APIError) { if (request.Course.GetLMSAdapter() == nil) { return nil, core.NewBadRequestError("-403", &request.APIRequest, "Course is not linked to an LMS."). Add("course", request.Course.GetID()); } - result, err := procedures.SyncAllLMSUsers(request.Course, request.DryRun, !request.SkipEmails); + result, err := procedures.SyncLMS(request.Course, request.DryRun, !request.SkipEmails); if (err != nil) { return nil, core.NewInternalError("-404", &request.APIRequestCourseUserContext, - "Failed to sync LMS users.").Err(err); + "Failed to sync LMS information.").Err(err); } - response := SyncUsersResponse{ - SyncUsersInfo: *core.NewSyncUsersInfo(result), + response := SyncResponse{ + Users: core.NewSyncUsersInfo(result.UserSync), + Assignments: result.AssignmentSync, }; return &response, nil; diff --git a/api/lms/sync_users_test.go b/api/lms/sync_test.go similarity index 92% rename from api/lms/sync_users_test.go rename to api/lms/sync_test.go index a312cf61..34a2b0b6 100644 --- a/api/lms/sync_users_test.go +++ b/api/lms/sync_test.go @@ -8,7 +8,7 @@ import ( "github.com/eriq-augustine/autograder/util" ) -func TestAPISyncUsers(test *testing.T) { +func TestLMSSync(test *testing.T) { testCases := []struct{ role model.UserRole; dryRun bool; skipEmails bool; permError bool }{ {model.RoleOther, false, true, true}, {model.RoleStudent, false, true, true}, @@ -28,7 +28,7 @@ func TestAPISyncUsers(test *testing.T) { "skip-emails": testCase.skipEmails, }; - response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`lms/sync/users`), fields, nil, testCase.role); + response := core.SendTestAPIRequestFull(test, core.NewEndpoint(`lms/sync`), fields, nil, testCase.role); if (!response.Success) { if (testCase.permError) { expectedLocator := "-020"; @@ -44,7 +44,7 @@ func TestAPISyncUsers(test *testing.T) { } // Ensure the response can unmarshal. - var responseContent SyncUsersResponse; + var responseContent SyncResponse; util.MustJSONFromString(util.MustToJSON(response.Content), &responseContent); } } diff --git a/cmd/lms-sync-users/main.go b/cmd/lms-sync/main.go similarity index 73% rename from cmd/lms-sync-users/main.go rename to cmd/lms-sync/main.go index 7fc20e8e..7527ef5f 100644 --- a/cmd/lms-sync-users/main.go +++ b/cmd/lms-sync/main.go @@ -1,12 +1,15 @@ package main import ( + "fmt" + "github.com/alecthomas/kong" "github.com/rs/zerolog/log" "github.com/eriq-augustine/autograder/config" "github.com/eriq-augustine/autograder/db" "github.com/eriq-augustine/autograder/procedures" + "github.com/eriq-augustine/autograder/util" ) var args struct { @@ -18,7 +21,7 @@ var args struct { func main() { kong.Parse(&args, - kong.Description("Sync local users with matching LMS users."), + kong.Description("Sync course information from a course's LMS."), ); err := config.HandleConfigArgs(args.ConfigArgs); @@ -31,10 +34,10 @@ func main() { course := db.MustGetCourse(args.Course); - result, err := procedures.SyncAllLMSUsers(course, args.DryRun, !args.SkipEmails); + result, err := procedures.SyncLMS(course, args.DryRun, !args.SkipEmails); if (err != nil) { - log.Fatal().Err(err).Msg("Failed to sync LMS users."); + log.Fatal().Err(err).Msg("Failed to sync LMS."); } - result.PrintReport(); + fmt.Println(util.MustToJSONIndent(result)); } diff --git a/cmd/users/main.go b/cmd/users/main.go index 551b9f5b..e44a2b28 100644 --- a/cmd/users/main.go +++ b/cmd/users/main.go @@ -52,7 +52,7 @@ func (this *AddUser) Run(course *model.Course) error { } fmt.Println("Add Report:"); - result.PrintReport(); + fmt.Println(util.MustToJSONIndent(result)); if (this.SyncLMS) { result, err = procedures.SyncLMSUserEmail(course, this.Email, this.DryRun, this.SendEmail); @@ -61,7 +61,7 @@ func (this *AddUser) Run(course *model.Course) error { } fmt.Println("\nLMS sync report:"); - result.PrintReport(); + fmt.Println(util.MustToJSONIndent(result)); } return nil; @@ -89,7 +89,7 @@ func (this *AddTSV) Run(course *model.Course) error { if (this.DryRun) { fmt.Println("Doing a dry run, users file will not be written to."); - result.PrintReport(); + fmt.Println(util.MustToJSONIndent(result)); } if (this.SyncLMS) { @@ -105,7 +105,7 @@ func (this *AddTSV) Run(course *model.Course) error { if (this.DryRun) { fmt.Println("LMS sync report:"); - result.PrintReport(); + fmt.Println(util.MustToJSONIndent(result)); } } diff --git a/common/timestamp.go b/common/timestamp.go index 6321c454..98ada177 100644 --- a/common/timestamp.go +++ b/common/timestamp.go @@ -45,6 +45,19 @@ func MustTimestampFromString(text string) Timestamp { return instance; } +func (this Timestamp) Validate() error { + if (this.IsZero()) { + return nil; + } + + _, err := this.Time(); + if (err != nil) { + return err; + } + + return nil; +} + func (this Timestamp) IsZero() bool { return (string(this) == ""); } diff --git a/db/database.go b/db/database.go index 0d32f9e9..2b9fdff8 100644 --- a/db/database.go +++ b/db/database.go @@ -44,14 +44,14 @@ type Backend interface { // Returns (nil, nil) if the course does not exist. GetCourse(courseID string) (*model.Course, error); - // Load a course into the databse and return the course. + // Load a course into the database and return the course. // This implies loading a course directory from a config and saving it in the db. // Will search for and load any assignments, users, and submissions // located in the same directory tree. // Override any existing settings for this course. LoadCourse(path string) (*model.Course, error); - // Explicitly save a course. + // Explicitly save a course (which includes all course assignments). SaveCourse(course *model.Course) error; // Dump a database course to the standard directory layout @@ -74,7 +74,7 @@ type Backend interface { // Remove a user. // Do nothing and return nil if the user does not exist. RemoveUser(course *model.Course, email string) error; - + // Remove a submission. // Return a bool indicating whether the submission exists or not and an error if there is one. RemoveSubmission(assignment *model.Assignment, email string, submissionID string) (bool, error); diff --git a/model/assignment.go b/model/assignment.go index 90d7809b..795493cc 100644 --- a/model/assignment.go +++ b/model/assignment.go @@ -21,6 +21,9 @@ type Assignment struct { Name string `json:"name"` SortID string `json:"sort-id,omitempty"` + DueDate common.Timestamp `json:"due-date,omitempty"` + MaxPoints float64 `json:"max-points,omitempty"` + LMSID string `json:"lms-id,omitempty"` LatePolicy *LateGradingPolicy `json:"late-policy,omitempty"` @@ -86,16 +89,21 @@ func (this *Assignment) Validate() error { return fmt.Errorf("No course found for assignment.") } - if (this.Name == "") { - this.Name = this.ID; - } - var err error; this.ID, err = common.ValidateID(this.ID); if (err != nil) { return err; } + err = this.DueDate.Validate(); + if (err != nil) { + return fmt.Errorf("Due date is not a valid timestamp: '%w'.", err); + } + + if (this.MaxPoints < 0.0) { + return fmt.Errorf("Max points cannot be negative: %f.", this.MaxPoints); + } + this.imageLock = &sync.Mutex{}; // Inherit submission limit from course or leave nil. diff --git a/model/assignment_load.go b/model/assignment_load.go index 6bb4c187..251d9c4f 100644 --- a/model/assignment_load.go +++ b/model/assignment_load.go @@ -36,13 +36,10 @@ func ReadAssignmentConfig(course *Course, path string) (*Assignment, error) { return nil, fmt.Errorf("Failed to validate assignment config (%s): '%w'.", path, err); } - otherAssignment := course.GetAssignment(assignment.GetID()); - if (otherAssignment != nil) { - return nil, fmt.Errorf( - "Found multiple assignments with the same ID ('%s'): ['%s', '%s'].", - assignment.GetID(), otherAssignment.GetSourceDir(), assignment.GetSourceDir()); + err = course.AddAssignment(&assignment); + if (err != nil) { + return nil, fmt.Errorf("Failed to add assignment to course (%s): '%w'.", path, err); } - course.Assignments[assignment.GetID()] = &assignment; return &assignment, nil; } diff --git a/model/course.go b/model/course.go index a40aeaf9..7120e29d 100644 --- a/model/course.go +++ b/model/course.go @@ -139,6 +139,31 @@ func (this *Course) Validate() error { return nil; } +func (this *Course) AddAssignment(assignment *Assignment) error { + for _, otherAssignment := range this.Assignments { + if (assignment.GetID() == otherAssignment.GetID()) { + return fmt.Errorf( + "Found multiple assignments with the same ID ('%s'): ['%s', '%s'].", + assignment.GetID(), otherAssignment.GetSourceDir(), assignment.GetSourceDir()); + } + + if ((assignment.GetName() != "") && (assignment.GetName() == otherAssignment.GetName())) { + return fmt.Errorf( + "Found multiple assignments with the same name ('%s'): ['%s', '%s'].", + assignment.GetName(), otherAssignment.GetID(), assignment.GetID()); + } + + if ((assignment.GetLMSID() != "") && (assignment.GetLMSID() == otherAssignment.GetLMSID())) { + return fmt.Errorf( + "Found multiple assignments with the same LMS ID ('%s'): ['%s', '%s'].", + assignment.GetLMSID(), otherAssignment.GetID(), assignment.GetID()); + } + } + + this.Assignments[assignment.GetID()] = assignment; + return nil; +} + // Returns: (successful image names, map[imagename]error). func (this *Course) BuildAssignmentImages(force bool, quick bool, options *docker.BuildOptions) ([]string, map[string]error) { goodImageNames := make([]string, 0, len(this.Assignments)); diff --git a/model/sync.go b/model/sync.go index f06d10e4..2927a39e 100644 --- a/model/sync.go +++ b/model/sync.go @@ -1,11 +1,29 @@ package model -import ( - "fmt" -) - type LMSSyncResult struct { UserSync *UserSyncResult `json:"user-sync"` + AssignmentSync *AssignmentSyncResult `json:"assignment-sync"` +} + +type AssignmentSyncResult struct { + SyncedAssignments []AssignmentInfo `json:"synced-assignments"` + AmbiguousMatches []AssignmentInfo `json:"ambiguous-matches"` + NonMatchedAssignments []AssignmentInfo `json:"non-matched-assignments"` + UnchangedAssignments []AssignmentInfo `json:"unchanged-assignments"` +} + +type AssignmentInfo struct { + ID string `json:"id"` + Name string `json:"name"` +} + +func NewAssignmentSyncResult() *AssignmentSyncResult { + return &AssignmentSyncResult{ + SyncedAssignments: make([]AssignmentInfo, 0), + AmbiguousMatches: make([]AssignmentInfo, 0), + NonMatchedAssignments: make([]AssignmentInfo, 0), + UnchangedAssignments: make([]AssignmentInfo, 0), + }; } type UserSyncResult struct { @@ -47,33 +65,6 @@ func (this *UserSyncResult) Count() int { return len(this.Add) + len(this.Mod) + len(this.Del); } -func (this *UserSyncResult) PrintReport() { - groups := []struct{operation string; users []*User}{ - {"Added", this.Add}, - {"Modified", this.Mod}, - {"Deleted", this.Del}, - {"Skipped", this.Skip}, - {"Unchanged", this.Unchanged}, - }; - - for i, group := range groups { - if (i != 0) { - fmt.Println(); - } - - fmt.Printf("%s %d users.\n", group.operation, len(group.users)); - for _, user := range group.users { - fmt.Println(" " + user.ToRow(", ")); - } - } - - fmt.Println(); - fmt.Printf("Generated %d passwords.\n", len(this.ClearTextPasswords)); - for email, pass := range this.ClearTextPasswords { - fmt.Printf(" %s, %s\n", email, pass); - } -} - func (this *UserSyncResult) AddResolveResult(resolveResult *UserResolveResult) { if (resolveResult == nil) { return; diff --git a/procedures/lms_sync.go b/procedures/lms_sync.go index ea4dc64e..7a7dd5f7 100644 --- a/procedures/lms_sync.go +++ b/procedures/lms_sync.go @@ -4,6 +4,7 @@ import ( "errors" "fmt" + "github.com/eriq-augustine/autograder/common" "github.com/eriq-augustine/autograder/db" "github.com/eriq-augustine/autograder/lms" "github.com/eriq-augustine/autograder/lms/lmstypes" @@ -11,6 +12,25 @@ import ( "github.com/eriq-augustine/autograder/util" ) +func SyncLMS(course *model.Course, dryRun bool, sendEmails bool) (*model.LMSSyncResult, error) { + userSync, err := SyncAllLMSUsers(course, dryRun, sendEmails); + if (err != nil) { + return nil, err; + } + + assignmentSync, err := syncAssignments(course, dryRun); + if (err != nil) { + return nil, err; + } + + result := &model.LMSSyncResult{ + UserSync: userSync, + AssignmentSync: assignmentSync, + }; + + return result, nil; +} + // Sync users with the provided LMS. func SyncAllLMSUsers(course *model.Course, dryRun bool, sendEmails bool) (*model.UserSyncResult, error) { lmsUsersSlice, err := lms.FetchUsers(course); @@ -203,3 +223,104 @@ func getAllEmails(localUsers map[string]*model.User, lmsUsers map[string]*lmstyp return emails; } + +func syncAssignments(course *model.Course, dryRun bool) (*model.AssignmentSyncResult, error) { + result := model.NewAssignmentSyncResult(); + + adapter := course.GetLMSAdapter(); + if (!adapter.SyncAssignments) { + return result, nil; + } + + lmsAssignments, err := lms.FetchAssignments(course); + if (err != nil) { + return nil, fmt.Errorf("Failed to get assignments: '%w'.", err); + } + + localAssignments := course.GetAssignments(); + + // Match local assignments to LMS assignments. + matches := make(map[string]int); + for _, localAssignment := range localAssignments { + localID := localAssignment.GetID(); + localName := localAssignment.GetName(); + lmsID := localAssignment.GetLMSID(); + + for i, lmsAssignment := range lmsAssignments { + matchIndex := -1; + + if (lmsID != "") { + // Exact ID match. + if (lmsID == lmsAssignment.ID) { + matchIndex = i; + } + } else { + // Name match. + if ((localName != "") && (localName == lmsAssignment.Name)) { + matchIndex = i; + } + } + + if (matchIndex != -1) { + _, exists := matches[localID]; + if (exists) { + delete(matches, localID); + result.AmbiguousMatches = append(result.AmbiguousMatches, model.AssignmentInfo{localID, localName}); + break; + } + + matches[localID] = matchIndex; + } + } + + _, exists := matches[localID]; + if (!exists) { + result.NonMatchedAssignments = append(result.NonMatchedAssignments, model.AssignmentInfo{localID, localName}); + } + } + + for localID, lmsIndex := range matches { + localName := localAssignments[localID].GetName(); + changed := mergeAssignment(localAssignments[localID], lmsAssignments[lmsIndex]); + if (changed) { + result.SyncedAssignments = append(result.SyncedAssignments, model.AssignmentInfo{localID, localName}); + } else { + result.UnchangedAssignments = append(result.UnchangedAssignments, model.AssignmentInfo{localID, localName}); + } + } + + if (!dryRun) { + err = db.SaveCourse(course); + if (err != nil) { + return nil, fmt.Errorf("Failed to save course: '%w'.", err); + } + } + + return result, nil; +} + +func mergeAssignment(localAssignment *model.Assignment, lmsAssignment *lmstypes.Assignment) bool { + changed := false; + + if (localAssignment.LMSID == "") { + localAssignment.LMSID = lmsAssignment.ID; + changed = true; + } + + if ((localAssignment.Name == "") && (lmsAssignment.Name != "")) { + localAssignment.Name = lmsAssignment.Name; + changed = true; + } + + if (localAssignment.DueDate.IsZero() && (lmsAssignment.DueDate != nil) && !lmsAssignment.DueDate.IsZero()) { + localAssignment.DueDate = common.TimestampFromTime(*lmsAssignment.DueDate); + changed = true; + } + + if (util.IsZero(localAssignment.MaxPoints) && !util.IsZero(lmsAssignment.MaxPoints)) { + localAssignment.MaxPoints = lmsAssignment.MaxPoints; + changed = true; + } + + return changed; +} diff --git a/util/dirent.go b/util/dirent.go index c10c1a67..025f1edb 100644 --- a/util/dirent.go +++ b/util/dirent.go @@ -19,6 +19,10 @@ func CopyDirent(source string, dest string, onlyContents bool) error { return fmt.Errorf("Source dirent for copy does not exist: '%s'", source); } + if (IsSymLink(source)) { + return CopyLink(source, dest); + } + if (IsDir(source)) { return CopyDir(source, dest, onlyContents); } @@ -26,9 +30,33 @@ func CopyDirent(source string, dest string, onlyContents bool) error { return CopyFile(source, dest); } +func CopyLink(source string, dest string) error { + if (!PathExists(source)) { + return fmt.Errorf("Source link for copy does not exist: '%s'", source); + } + + if (IsDir(dest)) { + dest = filepath.Join(dest, filepath.Base(source)); + } + + MkDir(filepath.Dir(dest)); + + target, err := os.Readlink(source); + if (err != nil) { + return fmt.Errorf("Failed to read link being copied (%s): %w.", source, err); + } + + err = os.Symlink(target, dest); + if (err != nil) { + return fmt.Errorf("Failed to write link (target: '%s', source: '%s', dest: '%s'): %w.", target, source, dest, err); + } + + return nil; +} + // Copy a file from source to dest creating any necessary parents along the way.. // If dest is a file, it will be truncated. -// If fest is a dir, then the file will be created inside dest. +// If dest is a dir, then the file will be created inside dest. func CopyFile(source string, dest string) error { if (!PathExists(source)) { return fmt.Errorf("Source file for copy does not exist: '%s'", source); @@ -53,7 +81,11 @@ func CopyFile(source string, dest string) error { defer destIO.Close() _, err = io.Copy(destIO, sourceIO); - return err; + if (err != nil) { + return fmt.Errorf("Failed to copy file contents from '%s' to '%s': %w.", source, dest, err); + } + + return nil; } // Copy a directory (or just it's contents) into dest. @@ -118,16 +150,9 @@ func CopyDirContents(source string, dest string) error { sourcePath := filepath.Join(source, dirent.Name()); destPath := filepath.Join(dest, dirent.Name()); - if (dirent.IsDir()) { - err = CopyDirWhole(sourcePath, destPath); - if (err != nil) { - return err; - } - } else { - err = CopyFile(sourcePath, destPath); - if (err != nil) { - return err; - } + err = CopyDirent(sourcePath, destPath, false); + if (err != nil) { + return err; } } diff --git a/util/path.go b/util/path.go index 4af6f1ca..2e69ffe3 100644 --- a/util/path.go +++ b/util/path.go @@ -90,6 +90,24 @@ func IsEmptyDir(path string) bool { return false; } +func IsSymLink(path string) bool { + if (!PathExists(path)) { + return false; + } + + stat, err := os.Stat(path); + if (err != nil) { + if os.IsNotExist(err) { + return false; + } + + log.Warn().Err(err).Str("path", path).Msg("Unexpected error when checking if a path is a symbolic link."); + return false; + } + + return (stat.Mode() & fs.ModeSymlink) != 0; +} + func FindFiles(filename string, dir string) ([]string, error) { return FindDirents(filename, dir, true, false, true); }