github: standardize language + fix api requests

language:
- funcs, vars, log messages, etc.

api requests:
- make all requests use the makeGet() function
- include packwiz user agent

Signed-off-by: unilock <unilock@fennet.rentals>

github/github.go

@@ -3,9 +3,7 @@ package github
 import (
 	"encoding/json"
 	"errors"
-	"fmt"
 	"io"
-	"net/http"
 
 	"github.com/mitchellh/mapstructure"
 	"github.com/packwiz/packwiz/cmd"
@@ -16,7 +14,7 @@ import (
 var githubCmd = &cobra.Command{
 	Use:     "github",
 	Aliases: []string{"gh"},
-	Short:   "Manage github-based mods",
+	Short:   "Manage projects released on GitHub",
 }
 
 func init() {
@@ -27,7 +25,7 @@ func init() {
 func fetchRepo(slug string) (Repo, error) {
 	var repo Repo
 
-	res, err := http.Get(githubApiUrl + "repos/" + slug)
+	res, err := ghDefaultClient.getRepo(slug)
 	if err != nil {
 		return repo, err
 	}
@@ -45,7 +43,7 @@ func fetchRepo(slug string) (Repo, error) {
 	}
 
 	if repo.FullName == "" {
-		return repo, errors.New("invalid json while fetching mod: " + slug)
+		return repo, errors.New("invalid json while fetching project: " + slug)
 	}
 
 	return repo, nil
@@ -53,14 +51,12 @@ func fetchRepo(slug string) (Repo, error) {
 
 type Repo struct {
 	ID       int    `json:"id"`
-	NodeID   string `json:"node_id"`   // TODO: use this with GH API, instead of name (to acct. for repo renames?) + store in mod.pw.toml
 	Name     string `json:"name"`      // "hello_world"
 	FullName string `json:"full_name"` // "owner/hello_world"
 }
 
 type Release struct {
 	URL             string  `json:"url"`
-	NodeID          string  `json:"node_id"` // TODO: probably also use this with GH API
 	TagName         string  `json:"tag_name"`
 	TargetCommitish string  `json:"target_commitish"` // The branch of the release
 	Name            string  `json:"name"`
@@ -87,13 +83,10 @@ func (u Asset) getSha256() (string, error) {
 		return "", err
 	}
 
-	resp, err := http.Get(u.BrowserDownloadURL)
+	resp, err := ghDefaultClient.makeGet(u.BrowserDownloadURL)
 	if err != nil {
 		return "", err
 	}
-	if resp.StatusCode != 200 {
-		return "", fmt.Errorf("invalid response status: %v", resp.StatusCode)
-	}
 
 	defer resp.Body.Close()
 

github/install.go

@@ -19,8 +19,8 @@ var GithubRegex = regexp.MustCompile(`^https?://(?:www\.)?github\.com/([^/]+/[^/
 
 // installCmd represents the install command
 var installCmd = &cobra.Command{
-	Use:     "add [URL]",
-	Short:   "Add a project from a GitHub repository URL",
+	Use:     "add [URL|slug]",
+	Short:   "Add a project from a GitHub repository URL or slug",
 	Aliases: []string{"install", "get"},
 	Args:    cobra.ArbitraryArgs,
 	Run: func(cmd *cobra.Command, args []string) {
@@ -50,7 +50,7 @@ var installCmd = &cobra.Command{
 		repo, err := fetchRepo(slug)
 
 		if err != nil {
-			fmt.Println("Failed to get the mod ", err)
+			fmt.Printf("Failed to add project: %s\n", err)
 			os.Exit(1)
 		}
 
@@ -62,25 +62,20 @@ func init() {
 	githubCmd.AddCommand(installCmd)
 }
 
-const githubApiUrl = "https://api.github.com/"
-
 func installMod(repo Repo, pack core.Pack) error {
-	latestVersion, err := getLatestVersion(repo.FullName, "")
+	latestRelease, err := getLatestRelease(repo.FullName, "")
 	if err != nil {
-		return fmt.Errorf("failed to get latest version: %v", err)
-	}
-	if latestVersion.URL == "" {
-		return errors.New("mod is not available for this Minecraft version (use the acceptable-game-versions option to accept more) or mod loader")
+		return fmt.Errorf("failed to get latest release: %v", err)
 	}
 
-	return installVersion(repo, latestVersion, pack)
+	return installRelease(repo, latestRelease, pack)
 }
 
-func getLatestVersion(slug string, branch string) (Release, error) {
-	var modReleases []Release
+func getLatestRelease(slug string, branch string) (Release, error) {
+	var releases []Release
 	var release Release
 
-	resp, err := ghDefaultClient.makeGet(slug)
+	resp, err := ghDefaultClient.getReleases(slug)
 	if err != nil {
 		return release, err
 	}
@@ -90,20 +85,20 @@ func getLatestVersion(slug string, branch string) (Release, error) {
 	if err != nil {
 		return release, err
 	}
-	err = json.Unmarshal(body, &modReleases)
+	err = json.Unmarshal(body, &releases)
 	if err != nil {
 		return release, err
 	}
-	for _, r := range modReleases {
+	for _, r := range releases {
 		if r.TargetCommitish == branch {
 			return r, nil
 		}
 	}
 
-	return modReleases[0], nil
+	return releases[0], nil
 }
 
-func installVersion(repo Repo, release Release, pack core.Pack) error {
+func installRelease(repo Repo, release Release, pack core.Pack) error {
 	var files = release.Assets
 
 	if len(files) == 0 {
@@ -157,7 +152,7 @@ func installVersion(repo Repo, release Release, pack core.Pack) error {
 	if folder == "" {
 		folder = "mods"
 	}
-	path = modMeta.SetMetaPath(filepath.Join(viper.GetString("meta-folder-base"), folder, repo.Name+core.MetaExtension))
+	path = modMeta.SetMetaPath(filepath.Join(viper.GetString("meta-folder-base"), folder, core.SlugifyName(repo.Name)+core.MetaExtension))
 
 	// If the file already exists, this will overwrite it!!!
 	// TODO: Should this be improved?
@@ -168,21 +163,5 @@ func installVersion(repo Repo, release Release, pack core.Pack) error {
 	if err != nil {
 		return err
 	}
-	err = index.RefreshFileWithHash(path, format, hash, true)
-	if err != nil {
-		return err
-	}
-	err = index.Write()
-	if err != nil {
-		return err
-	}
-	err = pack.UpdateIndexHash()
-	if err != nil {
-		return err
-	}
-	err = pack.Write()
-	if err != nil {
-		return err
-	}
-	return nil
+	return index.RefreshFileWithHash(path, format, hash, true)
 }

github/request.go

@@ -3,6 +3,8 @@ package github
 import (
 	"fmt"
 	"net/http"
+
+	"github.com/packwiz/packwiz/core"
 )
 
 const ghApiServer = "api.github.com"
@@ -13,21 +15,40 @@ type ghApiClient struct {
 
 var ghDefaultClient = ghApiClient{&http.Client{}}
 
-func (c *ghApiClient) makeGet(slug string) (*http.Response, error) {
-	req, err := http.NewRequest("GET", "https://" + ghApiServer + "/repos/" + slug + "/releases", nil)
+func (c *ghApiClient) makeGet(url string) (*http.Response, error) {
+	req, err := http.NewRequest("GET", url, nil)
 	if err != nil {
 		return nil, err
 	}
 
+	req.Header.Set("User-Agent", core.UserAgent)
 	req.Header.Set("Accept", "application/vnd.github+json")
 
 	resp, err := c.httpClient.Do(req)
 	if err != nil {
 		return nil, err
 	}
-
 	if resp.StatusCode != 200 {
 		return nil, fmt.Errorf("invalid response status: %v", resp.Status)
 	}
+
+	return resp, nil
+}
+
+func (c *ghApiClient) getRepo(slug string) (*http.Response, error) {
+	resp, err := c.makeGet("https://" + ghApiServer + "/repos/" + slug)
+	if err != nil {
+		return resp, err
+	}
+
+	return resp, nil
+}
+
+func (c *ghApiClient) getReleases(slug string) (*http.Response, error) {
+	resp, err := c.getRepo(slug + "/releases")
+	if err != nil {
+		return resp, err
+	}
+
 	return resp, nil
 }

github/updater.go

@@ -24,8 +24,8 @@ func (u ghUpdater) ParseUpdate(updateUnparsed map[string]interface{}) (interface
 }
 
 type cachedStateStore struct {
-	ModID   string
-	Version Release
+	Slug    string
+	Release Release
 }
 
 func (u ghUpdater) CheckUpdate(mods []*core.Mod, pack core.Pack) ([]core.UpdateCheck, error) {
@@ -40,28 +40,33 @@ func (u ghUpdater) CheckUpdate(mods []*core.Mod, pack core.Pack) ([]core.UpdateC
 
 		data := rawData.(ghUpdateData)
 
-		newVersion, err := getLatestVersion(data.Slug, data.Branch)
+		newRelease, err := getLatestRelease(data.Slug, data.Branch)
 		if err != nil {
-			results[i] = core.UpdateCheck{Error: fmt.Errorf("failed to get latest version: %v", err)}
+			results[i] = core.UpdateCheck{Error: fmt.Errorf("failed to get latest release: %v", err)}
 			continue
 		}
 
-		if newVersion.TagName == data.Tag { // The latest version from the site is the same as the installed one
+		if newRelease.TagName == data.Tag { // The latest release is the same as the installed one
 			results[i] = core.UpdateCheck{UpdateAvailable: false}
 			continue
 		}
 
-		if len(newVersion.Assets) == 0 {
-			results[i] = core.UpdateCheck{Error: errors.New("new version doesn't have any assets")}
+		if len(newRelease.Assets) == 0 {
+			results[i] = core.UpdateCheck{Error: errors.New("new release doesn't have any assets")}
 			continue
 		}
 
-		newFilename := newVersion.Assets[0].Name
+		newFile := newRelease.Assets[0]
+		for _, v := range newRelease.Assets {
+			if strings.HasSuffix(v.Name, ".jar") {
+				newFile = v
+			}
+		}
 
 		results[i] = core.UpdateCheck{
 			UpdateAvailable: true,
-			UpdateString:    mod.FileName + " -> " + newFilename,
-			CachedState:     cachedStateStore{data.Slug, newVersion},
+			UpdateString:    mod.FileName + " -> " + newFile.Name,
+			CachedState:     cachedStateStore{data.Slug, newRelease},
 		}
 	}
 
@@ -71,10 +76,11 @@ func (u ghUpdater) CheckUpdate(mods []*core.Mod, pack core.Pack) ([]core.UpdateC
 func (u ghUpdater) DoUpdate(mods []*core.Mod, cachedState []interface{}) error {
 	for i, mod := range mods {
 		modState := cachedState[i].(cachedStateStore)
-		var version = modState.Version
+		var release = modState.Release
 
-		var file = version.Assets[0]
-		for _, v := range version.Assets {
+		// yes, this is duplicated - i guess we should just cache asset + tag instead of entire release...?
+		var file = release.Assets[0]
+		for _, v := range release.Assets {
 			if strings.HasSuffix(v.Name, ".jar") {
 				file = v
 			}
@@ -91,7 +97,7 @@ func (u ghUpdater) DoUpdate(mods []*core.Mod, cachedState []interface{}) error {
 			HashFormat: "sha256",
 			Hash:       hash,
 		}
-		mod.Update["github"]["tag"] = version.TagName
+		mod.Update["github"]["tag"] = release.TagName
 	}
 
 	return nil