feat: Add version-based data parsing

This commit introduces the capability to parse data files from GitHub releases based on their semantic version tag. This is crucial for handling evolving data schemas from upstream sources like `web-features`.

Key changes:

-   **`lib/gh`**: `DownloadFileFromRelease` now returns a `*gh.File` struct containing both the file contents and the release's semver tag.

-   **`web_feature_consumer`**: A new `parseByVersion` method uses the release tag to select the appropriate parser. It currently supports routing to V2 and V3 parsers for `web-features` data, ensuring backward compatibility and readiness for the new schema.

-   **`bcd_consumer`**: Updated to use the new `*gh.File` struct, though it does not yet perform version-based parsing.

-   **Testing**: New tests have been added to validate the version-routing logic in the `web_feature_consumer`.

Author: jcscottiii

lib/gh/download_file_from_release.go

@@ -21,6 +21,7 @@ import (
 	"net/http"
 
 	"github.com/google/go-github/v73/github"
+	"golang.org/x/mod/semver"
 )
 
 var (
@@ -31,11 +32,21 @@ var (
 	ErrFatalError            = errors.New("fatal error using github")
 )
 
+type File 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) (*File, 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 +95,16 @@ func (c *Client) DownloadFileFromRelease(
 		return nil, errors.Join(ErrUnableToDownloadAsset, err)
 	}
 
-	return resp.Body, nil
+	tagName := release.GetTagName()
+	var tagNamePtr *string
+	if semver.IsValid(tagName) {
+		tagNamePtr = &tagName
+	}
+
+	return &File{
+		Contents: resp.Body,
+		Info: ReleaseInfo{
+			Tag: tagNamePtr,
+		},
+	}, nil
 }

lib/gh/download_file_from_release_test.go

@@ -34,6 +34,12 @@ func checkIfFileIsReadable(t *testing.T, file io.Reader) {
 	}
 }
 
+func checkIfTagIsPopulated(t *testing.T, file *File) {
+	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 +50,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 +66,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)
 	}
 }
 

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.File, 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.File
 	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.File, 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.File {
+		return &gh.File{
+			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

workflows/steps/services/web_feature_consumer/pkg/workflow/web_features_worker.go

@@ -16,13 +16,16 @@ package workflow
 
 import (
 	"context"
+	"errors"
 	"io"
 	"log/slog"
 	"net/http"
 	"time"
 
 	"github.com/GoogleChrome/webstatus.dev/lib/gen/jsonschema/web_platform_dx__web_features"
+	"github.com/GoogleChrome/webstatus.dev/lib/gh"
 	"github.com/GoogleChrome/webstatus.dev/lib/webdxfeaturetypes"
+	"golang.org/x/mod/semver"
 )
 
 // AssetGetter describes the behavior to get a certain asset from a github release.
@@ -31,14 +34,19 @@ type AssetGetter interface {
 		ctx context.Context,
 		owner, repo string,
 		httpClient *http.Client,
-		filePattern string) (io.ReadCloser, error)
+		filePattern string) (*gh.File, error)
 }
 
 // AssetParser describes the behavior to parse the io.ReadCloser from AssetGetter into the expected data type.
 type AssetParser interface {
 	Parse(io.ReadCloser) (*webdxfeaturetypes.ProcessedWebFeaturesData, error)
 }
 
+var (
+	ErrUnknownAssetVersion     = errors.New("unknown asset version")
+	ErrUnsupportedAssetVersion = errors.New("unsupported asset version")
+)
+
 // WebFeatureStorer describes the logic to insert the web features that were returned by the AssetParser.
 type WebFeatureStorer interface {
 	InsertWebFeatures(
@@ -86,25 +94,73 @@ func NewWebFeaturesJobProcessor(assetGetter AssetGetter,
 	metadataStorer WebFeatureMetadataStorer,
 	groupStorer WebDXGroupStorer,
 	snapshotStorer WebDXSnapshotStorer,
-	webFeaturesDataParser AssetParser,
+	webFeaturesDataV2Parser AssetParser,
+	webFeaturesDataV3Parser AssetParser,
 ) WebFeaturesJobProcessor {
 	return WebFeaturesJobProcessor{
-		assetGetter:           assetGetter,
-		storer:                storer,
-		metadataStorer:        metadataStorer,
-		groupStorer:           groupStorer,
-		snapshotStorer:        snapshotStorer,
-		webFeaturesDataParser: webFeaturesDataParser,
+		assetGetter:             assetGetter,
+		storer:                  storer,
+		metadataStorer:          metadataStorer,
+		groupStorer:             groupStorer,
+		snapshotStorer:          snapshotStorer,
+		webFeaturesDataV2Parser: webFeaturesDataV2Parser,
+		webFeaturesDataV3Parser: webFeaturesDataV3Parser,
 	}
 }
 
 type WebFeaturesJobProcessor struct {
-	assetGetter           AssetGetter
-	storer                WebFeatureStorer
-	metadataStorer        WebFeatureMetadataStorer
-	groupStorer           WebDXGroupStorer
-	snapshotStorer        WebDXSnapshotStorer
-	webFeaturesDataParser AssetParser
+	assetGetter             AssetGetter
+	storer                  WebFeatureStorer
+	metadataStorer          WebFeatureMetadataStorer
+	groupStorer             WebDXGroupStorer
+	snapshotStorer          WebDXSnapshotStorer
+	webFeaturesDataV2Parser AssetParser
+	webFeaturesDataV3Parser AssetParser
+}
+
+const (
+	// According to https://pkg.go.dev/golang.org/x/mod/semver, the version must start with "v".
+	v2 = "v2.0.0"
+	v3 = "v3.0.0"
+	v4 = "v4.0.0"
+)
+
+func (p WebFeaturesJobProcessor) parseByVersion(ctx context.Context, file *gh.File) (
+	*webdxfeaturetypes.ProcessedWebFeaturesData, error) {
+	if file.Info.Tag == nil {
+		slog.ErrorContext(ctx, "unknown version", "version", "nil")
+
+		return nil, ErrUnknownAssetVersion
+	}
+
+	if semver.Compare(*file.Info.Tag, v3) == -1 {
+		// If less than version 3, use default v2 parser
+		slog.InfoContext(ctx, "using v2 parser", "version", *file.Info.Tag)
+		data, err := p.webFeaturesDataV2Parser.Parse(file.Contents)
+		if err != nil {
+			slog.ErrorContext(ctx, "unable to parse v2 data", "error", err)
+
+			return nil, err
+		}
+
+		return data, nil
+
+	} else if semver.Compare(*file.Info.Tag, v4) == -1 {
+		// If version 3, use v3 parser
+		slog.InfoContext(ctx, "using v3 parser", "version", *file.Info.Tag)
+		data, err := p.webFeaturesDataV3Parser.Parse(file.Contents)
+		if err != nil {
+			slog.ErrorContext(ctx, "unable to parse v3 data", "error", err)
+
+			return nil, err
+		}
+
+		return data, nil
+	}
+
+	slog.ErrorContext(ctx, "unsupported version", "version", *file.Info.Tag)
+
+	return nil, ErrUnsupportedAssetVersion
 }
 
 func (p WebFeaturesJobProcessor) Process(ctx context.Context, job JobArguments) error {
@@ -120,7 +176,7 @@ func (p WebFeaturesJobProcessor) Process(ctx context.Context, job JobArguments)
 		return err
 	}
 
-	data, err := p.webFeaturesDataParser.Parse(file)
+	data, err := p.parseByVersion(ctx, file)
 	if err != nil {
 		slog.ErrorContext(ctx, "unable to parse data", "error", err)