Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Sign up for GitHub

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Jump to bottom

Fix all linter errors in loader package #2331

Merged
merged 1 commit into from
Jan 25, 2022
Merged

Fix all linter errors in loader package #2331

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
1 commit
Select commit
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 5 additions & 1 deletion loader/cdnjs_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -34,6 +34,7 @@ import (

func TestCDNJS(t *testing.T) {
t.Skip("skipped to avoid inconsistent API responses")
t.Parallel()

paths := map[string]struct {
parts []string
Expand Down Expand Up @@ -69,12 +70,13 @@ func TestCDNJS(t *testing.T) {
},
}

var root = &url.URL{Scheme: "https", Host: "example.com", Path: "/something/"}
root := &url.URL{Scheme: "https", Host: "example.com", Path: "/something/"}
logger := logrus.New()
logger.SetOutput(testutils.NewTestOutput(t))
for path, expected := range paths {
path, expected := path, expected
t.Run(path, func(t *testing.T) {
t.Parallel()
name, loader, parts := pickLoader(path)
assert.Equal(t, "cdnjs", name)
assert.Equal(t, expected.parts, parts)
Expand All @@ -96,6 +98,7 @@ func TestCDNJS(t *testing.T) {
}

t.Run("cdnjs.com/libraries/nonexistent", func(t *testing.T) {
t.Parallel()
path := "cdnjs.com/libraries/nonexistent"
name, loader, parts := pickLoader(path)
assert.Equal(t, "cdnjs", name)
Expand All @@ -105,6 +108,7 @@ func TestCDNJS(t *testing.T) {
})

t.Run("cdnjs.com/libraries/Faker/3.1.0/nonexistent.js", func(t *testing.T) {
t.Parallel()
path := "cdnjs.com/libraries/Faker/3.1.0/nonexistent.js"
name, loader, parts := pickLoader(path)
assert.Equal(t, "cdnjs", name)
Expand Down
11 changes: 8 additions & 3 deletions loader/github_test.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -33,6 +33,7 @@ import (
)

func TestGithub(t *testing.T) {
t.Parallel()
logger := logrus.New()
logger.SetOutput(testutils.NewTestOutput(t))
path := "github.com/github/gitignore/Go.gitignore"
Expand All @@ -44,12 +45,13 @@ func TestGithub(t *testing.T) {
assert.NoError(t, err)
assert.Equal(t, expectedEndSrc, src)

var root = &url.URL{Scheme: "https", Host: "example.com", Path: "/something/"}
root := &url.URL{Scheme: "https", Host: "example.com", Path: "/something/"}
resolvedURL, err := Resolve(root, path)
require.NoError(t, err)
require.Empty(t, resolvedURL.Scheme)
require.Equal(t, path, resolvedURL.Opaque)
t.Run("not cached", func(t *testing.T) {
t.Parallel()
data, err := Load(logger, map[string]afero.Fs{"https": afero.NewMemMapFs()}, resolvedURL, path)
require.NoError(t, err)
assert.Equal(t, data.URL, resolvedURL)
Expand All @@ -58,10 +60,11 @@ func TestGithub(t *testing.T) {
})

t.Run("cached", func(t *testing.T) {
t.Parallel()
fs := afero.NewMemMapFs()
testData := []byte("test data")

err := afero.WriteFile(fs, "/github.com/github/gitignore/Go.gitignore", testData, 0644)
err := afero.WriteFile(fs, "/github.com/github/gitignore/Go.gitignore", testData, 0o644)
require.NoError(t, err)

data, err := Load(logger, map[string]afero.Fs{"https": fs}, resolvedURL, path)
Expand All @@ -71,7 +74,8 @@ func TestGithub(t *testing.T) {
})

t.Run("relative", func(t *testing.T) {
var tests = map[string]string{
t.Parallel()
tests := map[string]string{
"./something.else": "github.com/github/gitignore/something.else",
"../something.else": "github.com/github/something.else",
"/something.else": "github.com/something.else",
Expand All @@ -84,6 +88,7 @@ func TestGithub(t *testing.T) {
})

t.Run("dir", func(t *testing.T) {
t.Parallel()
require.Equal(t, &url.URL{Opaque: "github.com/github/gitignore"}, Dir(resolvedURL))
})
}
74 changes: 43 additions & 31 deletions loader/loader.go
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -18,9 +18,11 @@
*
*/

// Package loader is about loading files from either the filesystem or through https requests.
package loader

import (
"context"
"errors"
"fmt"
"io/ioutil"
Expand Down Expand Up @@ -97,33 +99,7 @@ func Resolve(pwd *url.URL, moduleSpecifier string) (*url.URL, error) {
}

if moduleSpecifier[0] == '.' || moduleSpecifier[0] == '/' || filepath.IsAbs(moduleSpecifier) {
if pwd.Opaque != "" { // this is a loader reference
parts := strings.SplitN(pwd.Opaque, "/", 2)
if moduleSpecifier[0] == '/' {
return &url.URL{Opaque: path.Join(parts[0], moduleSpecifier)}, nil
}
return &url.URL{Opaque: path.Join(parts[0], path.Join(path.Dir(parts[1]+"/"), moduleSpecifier))}, nil
}

// The file is in format like C:/something/path.js. But this will be decoded as scheme `C`
// ... which is not what we want we want it to be decode as file:///C:/something/path.js
if filepath.VolumeName(moduleSpecifier) != "" {
moduleSpecifier = "/" + moduleSpecifier
}

// we always want for the pwd to end in a slash, but filepath/path.Clean strips it so we read
// it if it's missing
var finalPwd = pwd
if pwd.Opaque != "" {
if !strings.HasSuffix(pwd.Opaque, "/") {
finalPwd = &url.URL{Opaque: pwd.Opaque + "/"}
}
} else if !strings.HasSuffix(pwd.Path, "/") {
finalPwd = &url.URL{}
*finalPwd = *pwd
finalPwd.Path += "/"
}
return finalPwd.Parse(moduleSpecifier)
return resolveFilePath(pwd, moduleSpecifier)
}

if strings.Contains(moduleSpecifier, "://") {
Expand Down Expand Up @@ -155,6 +131,36 @@ func Resolve(pwd *url.URL, moduleSpecifier string) (*url.URL, error) {
return &url.URL{Opaque: moduleSpecifier}, nil
}

func resolveFilePath(pwd *url.URL, moduleSpecifier string) (*url.URL, error) {
if pwd.Opaque != "" { // this is a loader reference
parts := strings.SplitN(pwd.Opaque, "/", 2)
if moduleSpecifier[0] == '/' {
return &url.URL{Opaque: path.Join(parts[0], moduleSpecifier)}, nil
}
return &url.URL{Opaque: path.Join(parts[0], path.Join(path.Dir(parts[1]+"/"), moduleSpecifier))}, nil
}

// The file is in format like C:/something/path.js. But this will be decoded as scheme `C`
// ... which is not what we want we want it to be decode as file:///C:/something/path.js
if filepath.VolumeName(moduleSpecifier) != "" {
moduleSpecifier = "/" + moduleSpecifier
}

// we always want for the pwd to end in a slash, but filepath/path.Clean strips it so we read
// it if it's missing
finalPwd := pwd
if pwd.Opaque != "" {
if !strings.HasSuffix(pwd.Opaque, "/") {
finalPwd = &url.URL{Opaque: pwd.Opaque + "/"}
}
} else if !strings.HasSuffix(pwd.Path, "/") {
finalPwd = &url.URL{}
*finalPwd = *pwd
finalPwd.Path += "/"
}
return finalPwd.Parse(moduleSpecifier)
}

// Dir returns the directory for the path.
func Dir(old *url.URL) *url.URL {
if old.Opaque != "" { // loader
Expand Down Expand Up @@ -203,7 +209,7 @@ func Load(
return nil, err
}
if scheme == "https" {
var finalModuleSpecifierURL = &url.URL{}
finalModuleSpecifierURL := &url.URL{}

switch {
case moduleSpecifier.Opaque != "": // This is loader
Expand All @@ -226,7 +232,7 @@ func Load(
result.URL = moduleSpecifier
// TODO maybe make an afero.Fs which makes request directly and than use CacheOnReadFs
// on top of as with the `file` scheme fs
_ = afero.WriteFile(filesystems[scheme], pathOnFs, result.Data, 0644)
_ = afero.WriteFile(filesystems[scheme], pathOnFs, result.Data, 0o644)
return result, nil
}

Expand Down Expand Up @@ -255,7 +261,7 @@ func resolveUsingLoaders(logger logrus.FieldLogger, name string) (*url.URL, erro
}

func loadRemoteURL(logger logrus.FieldLogger, u *url.URL) (*SourceData, error) {
var oldQuery = u.RawQuery
oldQuery := u.RawQuery
if u.RawQuery != "" {
u.RawQuery += "&"
}
Expand Down Expand Up @@ -292,7 +298,13 @@ func pickLoader(path string) (string, loaderFunc, []string) {
func fetch(logger logrus.FieldLogger, u string) ([]byte, error) {
logger.WithField("url", u).Debug("Fetching source...")
startTime := time.Now()
res, err := http.Get(u)
ctx, cancel := context.WithTimeout(context.Background(), time.Minute)
defer cancel()
req, err := http.NewRequestWithContext(ctx, http.MethodGet, u, nil)
if err != nil {
return nil, err
}
res, err := http.DefaultClient.Do(req)
if err != nil {
return nil, err
}
Expand Down
Loading