Skip to content

Commit

Permalink
Merge pull request #851 from hairyhenderson/deduplicate-url-parsing-f…
Browse files Browse the repository at this point in the history
…uncs

refactoring: deduplicating identical URL-parsing functions
  • Loading branch information
Dave Henderson authored and GitHub committed May 21, 2020
2 parents 7597dd9 + aa527c4 commit da66c34
Show file tree
Hide file tree
Showing 8 changed files with 199 additions and 307 deletions.
2 changes: 1 addition & 1 deletion Makefile
Original file line number Diff line number Diff line change
Expand Up @@ -152,7 +152,7 @@ gomplate.png: gomplate.svg
cloudconvert -f png -c density=288 $^

lint:
golangci-lint --version
@golangci-lint --version
@golangci-lint run --timeout 2m --disable-all \
--enable depguard \
--enable dupl \
Expand Down
109 changes: 5 additions & 104 deletions data/datasource.go
Original file line number Diff line number Diff line change
Expand Up @@ -8,7 +8,6 @@ import (
"net/http"
"net/url"
"os"
"path"
"path/filepath"
"strings"

Expand Down Expand Up @@ -101,28 +100,14 @@ func (d *Data) Cleanup() {
}

// NewData - constructor for Data
// Deprecated: will be replaced in future
func NewData(datasourceArgs, headerArgs []string) (*Data, error) {
headers, err := parseHeaderArgs(headerArgs)
cfg := &config.Config{}
err := cfg.ParseDataSourceFlags(datasourceArgs, nil, headerArgs)
if err != nil {
return nil, err
}

data := &Data{
Sources: make(map[string]*Source),
extraHeaders: headers,
}

for _, v := range datasourceArgs {
s, err := parseSource(v)
if err != nil {
return nil, errors.Wrapf(err, "error parsing datasource")
}
s.header = headers[s.Alias]
// pop the header out of the map, so we end up with only the unreferenced ones
delete(headers, s.Alias)

data.Sources[s.Alias] = s
}
data := FromConfig(cfg)
return data, nil
}

Expand Down Expand Up @@ -242,90 +227,6 @@ func (s *Source) String() string {
return fmt.Sprintf("%s=%s (%s)", s.Alias, s.URL.String(), s.mediaType)
}

// parseSource creates a *Source by parsing the value provided to the
// --datasource/-d commandline flag
func parseSource(value string) (source *Source, err error) {
source = &Source{}
parts := strings.SplitN(value, "=", 2)
if len(parts) == 1 {
f := parts[0]
source.Alias = strings.SplitN(value, ".", 2)[0]
if path.Base(f) != f {
err = errors.Errorf("Invalid datasource (%s). Must provide an alias with files not in working directory", value)
return nil, err
}
source.URL, err = absFileURL(f)
if err != nil {
return nil, err
}
} else if len(parts) == 2 {
source.Alias = parts[0]
source.URL, err = parseSourceURL(parts[1])
if err != nil {
return nil, err
}
}

return source, nil
}

func parseSourceURL(value string) (*url.URL, error) {
if value == "-" {
value = "stdin://"
}
value = filepath.ToSlash(value)
// handle absolute Windows paths
volName := ""
if volName = filepath.VolumeName(value); volName != "" {
// handle UNCs
if len(volName) > 2 {
value = "file:" + value
} else {
value = "file:///" + value
}
}
srcURL, err := url.Parse(value)
if err != nil {
return nil, err
}

if volName != "" {
if strings.HasPrefix(srcURL.Path, "/") && srcURL.Path[2] == ':' {
srcURL.Path = srcURL.Path[1:]
}
}

if !srcURL.IsAbs() {
srcURL, err = absFileURL(value)
if err != nil {
return nil, err
}
}
return srcURL, nil
}

func absFileURL(value string) (*url.URL, error) {
cwd, err := os.Getwd()
if err != nil {
return nil, errors.Wrapf(err, "can't get working directory")
}
urlCwd := filepath.ToSlash(cwd)
baseURL := &url.URL{
Scheme: "file",
Path: urlCwd + "/",
}
relURL, err := url.Parse(value)
if err != nil {
return nil, fmt.Errorf("can't parse value %s as URL: %w", value, err)
}
resolved := baseURL.ResolveReference(relURL)
// deal with Windows drive letters
if !strings.HasPrefix(urlCwd, "/") && resolved.Path[2] == ':' {
resolved.Path = resolved.Path[1:]
}
return resolved, nil
}

// DefineDatasource -
func (d *Data) DefineDatasource(alias, value string) (string, error) {
if alias == "" {
Expand All @@ -334,7 +235,7 @@ func (d *Data) DefineDatasource(alias, value string) (string, error) {
if d.DatasourceExists(alias) {
return "", nil
}
srcURL, err := parseSourceURL(value)
srcURL, err := config.ParseSourceURL(value)
if err != nil {
return "", err
}
Expand Down
38 changes: 0 additions & 38 deletions data/datasource_http.go
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@ import (
"mime"
"net/http"
"net/url"
"strings"
"time"

"github.com/pkg/errors"
Expand Down Expand Up @@ -61,40 +60,3 @@ func readHTTP(source *Source, args ...string) ([]byte, error) {
}
return body, nil
}

func parseHeaderArgs(headerArgs []string) (map[string]http.Header, error) {
headers := make(map[string]http.Header)
for _, v := range headerArgs {
ds, name, value, err := splitHeaderArg(v)
if err != nil {
return nil, err
}
if _, ok := headers[ds]; !ok {
headers[ds] = make(http.Header)
}
headers[ds][name] = append(headers[ds][name], strings.TrimSpace(value))
}
return headers, nil
}

func splitHeaderArg(arg string) (datasourceAlias, name, value string, err error) {
parts := strings.SplitN(arg, "=", 2)
if len(parts) != 2 {
err = errors.Errorf("Invalid datasource-header option '%s'", arg)
return "", "", "", err
}
datasourceAlias = parts[0]
name, value, err = splitHeader(parts[1])
return datasourceAlias, name, value, err
}

func splitHeader(header string) (name, value string, err error) {
parts := strings.SplitN(header, ":", 2)
if len(parts) != 2 {
err = errors.Errorf("Invalid HTTP Header format '%s'", header)
return "", "", err
}
name = http.CanonicalHeaderKey(parts[0])
value = parts[1]
return name, value, nil
}
44 changes: 0 additions & 44 deletions data/datasource_http_test.go
Original file line number Diff line number Diff line change
Expand Up @@ -118,50 +118,6 @@ func TestHTTPFileWithHeaders(t *testing.T) {
assert.Equal(t, must(marshalObj(expected, json.Marshal)), must(marshalObj(actual, json.Marshal)))
}

func TestParseHeaderArgs(t *testing.T) {
args := []string{
"foo=Accept: application/json",
"bar=Authorization: Bearer supersecret",
}
expected := map[string]http.Header{
"foo": {
"Accept": {jsonMimetype},
},
"bar": {
"Authorization": {"Bearer supersecret"},
},
}
parsed, err := parseHeaderArgs(args)
assert.NoError(t, err)
assert.Equal(t, expected, parsed)

_, err = parseHeaderArgs([]string{"foo"})
assert.Error(t, err)

_, err = parseHeaderArgs([]string{"foo=bar"})
assert.Error(t, err)

args = []string{
"foo=Accept: application/json",
"foo=Foo: bar",
"foo=foo: baz",
"foo=fOO: qux",
"bar=Authorization: Bearer supersecret",
}
expected = map[string]http.Header{
"foo": {
"Accept": {jsonMimetype},
"Foo": {"bar", "baz", "qux"},
},
"bar": {
"Authorization": {"Bearer supersecret"},
},
}
parsed, err = parseHeaderArgs(args)
assert.NoError(t, err)
assert.Equal(t, expected, parsed)
}

func TestBuildURL(t *testing.T) {
expected := "https://example.com/index.html"
base := mustParseURL(expected)
Expand Down
7 changes: 6 additions & 1 deletion data/datasource_merge.go
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@ import (
"strings"

"github.com/hairyhenderson/gomplate/v3/coll"
"github.com/hairyhenderson/gomplate/v3/internal/config"

"github.com/pkg/errors"
)
Expand All @@ -30,10 +31,14 @@ func (d *Data) readMerge(source *Source, args ...string) ([]byte, error) {
subSource, err := d.lookupSource(part)
if err != nil {
// maybe it's a relative filename?
subSource, err = parseSource(part + "=" + part)
u, err := config.ParseSourceURL(part)
if err != nil {
return nil, err
}
subSource = &Source{
Alias: part,
URL: u,
}
}
subSource.inherit(source)

Expand Down
Loading

0 comments on commit da66c34

Please sign in to comment.