Merge pull request #673 from hairyhenderson/stop-ignoring-args-for-mi…

…metypes-672 Fix bug where subpath was ignored when inferring MIME type
hairyhenderson · Nov 10, 2019 · cd14b59 · cd14b59
2 parents 677a9f9 + ed15277
commit cd14b59
Show file tree

Hide file tree

Showing 5 changed files with 126 additions and 39 deletions.
diff --git a/data/datasource.go b/data/datasource.go
@@ -160,13 +160,36 @@ func (s *Source) cleanup() {
 // 2. otherwise, the Type property on the Source is used, if present
 // 3. otherwise, a MIME type is calculated from the file extension, if the extension is registered
 // 4. otherwise, the default type of 'text/plain' is used
-func (s *Source) mimeType() (mimeType string, err error) {
-	mediatype := s.URL.Query().Get("type")
+func (s *Source) mimeType(arg string) (mimeType string, err error) {
+	if len(arg) > 0 {
+		if strings.HasPrefix(arg, "//") {
+			arg = arg[1:]
+		}
+		if !strings.HasPrefix(arg, "/") {
+			arg = "/" + arg
+		}
+	}
+	argURL, err := url.Parse(arg)
+	if err != nil {
+		return "", fmt.Errorf("mimeType: couldn't parse arg %q: %w", arg, err)
+	}
+	mediatype := argURL.Query().Get("type")
+	if mediatype == "" {
+		mediatype = s.URL.Query().Get("type")
+	}
+
 	if mediatype == "" {
 		mediatype = s.mediaType
 	}
+
 	// make it so + doesn't need to be escaped
 	mediatype = strings.ReplaceAll(mediatype, " ", "+")
+
+	if mediatype == "" {
+		ext := filepath.Ext(argURL.Path)
+		mediatype = mime.TypeByExtension(ext)
+	}
+
 	if mediatype == "" {
 		ext := filepath.Ext(s.URL.Path)
 		mediatype = mime.TypeByExtension(ext)
@@ -333,7 +356,11 @@ func (d *Data) readDataSource(alias string, args ...string) (data, mimeType stri
 		return "", "", errors.Wrapf(err, "Couldn't read datasource '%s'", alias)
 	}
 
-	mimeType, err = source.mimeType()
+	subpath := ""
+	if len(args) > 0 {
+		subpath = args[0]
+	}
+	mimeType, err = source.mimeType(subpath)
 	if err != nil {
 		return "", "", err
 	}

diff --git a/data/datasource_file_test.go b/data/datasource_file_test.go
@@ -51,7 +51,7 @@ func TestReadFile(t *testing.T) {
 	actual, err = readFile(source)
 	assert.NoError(t, err)
 	assert.Equal(t, []byte(`["bar.txt","baz.txt","foo.txt"]`), actual)
-	mime, err := source.mimeType()
+	mime, err := source.mimeType("")
 	assert.NoError(t, err)
 	assert.Equal(t, "application/json", mime)
 
@@ -60,7 +60,7 @@ func TestReadFile(t *testing.T) {
 	actual, err = readFile(source, "foo.txt")
 	assert.NoError(t, err)
 	assert.Equal(t, content, actual)
-	mime, err = source.mimeType()
+	mime, err = source.mimeType("")
 	assert.NoError(t, err)
 	assert.Equal(t, "application/json", mime)
 }
diff --git a/data/datasource_merge.go b/data/datasource_merge.go
@@ -42,7 +42,7 @@ func (d *Data) readMerge(source *Source, args ...string) ([]byte, error) {
 			return nil, errors.Wrapf(err, "Couldn't read datasource '%s'", part)
 		}
 
-		mimeType, err := subSource.mimeType()
+		mimeType, err := subSource.mimeType("")
 		if err != nil {
 			return nil, errors.Wrapf(err, "failed to read datasource %s", subSource.URL)
 		}

diff --git a/data/datasource_test.go b/data/datasource_test.go
@@ -306,44 +306,98 @@ func TestDefineDatasource(t *testing.T) {
 }
 
 func TestMimeType(t *testing.T) {
-	s := &Source{URL: mustParseURL("http://example.com/foo.json")}
-	mt, err := s.mimeType()
-	assert.NoError(t, err)
-	assert.Equal(t, jsonMimetype, mt)
-
-	s = &Source{URL: mustParseURL("http://example.com/foo.json"), mediaType: "text/foo"}
-	mt, err = s.mimeType()
-	assert.NoError(t, err)
-	assert.Equal(t, "text/foo", mt)
-
-	s = &Source{URL: mustParseURL("http://example.com/foo.json"), mediaType: "text/foo"}
-	mt, err = s.mimeType()
-	assert.NoError(t, err)
-	assert.Equal(t, "text/foo", mt)
-
-	s = &Source{URL: mustParseURL("http://example.com/foo.json?type=application/yaml"), mediaType: "text/foo"}
-	mt, err = s.mimeType()
-	assert.NoError(t, err)
-	assert.Equal(t, "application/yaml", mt)
+	s := &Source{URL: mustParseURL("http://example.com/list?type=a/b/c")}
+	_, err := s.mimeType("")
+	assert.Error(t, err)
 
-	s = &Source{URL: mustParseURL("http://example.com/list?type=application/array%2Bjson"), mediaType: "text/foo"}
-	mt, err = s.mimeType()
-	assert.NoError(t, err)
-	assert.Equal(t, "application/array+json", mt)
+	data := []struct {
+		url       string
+		mediaType string
+		expected  string
+	}{
+		{"http://example.com/foo.json",
+			"",
+			jsonMimetype},
+		{"http://example.com/foo.json",
+			"text/foo",
+			"text/foo"},
+		{"http://example.com/foo.json?type=application/yaml",
+			"text/foo",
+			"application/yaml"},
+		{"http://example.com/list?type=application/array%2Bjson",
+			"text/foo",
+			"application/array+json"},
+		{"http://example.com/list?type=application/array+json",
+			"",
+			"application/array+json"},
+		{"http://example.com/unknown",
+			"",
+			"text/plain"},
+	}
 
-	s = &Source{URL: mustParseURL("http://example.com/list?type=application/array+json")}
-	mt, err = s.mimeType()
-	assert.NoError(t, err)
-	assert.Equal(t, "application/array+json", mt)
+	for i, d := range data {
+		t.Run(fmt.Sprintf("%d:%q,%q==%q", i, d.url, d.mediaType, d.expected), func(t *testing.T) {
+			s := &Source{URL: mustParseURL(d.url), mediaType: d.mediaType}
+			mt, err := s.mimeType("")
+			assert.NoError(t, err)
+			assert.Equal(t, d.expected, mt)
+		})
+	}
+}
 
-	s = &Source{URL: mustParseURL("http://example.com/list?type=a/b/c")}
-	_, err = s.mimeType()
+func TestMimeTypeWithArg(t *testing.T) {
+	s := &Source{URL: mustParseURL("http://example.com")}
+	_, err := s.mimeType("h\nttp://foo")
 	assert.Error(t, err)
 
-	s = &Source{URL: mustParseURL("http://example.com/unknown")}
-	mt, err = s.mimeType()
-	assert.NoError(t, err)
-	assert.Equal(t, "text/plain", mt)
+	data := []struct {
+		url       string
+		mediaType string
+		arg       string
+		expected  string
+	}{
+		{"http://example.com/unknown",
+			"",
+			"/foo.json",
+			"application/json"},
+		{"http://example.com/unknown",
+			"",
+			"foo.json",
+			"application/json"},
+		{"http://example.com/",
+			"text/foo",
+			"/foo.json",
+			"text/foo"},
+		{"git+https://example.com/myrepo",
+			"",
+			"//foo.yaml",
+			"application/yaml"},
+		{"http://example.com/foo.json",
+			"",
+			"/foo.yaml",
+			"application/yaml"},
+		{"http://example.com/foo.json?type=application/array+yaml",
+			"",
+			"/foo.yaml",
+			"application/array+yaml"},
+		{"http://example.com/foo.json?type=application/array+yaml",
+			"",
+			"/foo.yaml?type=application/yaml",
+			"application/yaml"},
+		{"http://example.com/foo.json?type=application/array+yaml",
+			"text/plain",
+			"/foo.yaml?type=application/yaml",
+			"application/yaml"},
+	}
+
+	for i, d := range data {
+		t.Run(fmt.Sprintf("%d:%q,%q,%q==%q", i, d.url, d.mediaType, d.arg, d.expected), func(t *testing.T) {
+			s := &Source{URL: mustParseURL(d.url), mediaType: d.mediaType}
+			mt, err := s.mimeType(d.arg)
+			assert.NoError(t, err)
+			assert.Equal(t, d.expected, mt)
+		})
+	}
 }
 
 func TestQueryParse(t *testing.T) {

diff --git a/tests/integration/datasources_file_test.go b/tests/integration/datasources_file_test.go
@@ -72,6 +72,12 @@ func (s *FileDatasourcesSuite) TestFileDatasources(c *C) {
 	)
 	result.Assert(c, icmd.Expected{ExitCode: 0, Out: "baz"})
 
+	result = icmd.RunCommand(GomplateBin,
+		"-d", "dir="+s.tmpDir.Path(),
+		"-i", `{{ (datasource "dir" "config.json").foo.bar }}`,
+	)
+	result.Assert(c, icmd.Expected{ExitCode: 0, Out: "baz"})
+
 	result = icmd.RunCmd(icmd.Command(GomplateBin,
 		"-d", "config=config.json",
 		"-i", `{{ (ds "config").foo.bar }}`,