feat: Add version-based data parsing

lib/gh/download_file_from_release.go

@@ -18,9 +18,12 @@ import (
 	"context"
 	"errors"
 	"io"
+	"log/slog"
 	"net/http"
+	"strings"
 
 	"github.com/google/go-github/v73/github"
+	"golang.org/x/mod/semver"
 )
 
 var (
@@ -31,11 +34,22 @@ var (
 	ErrFatalError            = errors.New("fatal error using github")
 )
 
+// ReleaseFile represents a file in a given Github release.
+type ReleaseFile struct {
+	Contents io.ReadCloser
+	Info     ReleaseInfo
+}
+
+type ReleaseInfo struct {
+	// If the tag is valid, the will be non null.
+	Tag *string
+}
+
 func (c *Client) DownloadFileFromRelease(
 	ctx context.Context,
 	owner, repo string,
 	httpClient *http.Client,
-	filePattern string) (io.ReadCloser, error) {
+	filePattern string) (*ReleaseFile, error) {
 	release, _, err := c.repoClient.GetLatestRelease(ctx, owner, repo)
 	if err != nil {
 		// nolint: exhaustruct // WONTFIX. This is an external package. Cannot control it.
@@ -84,5 +98,24 @@ func (c *Client) DownloadFileFromRelease(
 		return nil, errors.Join(ErrUnableToDownloadAsset, err)
 	}
 
-	return resp.Body, nil
+	// Returns a tag or empty string if not found.
+	tagName := release.GetTagName()
+	// In the event the tag is missing the prefix "v", add it.
+	// According https://pkg.go.dev/golang.org/x/mod/semver, the version must start with v
+	if len(tagName) > 0 && !strings.HasPrefix(tagName, "v") {
+		tagName = "v" + tagName
+	}
+	var tagNamePtr *string
+	if semver.IsValid(tagName) {
+		tagNamePtr = &tagName
+	} else {
+		slog.WarnContext(ctx, "invalid tag. it will not be used", "tag", tagName)
+	}
+
+	return &ReleaseFile{
+		Contents: resp.Body,
+		Info: ReleaseInfo{
+			Tag: tagNamePtr,
+		},
+	}, nil
 }

lib/gh/download_file_from_release_test.go

@@ -21,6 +21,7 @@ import (
 	"net/http"
 	"testing"
 
+	"github.com/google/go-cmp/cmp"
 	"github.com/google/go-github/v73/github"
 )
 
@@ -34,6 +35,12 @@ func checkIfFileIsReadable(t *testing.T, file io.Reader) {
 	}
 }
 
+func checkIfTagIsPopulated(t *testing.T, file *ReleaseFile) {
+	if file.Info.Tag == nil || *file.Info.Tag == "" {
+		t.Error("tag is empty")
+	}
+}
+
 func TestDownloadFileFromReleaseWebFeatures(t *testing.T) {
 	t.Skip("Used for debugging purposes.")
 	client := NewClient("")
@@ -44,14 +51,14 @@ func TestDownloadFileFromReleaseWebFeatures(t *testing.T) {
 		t.Errorf("unexpected error: %s", err.Error())
 	} else {
 		// Close to be safe at the end.
-		defer file.Close()
-		checkIfFileIsReadable(t, file)
+		defer file.Contents.Close()
+		checkIfFileIsReadable(t, file.Contents)
+		checkIfTagIsPopulated(t, file)
 	}
 }
 
 func TestDownloadFileFromReleaseBrowserCompatData(t *testing.T) {
 	t.Skip("Used for debugging purposes.")
-	t.Skip("Cannot remove until https://github.com/mdn/browser-compat-data/issues/22675 is fixed")
 	client := NewClient("")
 	ctx := context.Background()
 	httpClient := http.DefaultClient
@@ -60,8 +67,9 @@ func TestDownloadFileFromReleaseBrowserCompatData(t *testing.T) {
 		t.Errorf("unexpected error: %s", err.Error())
 	} else {
 		// Close to be safe at the end.
-		defer file.Close()
-		checkIfFileIsReadable(t, file)
+		defer file.Contents.Close()
+		checkIfFileIsReadable(t, file.Contents)
+		checkIfTagIsPopulated(t, file)
 	}
 }
 
@@ -113,6 +121,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 		cfg           mockGetLatestReleaseConfig
 		roundTripCfg  *mockRoundTripperConfig
 		expectedError error
+		expectedFile  *ReleaseFile
 	}{
 		{
 			name: "successful download",
@@ -127,6 +136,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 							BrowserDownloadURL: valuePtr("http://example.com/file.txt"),
 						},
 					},
+					TagName: valuePtr("v1.0.0"),
 				},
 				err: nil,
 			},
@@ -135,10 +145,51 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 				//nolint: exhaustruct
 				resp: &http.Response{
 					StatusCode: http.StatusOK,
+					Body:       io.NopCloser(nil),
 				},
 				err: nil,
 			},
 			expectedError: nil,
+			expectedFile: &ReleaseFile{
+				Contents: io.NopCloser(nil),
+				Info: ReleaseInfo{
+					Tag: valuePtr("v1.0.0"),
+				},
+			},
+		},
+		{
+			name: "successful download without leading v in tag",
+			cfg: mockGetLatestReleaseConfig{
+				expectedOwner: "owner",
+				expectedRepo:  "repo",
+				//nolint: exhaustruct
+				release: &github.RepositoryRelease{
+					Assets: []*github.ReleaseAsset{
+						{
+							Name:               valuePtr("file.txt"),
+							BrowserDownloadURL: valuePtr("http://example.com/file.txt"),
+						},
+					},
+					TagName: valuePtr("2.0.0"),
+				},
+				err: nil,
+			},
+			roundTripCfg: &mockRoundTripperConfig{
+				expectedURL: "http://example.com/file.txt",
+				//nolint: exhaustruct
+				resp: &http.Response{
+					StatusCode: http.StatusOK,
+					Body:       io.NopCloser(nil),
+				},
+				err: nil,
+			},
+			expectedError: nil,
+			expectedFile: &ReleaseFile{
+				Contents: io.NopCloser(nil),
+				Info: ReleaseInfo{
+					Tag: valuePtr("v2.0.0"),
+				},
+			},
 		},
 		{
 			name: "rate limit with github api",
@@ -151,6 +202,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 			},
 			roundTripCfg:  nil,
 			expectedError: ErrRateLimit,
+			expectedFile:  nil,
 		},
 		{
 			name: "unknown error with github api",
@@ -162,6 +214,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 			},
 			roundTripCfg:  nil,
 			expectedError: ErrFatalError,
+			expectedFile:  nil,
 		},
 		{
 			name: "missing asset",
@@ -174,6 +227,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 			},
 			roundTripCfg:  nil,
 			expectedError: ErrAssetNotFound,
+			expectedFile:  nil,
 		},
 		{
 			name: "request.DO() fails",
@@ -198,6 +252,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 				err:  errors.New("something went wrong"),
 			},
 			expectedError: ErrUnableToDownloadAsset,
+			expectedFile:  nil,
 		},
 		{
 			name: "failed to download",
@@ -224,6 +279,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 				err: nil,
 			},
 			expectedError: ErrUnableToDownloadAsset,
+			expectedFile:  nil,
 		},
 	}
 	for _, tc := range testCases {
@@ -241,7 +297,7 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 
 			httpClient := http.DefaultClient
 			httpClient.Transport = &rt
-			_, err := client.DownloadFileFromRelease(
+			file, err := client.DownloadFileFromRelease(
 				context.Background(),
 				"owner",
 				"repo",
@@ -251,6 +307,9 @@ func TestMockDownloadFileFromRelease(t *testing.T) {
 			if !errors.Is(err, tc.expectedError) {
 				t.Errorf("unexpected error expected: %v received: %v", tc.expectedError, err)
 			}
+			if diff := cmp.Diff(tc.expectedFile, file); diff != "" {
+				t.Errorf("unexpected file (-want +got):\n%s", diff)
+			}
 		})
 	}
 }

lib/go.mod

@@ -68,6 +68,7 @@ require (
 	go.opentelemetry.io/otel/exporters/stdout/stdoutlog v0.13.0 // indirect
 	go.opentelemetry.io/otel/log v0.13.0 // indirect
 	go.opentelemetry.io/otel/sdk/log v0.13.0 // indirect
+	golang.org/x/mod v0.27.0 // indirect
 	google.golang.org/appengine/v2 v2.0.6 // indirect
 )
 

lib/go.sum

@@ -1253,6 +1253,8 @@ golang.org/x/mod v0.6.0-dev.0.20220419223038-86c51ed26bb4/go.mod h1:jJ57K6gSWd91
 golang.org/x/mod v0.7.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
 golang.org/x/mod v0.8.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
 golang.org/x/mod v0.9.0/go.mod h1:iBbtSCu2XBx23ZKBPSOrRkjjQPZFPuis4dIYUhu/chs=
+golang.org/x/mod v0.27.0 h1:kb+q2PyFnEADO2IEF935ehFUXlWiNjJWtRNgBLSfbxQ=
+golang.org/x/mod v0.27.0/go.mod h1:rWI627Fq0DEoudcK+MBkNkCe0EetEaDSwJJkCcjpazc=
 golang.org/x/net v0.0.0-20180724234803-3673e40ba225/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20180826012351-8a410e7b638d/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=
 golang.org/x/net v0.0.0-20190108225652-1e06a53dbb7e/go.mod h1:mL1N/T3taQHkDXs73rZJwtUhF3w3ftmwwsq0BUmARs4=

workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker.go

@@ -20,6 +20,7 @@ import (
 	"net/http"
 
 	"github.com/GoogleChrome/webstatus.dev/lib/gcpspanner/spanneradapters/bcdconsumertypes"
+	"github.com/GoogleChrome/webstatus.dev/lib/gh"
 	"github.com/GoogleChrome/webstatus.dev/workflows/steps/services/bcd_consumer/pkg/data"
 )
 
@@ -45,7 +46,7 @@ type DataGetter interface {
 		ctx context.Context,
 		owner, repo string,
 		httpClient *http.Client,
-		filePattern string) (io.ReadCloser, error)
+		filePattern string) (*gh.ReleaseFile, error)
 }
 
 // DataParser describes the behavior to read raw bytes into the expected BCDData struct.
@@ -106,7 +107,7 @@ func (p BCDJobProcessor) Process(
 	}
 
 	// Step 2. Parse the file.
-	data, err := p.dataParser.Parse(file)
+	data, err := p.dataParser.Parse(file.Contents)
 	if err != nil {
 		return err
 	}

workflows/steps/services/bcd_consumer/pkg/workflow/bcd_releases_worker_test.go

@@ -27,6 +27,7 @@ import (
 
 	"github.com/GoogleChrome/webstatus.dev/lib/gcpspanner/spanneradapters/bcdconsumertypes"
 	"github.com/GoogleChrome/webstatus.dev/lib/gen/jsonschema/mdn__browser_compat_data"
+	"github.com/GoogleChrome/webstatus.dev/lib/gh"
 	"github.com/GoogleChrome/webstatus.dev/workflows/steps/services/bcd_consumer/pkg/data"
 )
 
@@ -56,7 +57,7 @@ type mockDownloadFileFromReleaseConfig struct {
 	repoOwner   string
 	repoName    string
 	filePattern string
-	fakeFile    io.ReadCloser
+	fakeFile    *gh.ReleaseFile
 	err         error
 }
 
@@ -69,7 +70,7 @@ func (m *MockDataGetter) DownloadFileFromRelease(
 	_ context.Context,
 	owner, repo string,
 	_ *http.Client,
-	filePattern string) (io.ReadCloser, error) {
+	filePattern string) (*gh.ReleaseFile, error) {
 	if m.mockDownloadFileFromReleaseCfg.repoOwner != owner ||
 		m.mockDownloadFileFromReleaseCfg.repoName != repo ||
 		m.mockDownloadFileFromReleaseCfg.filePattern != filePattern {
@@ -97,7 +98,8 @@ func (m *MockDataParser) Parse(in io.ReadCloser) (*data.BCDData, error) {
 		m.t.Errorf("unable to read file")
 	}
 	if m.mockParseCfg.expectedFileContents != string(fileContents) {
-		m.t.Error("unexpected file contents")
+		m.t.Errorf("unexpected file contents. want: %s, got: %s",
+			m.mockParseCfg.expectedFileContents, string(fileContents))
 	}
 
 	return m.mockParseCfg.ret, m.mockParseCfg.err
@@ -205,6 +207,16 @@ func getSampleReleases() []bcdconsumertypes.BrowserRelease {
 }
 
 func TestProcess(t *testing.T) {
+	// Create a function to generate a file because Contents can only be read once
+	testFileFn := func() *gh.ReleaseFile {
+		return &gh.ReleaseFile{
+			Contents: io.NopCloser(strings.NewReader("success")),
+			Info: gh.ReleaseInfo{
+				Tag: nil,
+			},
+		}
+	}
+
 	testCases := []processWorkflowTest{
 		{
 			name: "successful process",
@@ -215,7 +227,7 @@ func TestProcess(t *testing.T) {
 				repoOwner:   repoOwner,
 				repoName:    repoName,
 				filePattern: filePattern,
-				fakeFile:    io.NopCloser(strings.NewReader("success")),
+				fakeFile:    testFileFn(),
 				err:         nil,
 			},
 			mockParseCfg: &mockParseConfig{
@@ -244,7 +256,7 @@ func TestProcess(t *testing.T) {
 				repoOwner:   repoOwner,
 				repoName:    repoName,
 				filePattern: filePattern,
-				fakeFile:    io.NopCloser(strings.NewReader("success")),
+				fakeFile:    testFileFn(),
 				err:         errTestGetter,
 			},
 			mockParseCfg:                 nil,
@@ -261,7 +273,7 @@ func TestProcess(t *testing.T) {
 				repoOwner:   repoOwner,
 				repoName:    repoName,
 				filePattern: filePattern,
-				fakeFile:    io.NopCloser(strings.NewReader("success")),
+				fakeFile:    testFileFn(),
 				err:         nil,
 			},
 			mockParseCfg: &mockParseConfig{
@@ -282,7 +294,7 @@ func TestProcess(t *testing.T) {
 				repoOwner:   repoOwner,
 				repoName:    repoName,
 				filePattern: filePattern,
-				fakeFile:    io.NopCloser(strings.NewReader("success")),
+				fakeFile:    testFileFn(),
 				err:         nil,
 			},
 			mockParseCfg: &mockParseConfig{
@@ -308,7 +320,7 @@ func TestProcess(t *testing.T) {
 				repoOwner:   repoOwner,
 				repoName:    repoName,
 				filePattern: filePattern,
-				fakeFile:    io.NopCloser(strings.NewReader("success")),
+				fakeFile:    testFileFn(),
 				err:         nil,
 			},
 			mockParseCfg: &mockParseConfig{

workflows/steps/services/web_feature_consumer/cmd/job/main.go

@@ -92,6 +92,7 @@ func main() {
 		spanneradapters.NewWebFeatureGroupsConsumer(spannerClient),
 		spanneradapters.NewWebFeatureSnapshotsConsumer(spannerClient),
 		data.Parser{},
+		data.V3Parser{},
 	)
 
 	// Job Generation