Better messaging for custom validators

This change adds better messaging when custom validators are involved,
which should reduce the amount of guesswork.

gitservertest/zip.go

@@ -33,6 +33,34 @@ const (
 		"Name": "token-caseless"
 	}
 }
+`
+
+	// CustomValidatorSettingsJSON is the JSON representation of a problem with
+	// just one case called "0" and a custom validator.
+	CustomValidatorSettingsJSON = `{
+	"Cases": [
+		{
+			"Cases": [
+				{
+					"Name": "0",
+					"Weight": 1
+				}
+			],
+			"Name": "0"
+		}
+	],
+	"Limits": {
+		"ExtraWallTime": "0s",
+		"MemoryLimit": 33554432,
+		"OutputLimit": 10240,
+		"OverallWallTimeLimit": "1m0s",
+		"TimeLimit": "1s"
+	},
+	"Slow": false,
+	"Validator": {
+		"Name": "custom"
+	}
+}
 `
 )
 

handler.go

@@ -751,7 +751,9 @@ func validateUpdateMaster(
 		return base.ErrorWithCategory(
 			ErrProblemBadLayout,
 			errors.Errorf(
-				"problem with unused validator",
+				"problem requested using validator %s, but has an unused validator.%s file",
+				problemSettings.Validator.Name,
+				validatorLang,
 			),
 		)
 	}

ziphandler.go

@@ -955,7 +955,6 @@ func CreatePackfile(
 
 func getUpdatedProblemSettings(
 	problemSettings *common.ProblemSettings,
-	zipMergeStrategy ZipMergeStrategy,
 	repo *git.Repository,
 	parent *git.Oid,
 ) (*common.ProblemSettings, error) {
@@ -1015,25 +1014,6 @@ func getUpdatedProblemSettings(
 		)
 	}
 
-	if updatedProblemSettings.Validator.Name != problemSettings.Validator.Name {
-		if updatedProblemSettings.Validator.Name == "custom" {
-			return nil, base.ErrorWithCategory(
-				ErrProblemBadLayout,
-				errors.Errorf(
-					"problem with unused validator",
-				),
-			)
-		}
-		if problemSettings.Validator.Name == "custom" {
-			return nil, base.ErrorWithCategory(
-				ErrProblemBadLayout,
-				errors.Errorf(
-					"problem with custom validator missing a validator",
-				),
-			)
-		}
-	}
-
 	updatedProblemSettings.Limits = problemSettings.Limits
 	updatedProblemSettings.Validator.Name = problemSettings.Validator.Name
 	updatedProblemSettings.Validator.Tolerance = problemSettings.Validator.Tolerance
@@ -1139,7 +1119,6 @@ func ConvertZipToPackfile(
 			var err error
 			if settings, err = getUpdatedProblemSettings(
 				settings,
-				zipMergeStrategy,
 				repo,
 				parent,
 			); err != nil {

ziphandler_test.go

@@ -111,7 +111,7 @@ func postZip(
 }
 
 func TestPushZip(t *testing.T) {
-	tmpDir, err := ioutil.TempDir("", "handler_test")
+	tmpDir, err := ioutil.TempDir("", t.Name())
 	if err != nil {
 		t.Fatalf("Failed to create directory: %v", err)
 	}
@@ -184,7 +184,7 @@ func TestPushZip(t *testing.T) {
 }
 
 func TestConvertZip(t *testing.T) {
-	tmpDir, err := ioutil.TempDir("", "handler_test")
+	tmpDir, err := ioutil.TempDir("", t.Name())
 	if err != nil {
 		t.Fatalf("Failed to create directory: %v", err)
 	}
@@ -264,7 +264,7 @@ func TestConvertZip(t *testing.T) {
 }
 
 func TestUpdateProblemSettings(t *testing.T) {
-	tmpDir, err := ioutil.TempDir("", "handler_test")
+	tmpDir, err := ioutil.TempDir("", t.Name())
 	if err != nil {
 		t.Fatalf("Failed to create directory: %v", err)
 	}
@@ -397,3 +397,249 @@ func TestUpdateProblemSettings(t *testing.T) {
 		}
 	}
 }
+
+func TestUpdateProblemSettingsWithCustomValidator(t *testing.T) {
+	tmpDir, err := ioutil.TempDir("", t.Name())
+	if err != nil {
+		t.Fatalf("Failed to create directory: %v", err)
+	}
+	if os.Getenv("PRESERVE") == "" {
+		defer os.RemoveAll(tmpDir)
+	}
+
+	log := base.StderrLog()
+	ts := httptest.NewServer(ZipHandler(
+		tmpDir,
+		NewGitProtocol(authorize, nil, true, OverallWallTimeHardLimit, fakeInteractiveSettingsCompiler, log),
+		&base.NoOpMetrics{},
+		log,
+	))
+	defer ts.Close()
+
+	problemAlias := "sumas-validator"
+
+	// Create the problem.
+	{
+		zipContents, err := gitservertest.CreateZip(
+			map[string]io.Reader{
+				"settings.json":          strings.NewReader(gitservertest.CustomValidatorSettingsJSON),
+				"cases/0.in":             strings.NewReader("1 2\n"),
+				"cases/0.out":            strings.NewReader("3\n"),
+				"statements/es.markdown": strings.NewReader("Sumaz\n"),
+				"validator.py":           strings.NewReader("print 1\n"),
+			},
+		)
+		if err != nil {
+			t.Fatalf("Failed to create zip: %v", err)
+		}
+		postZip(
+			t,
+			adminAuthorization,
+			problemAlias,
+			nil,
+			ZipMergeStrategyTheirs,
+			zipContents,
+			"initial commit",
+			true, // create
+			true, // useMultipartFormData
+			ts,
+		)
+	}
+
+	// Update settings.
+	{
+		zipContents, err := gitservertest.CreateZip(
+			map[string]io.Reader{},
+		)
+		if err != nil {
+			t.Fatalf("Failed to create zip: %v", err)
+		}
+		problemSettings := &common.ProblemSettings{
+			Limits: common.LimitsSettings{
+				ExtraWallTime:        base.Duration(0),
+				MemoryLimit:          33554432,
+				OutputLimit:          10240,
+				OverallWallTimeLimit: base.Duration(5 * time.Minute),
+				TimeLimit:            base.Duration(time.Second),
+			},
+			Slow: false,
+			Validator: common.ValidatorSettings{
+				Name: "custom",
+			},
+		}
+		updateResult := postZip(
+			t,
+			adminAuthorization,
+			problemAlias,
+			problemSettings,
+			ZipMergeStrategyOurs,
+			zipContents,
+			"updated settings",
+			false, // create
+			false, // useMultipartFormData
+			ts,
+		)
+
+		updatedFiles := make(map[string]struct{})
+		for _, updatedFile := range updateResult.UpdatedFiles {
+			updatedFiles[updatedFile.Path] = struct{}{}
+			if updatedFile.Type != "modified" {
+				t.Errorf("unexpected updated file: %v", updatedFile)
+			}
+		}
+
+		expectedUpdatedFiles := map[string]struct{}{
+			"settings.json":         {},
+			"settings.distrib.json": {},
+		}
+		if !reflect.DeepEqual(expectedUpdatedFiles, updatedFiles) {
+			t.Errorf("mismatched updated files, expected %v, got %v", expectedUpdatedFiles, updatedFiles)
+		}
+	}
+
+	// Update statements.
+	{
+		zipContents, err := gitservertest.CreateZip(
+			map[string]io.Reader{
+				"statements/es.markdown": strings.NewReader("Sumas\n"),
+			},
+		)
+		if err != nil {
+			t.Fatalf("Failed to create zip: %v", err)
+		}
+		updateResult := postZip(
+			t,
+			adminAuthorization,
+			problemAlias,
+			nil,
+			ZipMergeStrategyRecursiveTheirs,
+			zipContents,
+			"updated statement",
+			false, // create
+			false, // useMultipartFormData
+			ts,
+		)
+
+		updatedFiles := make(map[string]struct{})
+		for _, updatedFile := range updateResult.UpdatedFiles {
+			updatedFiles[updatedFile.Path] = struct{}{}
+			if updatedFile.Type != "modified" {
+				t.Errorf("unexpected updated file: %v", updatedFile)
+			}
+		}
+
+		expectedUpdatedFiles := map[string]struct{}{
+			"statements/es.markdown": {},
+		}
+		if !reflect.DeepEqual(expectedUpdatedFiles, updatedFiles) {
+			t.Errorf("mismatched updated files, expected %v, got %v", expectedUpdatedFiles, updatedFiles)
+		}
+	}
+
+	// Use token validator.
+	{
+		zipContents, err := gitservertest.CreateZip(
+			map[string]io.Reader{
+				"cases/0.in":             strings.NewReader("1 2\n"),
+				"cases/0.out":            strings.NewReader("3\n"),
+				"statements/es.markdown": strings.NewReader("Sumas\n"),
+			},
+		)
+		if err != nil {
+			t.Fatalf("Failed to create zip: %v", err)
+		}
+		problemSettings := &common.ProblemSettings{
+			Limits: common.LimitsSettings{
+				ExtraWallTime:        base.Duration(0),
+				MemoryLimit:          33554432,
+				OutputLimit:          10240,
+				OverallWallTimeLimit: base.Duration(5 * time.Minute),
+				TimeLimit:            base.Duration(time.Second),
+			},
+			Slow: false,
+			Validator: common.ValidatorSettings{
+				Name: "token-caseless",
+			},
+		}
+		updateResult := postZip(
+			t,
+			adminAuthorization,
+			problemAlias,
+			problemSettings,
+			ZipMergeStrategyTheirs,
+			zipContents,
+			"updated validator",
+			false, // create
+			false, // useMultipartFormData
+			ts,
+		)
+
+		updatedFiles := make(map[string]string)
+		for _, updatedFile := range updateResult.UpdatedFiles {
+			updatedFiles[updatedFile.Path] = updatedFile.Type
+		}
+
+		expectedUpdatedFiles := map[string]string{
+			"settings.distrib.json": "modified",
+			"settings.json":         "modified",
+			"validator.py":          "deleted",
+		}
+		if !reflect.DeepEqual(expectedUpdatedFiles, updatedFiles) {
+			t.Errorf("mismatched updated files, expected %v, got %v", expectedUpdatedFiles, updatedFiles)
+		}
+	}
+
+	// Restore custom validator.
+	{
+		zipContents, err := gitservertest.CreateZip(
+			map[string]io.Reader{
+				"cases/0.in":             strings.NewReader("1 2\n"),
+				"cases/0.out":            strings.NewReader("3\n"),
+				"statements/es.markdown": strings.NewReader("Sumas\n"),
+				"validator.py":           strings.NewReader("print 1\n"),
+			},
+		)
+		if err != nil {
+			t.Fatalf("Failed to create zip: %v", err)
+		}
+		problemSettings := &common.ProblemSettings{
+			Limits: common.LimitsSettings{
+				ExtraWallTime:        base.Duration(0),
+				MemoryLimit:          33554432,
+				OutputLimit:          10240,
+				OverallWallTimeLimit: base.Duration(5 * time.Minute),
+				TimeLimit:            base.Duration(time.Second),
+			},
+			Slow: false,
+			Validator: common.ValidatorSettings{
+				Name: "custom",
+			},
+		}
+		updateResult := postZip(
+			t,
+			adminAuthorization,
+			problemAlias,
+			problemSettings,
+			ZipMergeStrategyTheirs,
+			zipContents,
+			"updated validator",
+			false, // create
+			false, // useMultipartFormData
+			ts,
+		)
+
+		updatedFiles := make(map[string]string)
+		for _, updatedFile := range updateResult.UpdatedFiles {
+			updatedFiles[updatedFile.Path] = updatedFile.Type
+		}
+
+		expectedUpdatedFiles := map[string]string{
+			"settings.distrib.json": "modified",
+			"settings.json":         "modified",
+			"validator.py":          "added",
+		}
+		if !reflect.DeepEqual(expectedUpdatedFiles, updatedFiles) {
+			t.Errorf("mismatched updated files, expected %v, got %v", expectedUpdatedFiles, updatedFiles)
+		}
+	}
+}