From 6cc0ed5d63f4ab37f67bcbc19078214828520cc4 Mon Sep 17 00:00:00 2001 From: Fabio Bonelli Date: Fri, 13 Nov 2026 09:36:15 +0000 Subject: [PATCH] fix: keep the stored publiccode.yml when parsing fails A YAML syntax error leaves the parser without a document. For known active software the crawler still sent publiccodeYml as an empty string, so the API overwrote the last stored file while keeping the software active. Omit the file from the PATCH when parsing returns no document, so the API keeps the stored copy. Files that parse but fail validation are still saved, as before. --- apiclient/apiclient.go | 36 +++++++++++++++++------------- apiclient/apiclient_test.go | 44 +++++++++++++++++++++++++++++++++---- crawler/crawler.go | 21 +++++++++++++----- crawler/crawler_test.go | 42 +++++++++++++++++++++++++++++++++++ 4 files changed, 118 insertions(+), 25 deletions(-) diff --git a/apiclient/apiclient.go b/apiclient/apiclient.go index 508435f..30abfd0 100644 --- a/apiclient/apiclient.go +++ b/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 { diff --git a/apiclient/apiclient_test.go b/apiclient/apiclient_test.go index 91f9fc4..c1a187c 100644 --- a/apiclient/apiclient_test.go +++ b/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()) diff --git a/crawler/crawler.go b/crawler/crawler.go index 591c922..ec532f2 100644 --- a/crawler/crawler.go +++ b/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. diff --git a/crawler/crawler_test.go b/crawler/crawler_test.go index 4c6fdf6..c9ea1ac 100644 --- a/crawler/crawler_test.go +++ b/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()