fix: keep the stored publiccode.yml when parsing fails

apiclient/apiclient.go

@@ -385,15 +385,11 @@ func (clt APIClient) PostCatalogSoftware(
 }
 
 // PatchCatalogSoftware updates a software resource within the given catalog.
+// A nil publiccodeYml leaves the stored file unchanged.
 func (clt APIClient) PatchCatalogSoftware(
-	catalogID string, softwareID string, softwareURL string, aliases []string, publiccodeYml string, active bool,
+	catalogID string, softwareID string, softwareURL string, aliases []string, publiccodeYml *string, active bool,
 ) error {
-	body, err := json.Marshal(map[string]any{
-		"publiccodeYml": publiccodeYml,
-		"url":           softwareURL,
-		"aliases":       aliases,
-		"active":        active,
-	})
+	body, err := json.Marshal(softwarePatch(softwareURL, aliases, publiccodeYml, active))
 	if err != nil {
 		return fmt.Errorf("can't update software in catalog %s: %w", catalogID, err)
 	}
@@ -541,16 +537,11 @@ func (clt APIClient) PostSoftware(url string, aliases []string, publiccodeYml st
 }
 
 // PatchSoftware updates a software resource with the given fields and returns
-// any error encountered.
+// any error encountered. A nil publiccodeYml leaves the stored file unchanged.
 func (clt APIClient) PatchSoftware(
-	softwareID string, url string, aliases []string, publiccodeYml string, active bool,
+	softwareID string, url string, aliases []string, publiccodeYml *string, active bool,
 ) error {
-	body, err := json.Marshal(map[string]any{
-		"publiccodeYml": publiccodeYml,
-		"url":           url,
-		"aliases":       aliases,
-		"active":        active,
-	})
+	body, err := json.Marshal(softwarePatch(url, aliases, publiccodeYml, active))
 	if err != nil {
 		return fmt.Errorf("can't update software: %w", err)
 	}
@@ -569,6 +560,21 @@ func (clt APIClient) PatchSoftware(
 	return nil
 }
 
+func softwarePatch(url string, aliases []string, publiccodeYml *string, active bool) map[string]any {
+	fields := map[string]any{
+		"url":     url,
+		"aliases": aliases,
+		"active":  active,
+	}
+
+	// The API keeps any field missing from a PATCH body.
+	if publiccodeYml != nil {
+		fields["publiccodeYml"] = *publiccodeYml
+	}
+
+	return fields
+}
+
 // PostSoftwareLog creates a new software log with the given fields and returns
 // any error encountered.
 func (clt APIClient) PostSoftwareLog(softwareID string, message string) error {

apiclient/apiclient_test.go

@@ -132,12 +132,48 @@ func TestPatchSoftware_sendsActive(t *testing.T) {
 		_, _ = w.Write([]byte(`{}`))
 	})
 
-	err := client.PatchSoftware("id1", "https://example.org/repo.git", nil, "yml", true)
+	err := client.PatchSoftware("id1", "https://example.org/repo.git", nil, ptr("yml"), true)
 
 	assert.NoError(t, err)
 	assert.Equal(t, true, fields["active"])
 }
 
+func TestPatchSoftware_omitsMissingFile(t *testing.T) {
+	var fields map[string]any
+
+	client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
+		fields = patchBody(t, r)
+
+		_, _ = w.Write([]byte(`{}`))
+	})
+
+	err := client.PatchSoftware("id1", "https://example.org/repo.git", nil, nil, true)
+
+	assert.NoError(t, err)
+	assert.NotContains(t, fields, "publiccodeYml")
+	assert.Equal(t, "https://example.org/repo.git", fields["url"])
+}
+
+func TestPatchCatalogSoftware_omitsMissingFile(t *testing.T) {
+	var fields map[string]any
+
+	client := newTestClient(t, func(w http.ResponseWriter, r *http.Request) {
+		fields = patchBody(t, r)
+
+		_, _ = w.Write([]byte(`{}`))
+	})
+
+	err := client.PatchCatalogSoftware("catalog1", "id1", "https://example.org/repo.git", nil, nil, true)
+
+	assert.NoError(t, err)
+	assert.NotContains(t, fields, "publiccodeYml")
+	assert.Equal(t, "https://example.org/repo.git", fields["url"])
+}
+
+func ptr(value string) *string {
+	return &value
+}
+
 func TestPatchCatalogSoftware_sendsActive(t *testing.T) {
 	var fields map[string]any
 
@@ -147,7 +183,7 @@ func TestPatchCatalogSoftware_sendsActive(t *testing.T) {
 		_, _ = w.Write([]byte(`{}`))
 	})
 
-	err := client.PatchCatalogSoftware("catalog1", "id1", "https://example.org/repo.git", nil, "yml", true)
+	err := client.PatchCatalogSoftware("catalog1", "id1", "https://example.org/repo.git", nil, ptr("yml"), true)
 
 	assert.NoError(t, err)
 	assert.Equal(t, true, fields["active"])
@@ -264,7 +300,7 @@ func TestPatchSoftware_boundsRetryAfterWait(t *testing.T) {
 		})
 
 	start := time.Now()
-	err := client.PatchSoftware("id1", "https://example.org/repo.git", nil, "yml", true)
+	err := client.PatchSoftware("id1", "https://example.org/repo.git", nil, ptr("yml"), true)
 
 	assert.ErrorContains(t, err, "can't update software")
 	assert.Less(t, time.Since(start), testOperationTimeout)
@@ -285,7 +321,7 @@ func TestPatchCatalogSoftware_retriesWithinTimeouts(t *testing.T) {
 			_, _ = w.Write([]byte(`{}`))
 		})
 
-	err := client.PatchCatalogSoftware("catalog1", "id1", "https://example.org/repo.git", nil, "yml", true)
+	err := client.PatchCatalogSoftware("catalog1", "id1", "https://example.org/repo.git", nil, ptr("yml"), true)
 
 	assert.NoError(t, err)
 	assert.EqualValues(t, 2, attempts.Load())

crawler/crawler.go

@@ -779,15 +779,24 @@ func (c *Crawler) upsertSoftware(
 	// file.
 	active := valid || software.Active
 
-	if !c.DryRun {
-		if catalogID != "" {
-			return c.apiClient.PatchCatalogSoftware(catalogID, software.ID, repoURL, aliases, string(publiccodeYml), active)
-		}
+	if c.DryRun {
+		return nil
+	}
 
-		return c.apiClient.PatchSoftware(software.ID, repoURL, aliases, string(publiccodeYml), active)
+	// A file the parser could not read leaves publiccodeYml nil. Keep the
+	// last stored file instead of replacing it with an empty one.
+	var storedYml *string
+
+	if publiccodeYml != nil {
+		yml := string(publiccodeYml)
+		storedYml = &yml
 	}
 
-	return nil
+	if catalogID != "" {
+		return c.apiClient.PatchCatalogSoftware(catalogID, software.ID, repoURL, aliases, storedYml, active)
+	}
+
+	return c.apiClient.PatchSoftware(software.ID, repoURL, aliases, storedYml, active)
 }
 
 // mergeNewAliases appends aliases from newAliases that are not already in existing.

crawler/crawler_test.go

@@ -290,6 +290,48 @@ func TestProcessRepo_usesCanonicalURLAfterParseFailure(t *testing.T) {
 	assert.FileExists(t, filepath.Join(dataDir, "repos", repositoryURL.Host, "owner", "repository", "vitality.json"))
 }
 
+func TestProcessRepo_keepsStoredFileOnSyntaxError(t *testing.T) {
+	viper.Set("MAIN_PUBLISHER_ID", "")
+	viper.Set("SKIP_VITALITY", true)
+	t.Cleanup(viper.Reset)
+
+	var patched map[string]any
+	var patches int
+	crawler := newTestCrawler(t, func(w http.ResponseWriter, r *http.Request) {
+		switch {
+		case r.URL.Path == "/publiccode.yml":
+			_, _ = w.Write([]byte("not: [valid"))
+		case r.Method == http.MethodGet && r.URL.Path == "/software":
+			_, _ = w.Write([]byte(`{"data":[{"id":"sw1","active":true,"publiccodeYml":"name: Stored"}]}`))
+		case r.Method == http.MethodPatch && r.URL.Path == "/software/sw1":
+			patches++
+			assert.NoError(t, json.NewDecoder(r.Body).Decode(&patched))
+			_, _ = w.Write([]byte(`{}`))
+		case r.Method == http.MethodPost && strings.HasSuffix(r.URL.Path, "/logs"):
+			_, _ = w.Write([]byte(`{}`))
+		default:
+			t.Errorf("unexpected request: %s %s", r.Method, r.URL.Path)
+		}
+	})
+	crawler.newParser = func(config publiccode.ParserConfig) (*publiccode.Parser, error) {
+		config.AllowNetworkToPrivateHosts = true
+
+		return publiccode.NewParser(config)
+	}
+
+	repoURL, err := url.Parse("https://example.test/owner/repo")
+	require.NoError(t, err)
+	crawler.ProcessRepo(common.Repository{
+		Name: "owner/repo", URL: *repoURL, CanonicalURL: *repoURL,
+		FileRawURL: viper.GetString("API_BASEURL") + "/publiccode.yml",
+	})
+
+	require.Equal(t, 1, patches)
+	assert.NotContains(t, patched, "publiccodeYml")
+	assert.Equal(t, true, patched["active"])
+	assert.Equal(t, repoURL.String(), patched["url"])
+}
+
 func runGit(t *testing.T, dir string, args ...string) {
 	t.Helper()