Refact cwhub: make some methods private (#2584)

* make hub.enableItem() private
* make hub.downloadLatest() private
* make getData() private
* make hub.disableItem() private
* make hub.downloadItem() private
* make hub.syncDir() private
* make hub.localSync() private; keep warnings in Hub struct (no need to call LocalSync to get them)
This commit is contained in:
mmetc 2023-11-09 12:07:09 +01:00 committed by GitHub
parent ec4b5bdc86
commit f80d841188
No known key found for this signature in database
GPG key ID: 4AEE18F83AFDEB23
7 changed files with 52 additions and 51 deletions

View file

@ -53,9 +53,7 @@ func runHubList(cmd *cobra.Command, args []string) error {
return err
}
// use LocalSync to get warnings about tainted / outdated items
warn, _ := hub.LocalSync()
for _, v := range warn {
for _, v := range hub.Warnings {
log.Info(v)
}
@ -96,9 +94,7 @@ func runHubUpdate(cmd *cobra.Command, args []string) error {
return fmt.Errorf("failed to update hub: %w", err)
}
// use LocalSync to get warnings about tainted / outdated items
warn, _ := hub.LocalSync()
for _, v := range warn {
for _, v := range hub.Warnings {
log.Info(v)
}

View file

@ -54,7 +54,7 @@ func downloadFile(url string, destPath string) error {
return nil
}
func GetData(data []types.DataSource, dataDir string) error {
func getData(data []types.DataSource, dataDir string) error {
for _, dataS := range data {
destPath := filepath.Join(dataDir, dataS.DestPath)
log.Infof("downloading data '%s' in '%s'", dataS.SourceURL, destPath)
@ -91,7 +91,7 @@ func downloadData(dataFolder string, force bool, reader io.Reader) error {
}
if download || force {
if err := GetData(data.Data, dataFolder); err != nil {
if err := getData(data.Data, dataFolder); err != nil {
return fmt.Errorf("while getting data: %w", err)
}
}

View file

@ -10,10 +10,9 @@ import (
log "github.com/sirupsen/logrus"
)
// EnableItem creates a symlink between actual config file at hub.HubDir and hub.ConfigDir
// enableItem creates a symlink between actual config file at hub.HubDir and hub.ConfigDir
// Handles collections recursively
// XXX: called from config_restore otherwise no need to export
func (h *Hub) EnableItem(target *Item) error {
func (h *Hub) enableItem(target *Item) error {
parentDir := filepath.Clean(h.local.InstallDir + "/" + target.Type + "/" + target.Stage + "/")
// create directories if needed
@ -48,7 +47,7 @@ func (h *Hub) EnableItem(target *Item) error {
return fmt.Errorf("required %s %s of %s doesn't exist, abort", sub.Type, sub.Name, target.Name)
}
if err := h.EnableItem(&val); err != nil {
if err := h.enableItem(&val); err != nil {
return fmt.Errorf("while installing %s: %w", sub.Name, err)
}
}
@ -96,8 +95,8 @@ func (h *Hub) purgeItem(target Item) (Item, error) {
return target, nil
}
// DisableItem to disable an item managed by the hub, removes the symlink if purge is true
func (h *Hub) DisableItem(target *Item, purge bool, force bool) error {
// disableItem to disable an item managed by the hub, removes the symlink if purge is true
func (h *Hub) disableItem(target *Item, purge bool, force bool) error {
// XXX: should return the number of disabled/purged items to inform the upper layer whether to reload or not
var err error
@ -141,7 +140,7 @@ func (h *Hub) DisableItem(target *Item, purge bool, force bool) error {
}
if toRemove {
if err = h.DisableItem(&val, purge, force); err != nil {
if err = h.disableItem(&val, purge, force); err != nil {
return fmt.Errorf("while disabling %s: %w", sub.Name, err)
}
} else {

View file

@ -10,20 +10,20 @@ import (
func testInstall(hub *Hub, t *testing.T, item Item) {
// Install the parser
err := hub.DownloadLatest(&item, false, false)
err := hub.downloadLatest(&item, false, false)
require.NoError(t, err, "failed to download %s", item.Name)
_, err = hub.LocalSync()
err = hub.localSync()
require.NoError(t, err, "failed to run localSync")
assert.True(t, hub.Items[item.Type][item.Name].UpToDate, "%s should be up-to-date", item.Name)
assert.False(t, hub.Items[item.Type][item.Name].Installed, "%s should not be installed", item.Name)
assert.False(t, hub.Items[item.Type][item.Name].Tainted, "%s should not be tainted", item.Name)
err = hub.EnableItem(&item)
err = hub.enableItem(&item)
require.NoError(t, err, "failed to enable %s", item.Name)
_, err = hub.LocalSync()
err = hub.localSync()
require.NoError(t, err, "failed to run localSync")
assert.True(t, hub.Items[item.Type][item.Name].Installed, "%s should be installed", item.Name)
@ -41,7 +41,7 @@ func testTaint(hub *Hub, t *testing.T, item Item) {
require.NoError(t, err, "failed to write to %s (%s)", item.LocalPath, item.Name)
// Local sync and check status
_, err = hub.LocalSync()
err = hub.localSync()
require.NoError(t, err, "failed to run localSync")
assert.True(t, hub.Items[item.Type][item.Name].Tainted, "%s should be tainted", item.Name)
@ -51,11 +51,11 @@ func testUpdate(hub *Hub, t *testing.T, item Item) {
assert.False(t, hub.Items[item.Type][item.Name].UpToDate, "%s should not be up-to-date", item.Name)
// Update it + check status
err := hub.DownloadLatest(&item, true, true)
err := hub.downloadLatest(&item, true, true)
require.NoError(t, err, "failed to update %s", item.Name)
// Local sync and check status
_, err = hub.LocalSync()
err = hub.localSync()
require.NoError(t, err, "failed to run localSync")
assert.True(t, hub.Items[item.Type][item.Name].UpToDate, "%s should be up-to-date", item.Name)
@ -66,26 +66,26 @@ func testDisable(hub *Hub, t *testing.T, item Item) {
assert.True(t, hub.Items[item.Type][item.Name].Installed, "%s should be installed", item.Name)
// Remove
err := hub.DisableItem(&item, false, false)
err := hub.disableItem(&item, false, false)
require.NoError(t, err, "failed to disable %s", item.Name)
// Local sync and check status
warns, err := hub.LocalSync()
err = hub.localSync()
require.NoError(t, err, "failed to run localSync")
require.Empty(t, warns, "unexpected warnings: %+v", warns)
require.Empty(t, hub.Warnings)
assert.False(t, hub.Items[item.Type][item.Name].Tainted, "%s should not be tainted anymore", item.Name)
assert.False(t, hub.Items[item.Type][item.Name].Installed, "%s should not be installed anymore", item.Name)
assert.True(t, hub.Items[item.Type][item.Name].Downloaded, "%s should still be downloaded", item.Name)
// Purge
err = hub.DisableItem(&item, true, false)
err = hub.disableItem(&item, true, false)
require.NoError(t, err, "failed to purge %s", item.Name)
// Local sync and check status
warns, err = hub.LocalSync()
err = hub.localSync()
require.NoError(t, err, "failed to run localSync")
require.Empty(t, warns, "unexpected warnings: %+v", warns)
require.Empty(t, hub.Warnings)
assert.False(t, hub.Items[item.Type][item.Name].Installed, "%s should not be installed anymore", item.Name)
assert.False(t, hub.Items[item.Type][item.Name].Downloaded, "%s should not be downloaded", item.Name)

View file

@ -35,7 +35,7 @@ func (h *Hub) InstallItem(name string, itemType string, force bool, downloadOnly
}
// XXX: confusing semantic between force and updateOnly?
if err := h.DownloadLatest(item, force, true); err != nil {
if err := h.downloadLatest(item, force, true); err != nil {
return fmt.Errorf("while downloading %s: %w", item.Name, err)
}
@ -44,14 +44,14 @@ func (h *Hub) InstallItem(name string, itemType string, force bool, downloadOnly
}
if downloadOnly {
// XXX: should get the path from DownloadLatest
// XXX: should get the path from downloadLatest
log.Infof("Downloaded %s to %s", item.Name, filepath.Join(h.local.HubDir, item.RemotePath))
return nil
}
// XXX: should we stop here if the item is already installed?
if err := h.EnableItem(item); err != nil {
if err := h.enableItem(item); err != nil {
return fmt.Errorf("while enabling %s: %w", item.Name, err)
}
@ -83,11 +83,11 @@ func (h *Hub) RemoveItem(itemType string, name string, purge bool, forceAction b
return false, nil
}
if err := h.DisableItem(item, purge, forceAction); err != nil {
if err := h.disableItem(item, purge, forceAction); err != nil {
return false, fmt.Errorf("unable to disable %s: %w", item.Name, err)
}
// XXX: should take the value from DisableItem
// XXX: should take the value from disableItem
removed = true
if err := h.AddItem(*item); err != nil {
@ -127,7 +127,7 @@ func (h *Hub) UpgradeItem(itemType string, name string, force bool) (bool, error
}
}
if err := h.DownloadLatest(item, force, true); err != nil {
if err := h.downloadLatest(item, force, true); err != nil {
return false, fmt.Errorf("%s: download failed: %w", item.Name, err)
}
@ -153,9 +153,9 @@ func (h *Hub) UpgradeItem(itemType string, name string, force bool) (bool, error
return updated, nil
}
// DownloadLatest will download the latest version of Item to the tdir directory
func (h *Hub) DownloadLatest(target *Item, overwrite bool, updateOnly bool) error {
// XXX: should return the path of the downloaded file (taken from DownloadItem)
// downloadLatest will download the latest version of Item to the tdir directory
func (h *Hub) downloadLatest(target *Item, overwrite bool, updateOnly bool) error {
// XXX: should return the path of the downloaded file (taken from downloadItem)
log.Debugf("Downloading %s %s", target.Type, target.Name)
if !target.HasSubItems() {
@ -165,7 +165,7 @@ func (h *Hub) DownloadLatest(target *Item, overwrite bool, updateOnly bool) erro
}
// XXX:
return h.DownloadItem(target, overwrite)
return h.downloadItem(target, overwrite)
}
// collection
@ -186,21 +186,21 @@ func (h *Hub) DownloadLatest(target *Item, overwrite bool, updateOnly bool) erro
if sub.HasSubItems() {
log.Tracef("collection, recurse")
if err := h.DownloadLatest(&val, overwrite, updateOnly); err != nil {
if err := h.downloadLatest(&val, overwrite, updateOnly); err != nil {
return fmt.Errorf("while downloading %s: %w", val.Name, err)
}
}
downloaded := val.Downloaded
if err := h.DownloadItem(&val, overwrite); err != nil {
if err := h.downloadItem(&val, overwrite); err != nil {
return fmt.Errorf("while downloading %s: %w", val.Name, err)
}
// We need to enable an item when it has been added to a collection since latest release of the collection.
// We check if val.Downloaded is false because maybe the item has been disabled by the user.
if !val.Installed && !downloaded {
if err := h.EnableItem(&val); err != nil {
if err := h.enableItem(&val); err != nil {
return fmt.Errorf("enabling '%s': %w", val.Name, err)
}
}
@ -208,14 +208,14 @@ func (h *Hub) DownloadLatest(target *Item, overwrite bool, updateOnly bool) erro
h.Items[sub.Type][sub.Name] = val
}
if err := h.DownloadItem(target, overwrite); err != nil {
if err := h.downloadItem(target, overwrite); err != nil {
return fmt.Errorf("failed to download item: %w", err)
}
return nil
}
func (h *Hub) DownloadItem(target *Item, overwrite bool) error {
func (h *Hub) downloadItem(target *Item, overwrite bool) error {
url, err := h.remote.urlTo(target.RemotePath)
if err != nil {
return fmt.Errorf("failed to build hub item request: %w", err)

View file

@ -17,6 +17,7 @@ type Hub struct {
remote *RemoteHubCfg
skippedLocal int
skippedTainted int
Warnings []string
}
var theHub *Hub
@ -56,7 +57,7 @@ func NewHub(local *csconfig.LocalHubCfg, remote *RemoteHubCfg, downloadIndex boo
return nil, fmt.Errorf("failed to load index: %w", err)
}
if _, err := theHub.LocalSync(); err != nil {
if err := theHub.localSync(); err != nil {
return nil, fmt.Errorf("failed to sync items: %w", err)
}

View file

@ -391,7 +391,7 @@ func (h *Hub) checkSubItems(v *Item) error {
return nil
}
func (h *Hub) SyncDir(dir string) ([]string, error) {
func (h *Hub) syncDir(dir string) ([]string, error) {
warnings := []string{}
// For each, scan PARSERS, POSTOVERFLOWS, SCENARIOS and COLLECTIONS last
@ -439,18 +439,23 @@ func (h *Hub) SyncDir(dir string) ([]string, error) {
}
// Updates the info from HubInit() with the local state
func (h *Hub) LocalSync() ([]string, error) {
func (h *Hub) localSync() error {
h.skippedLocal = 0
h.skippedTainted = 0
h.Warnings = []string{}
warnings, err := h.SyncDir(h.local.InstallDir)
warnings, err := h.syncDir(h.local.InstallDir)
if err != nil {
return warnings, fmt.Errorf("failed to scan %s: %w", h.local.InstallDir, err)
return fmt.Errorf("failed to scan %s: %w", h.local.InstallDir, err)
}
if _, err = h.SyncDir(h.local.HubDir); err != nil {
return warnings, fmt.Errorf("failed to scan %s: %w", h.local.HubDir, err)
h.Warnings = append(h.Warnings, warnings...)
if warnings, err = h.syncDir(h.local.HubDir); err != nil {
return fmt.Errorf("failed to scan %s: %w", h.local.HubDir, err)
}
return warnings, nil
h.Warnings = append(h.Warnings, warnings...)
return nil
}