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()