Kevin Burke (Gerrit)

unread,

Dec 2, 2025, 4:41:49 PM (11 hours ago) Dec 2

to goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Kevin Burke has uploaded the change for review

Commit message

maintner, cmd/gerritbot: migrate to bartventer/httpcache

Replace archived gregjones/httpcache (last updated 2018, archived
2023) with bartventer/httpcache, which is actively updated.

The new library simplifies the caching implementation by handling
cache control based on HTTP headers rather than custom URL filtering
logic. This provides standards-compliant behavior with backward
compatibility via the X-From-Cache header.

Updates both maintner and cmd/gerritbot to use the new library with
the same httpcache.NewTransport() API pattern.

Also remove outdated TODO comment from LastModified() - it turns out
the code did the right thing - and add comprehensive tests verifying
the function correctly tracks the most recent update time across
issues, comments, events, and reviews.

Change-Id: I3566f4ca012909a7e2292e52e79fdaa1995bb847

Change diff

diff --git a/cmd/gerritbot/gerritbot.go b/cmd/gerritbot/gerritbot.go
index b2eb50d..36f427c 100644
--- a/cmd/gerritbot/gerritbot.go
+++ b/cmd/gerritbot/gerritbot.go
@@ -25,8 +25,8 @@
 	"time"
 
 	"cloud.google.com/go/compute/metadata"
+	"github.com/bartventer/httpcache"
 	"github.com/google/go-github/v74/github"
-	"github.com/gregjones/httpcache"
 	"golang.org/x/build/cmd/gerritbot/internal/rules"
 	"golang.org/x/build/gerrit"
 	"golang.org/x/build/internal/https"
@@ -120,11 +120,10 @@
 	oauthTransport := &oauth2.Transport{
 		Source: oauth2.StaticTokenSource(&oauth2.Token{AccessToken: token}),
 	}
-	cachingTransport := &httpcache.Transport{
-		Transport:           oauthTransport,
-		Cache:               httpcache.NewMemoryCache(),
-		MarkCachedResponses: true,
-	}
+	cachingTransport := httpcache.NewTransport(
+		"memcache://",
+		httpcache.WithUpstream(oauthTransport),
+	)
 	httpClient := &http.Client{
 		Transport: cachingTransport,
 	}
diff --git a/go.mod b/go.mod
index c417eb6..902fefd 100644
--- a/go.mod
+++ b/go.mod
@@ -1,6 +1,6 @@
 module golang.org/x/build
 
-go 1.24.0
+go 1.25
 
 require (
 	cloud.google.com/go/bigquery v1.53.0
@@ -20,6 +20,7 @@
 	github.com/aclements/go-moremath v0.0.0-20210112150236-f10218a38794
 	github.com/ajstarks/svgo v0.0.0-20211024235047-1546f124cd8b
 	github.com/aws/aws-sdk-go v1.30.15
+	github.com/bartventer/httpcache v0.12.0
 	github.com/bradfitz/go-smtpd v0.0.0-20170404230938-deb6d6237625
 	github.com/creack/pty v1.1.23
 	github.com/davecgh/go-spew v1.1.1
@@ -35,7 +36,6 @@
 	github.com/google/uuid v1.6.0
 	github.com/googleapis/gax-go/v2 v2.12.0
 	github.com/googleapis/google-cloud-go-testing v0.0.0-20200911160855-bcd43fbb19e8
-	github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7
 	github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4
 	github.com/influxdata/influxdb-client-go/v2 v2.8.0
 	github.com/jackc/pgconn v1.14.3
diff --git a/go.sum b/go.sum
index f3c4dc7..b6c7ddd 100644
--- a/go.sum
+++ b/go.sum
@@ -158,6 +158,8 @@
 github.com/aws/aws-sdk-go-v2/service/sts v1.4.1/go.mod h1:G9osDWA52WQ38BDcj65VY1cNmcAQXAXTsE8IWH8j81w=
 github.com/aws/smithy-go v1.3.1/go.mod h1:SObp3lf9smib00L/v3U2eAKG8FyQ7iLrJnQiAmR5n+E=
 github.com/aws/smithy-go v1.4.0/go.mod h1:SObp3lf9smib00L/v3U2eAKG8FyQ7iLrJnQiAmR5n+E=
+github.com/bartventer/httpcache v0.12.0 h1:GzOgiAD0fuM+PTmhxlsai3WJKr9dk6NBvjkuEZyLCpA=
+github.com/bartventer/httpcache v0.12.0/go.mod h1:78LO7c36hcyx2GQanbDozHTozNaIL32MT4N+lErHHFs=
 github.com/bazelbuild/remote-apis v0.0.0-20230411132548-35aee1c4a425 h1:Lj8uXWW95oXyYguUSdQDvzywQb4f0jbJWsoLPQWAKTY=
 github.com/bazelbuild/remote-apis v0.0.0-20230411132548-35aee1c4a425/go.mod h1:ry8Y6CkQqCVcYsjPOlLXDX2iRVjOnjogdNwhvHmRcz8=
 github.com/bazelbuild/remote-apis-sdks v0.0.0-20230809203756-67f2ffbec0ef h1:H3VH2LBOyhrzU7nRvlzE4UxidMlLVZtIXPZdu2/sQ/A=
@@ -434,8 +436,6 @@
 github.com/gorilla/mux v1.8.0/go.mod h1:DVbg23sWSpFRCP0SfiEN6jmj59UnW/n46BH5rLB71So=
 github.com/gorilla/websocket v1.5.0 h1:PPwGk2jz7EePpoHN/+ClbZu8SPxiqlu12wZP/3sWmnc=
 github.com/gorilla/websocket v1.5.0/go.mod h1:YR8l580nyteQvAITg2hZ9XVh4b55+EU/adAjf1fMHhE=
-github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7 h1:pdN6V1QBWetyv/0+wjACpqVH+eVULgEjkurDLq3goeM=
-github.com/gregjones/httpcache v0.0.0-20180305231024-9cad4c3443a7/go.mod h1:FecbI9+v66THATjSRHfNgh1IVFe/9kFxbXtjV0ctIMA=
 github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4 h1:z53tR0945TRRQO/fLEVPI6SMv7ZflF0TEaTAoU7tOzg=
 github.com/grpc-ecosystem/go-grpc-middleware v1.0.1-0.20190118093823-f849b5445de4/go.mod h1:FiyG127CGDf3tlThmgyCl78X/SZQqEOJBCDaAfeWzPs=
 github.com/grpc-ecosystem/grpc-gateway v1.16.0/go.mod h1:BDjrQk3hbvj6Nolgz8mAMFbcEtjT1g+wF4CSlocrBnw=
diff --git a/maintner/cmd/maintserve/go.mod b/maintner/cmd/maintserve/go.mod
index bff7952..34ef610 100644
--- a/maintner/cmd/maintserve/go.mod
+++ b/maintner/cmd/maintserve/go.mod
@@ -1,6 +1,6 @@
 module golang.org/x/build/maintner/cmd/maintserve
 
-go 1.24.0
+go 1.25
 
 require (
 	dmitri.shuralyov.com/app/changes v0.0.0-20191223015216-e22f40b36873
diff --git a/maintner/github.go b/maintner/github.go
index a4b4732..98ef258 100644
--- a/maintner/github.go
+++ b/maintner/github.go
@@ -21,8 +21,11 @@
 	"strings"
 	"time"
 
+	"github.com/bartventer/httpcache"
+	"github.com/bartventer/httpcache/store"
+	"github.com/bartventer/httpcache/store/driver"
+	"github.com/bartventer/httpcache/store/memcache"
 	"github.com/google/go-github/v74/github"
-	"github.com/gregjones/httpcache"
 	"golang.org/x/build/maintner/maintpb"
 	"golang.org/x/oauth2"
 	"golang.org/x/sync/errgroup"
@@ -34,6 +37,13 @@
 // package for responses fulfilled from cache due to a 304 from the server.
 const xFromCache = "X-From-Cache"
 
+func init() {
+	// Register the in-memory cache driver for use with httpcache.
+	store.Register(memcache.Scheme, driver.DriverFunc(func(u *url.URL) (driver.Conn, error) {
+		return memcache.Open(), nil
+	}))
+}
+
 // GitHubRepoID is a GitHub org & repo, lowercase.
 type GitHubRepoID struct {
 	Owner, Repo string
@@ -287,9 +297,6 @@
 
 // LastModified reports the most recent time that any known metadata was updated.
 // In contrast to the Updated field, LastModified includes comments and events.
-//
-// TODO(bradfitz): this seems to not be working, at least events
-// aren't updating it. Investigate.
 func (gi *GitHubIssue) LastModified() time.Time {
 	ret := gi.Updated
 	if gi.commentsUpdatedTil.After(ret) {
@@ -1408,44 +1415,9 @@
 	}
 }
 
-// githubCache is an httpcache.Cache wrapper that only
-// stores responses for:
-//   - https://api.github.com/repos/$OWNER/$REPO/issues?direction=desc&page=1&sort=updated
-//   - https://api.github.com/repos/$OWNER/$REPO/milestones?page=1
-//   - https://api.github.com/repos/$OWNER/$REPO/labels?page=1
-type githubCache struct {
-	httpcache.Cache
-}
-
-var rxGithubCacheURLs = regexp.MustCompile(`^https://api.github.com/repos/\w+/\w+/(issues|milestones|labels)\?(.+)`)
-
-func cacheableURL(urlStr string) bool {
-	m := rxGithubCacheURLs.FindStringSubmatch(urlStr)
-	if m == nil {
-		return false
-	}
-	v, _ := url.ParseQuery(m[2])
-	if v.Get("page") != "1" {
-		return false
-	}
-	switch m[1] {
-	case "issues":
-		return v.Get("sort") == "updated" && v.Get("direction") == "desc"
-	case "milestones", "labels":
-		return true
-	default:
-		panic("unexpected cache key base " + m[1])
-	}
-}
-
-func (c *githubCache) Set(urlKey string, res []byte) {
-	// TODO: verify that the httpcache package guarantees that the
-	// first string parameter to Set here is actually a
-	// URL. Empirically they appear to be.
-	if cacheableURL(urlKey) {
-		c.Cache.Set(urlKey, res)
-	}
-}
+// Note: The previous githubCache wrapper filtered cacheable URLs to save memory.
+// The new httpcache library handles caching based on HTTP cache headers (RFC 9111),
+// which provides similar benefits with standards-compliant behavior.
 
 // sync checks for new changes on a single GitHub repository and
 // updates the Corpus with any changes. If loop is true, it runs
@@ -1460,11 +1432,10 @@
 	if gr.github.c.githubLimiter != nil {
 		directTransport = limitTransport{gr.github.c.githubLimiter, hc.Transport}
 	}
-	cachingTransport := &httpcache.Transport{
-		Transport:           directTransport,
-		Cache:               &githubCache{Cache: httpcache.NewMemoryCache()},
-		MarkCachedResponses: true, // adds "X-From-Cache: 1" response header.
-	}
+	cachingTransport := httpcache.NewTransport(
+		"memcache://",
+		httpcache.WithUpstream(directTransport),
+	)
 
 	p := &githubRepoPoller{
 		c:             gr.github.c,
diff --git a/maintner/github_test.go b/maintner/github_test.go
index b9d9b9c..0d34bf2 100644
--- a/maintner/github_test.go
+++ b/maintner/github_test.go
@@ -1089,3 +1089,62 @@
 		}
 	}
 }
+
+func TestLastModified(t *testing.T) {
+	tests := []struct {
+		name  string
+		issue *GitHubIssue
+		want  time.Time
+	}{
+		{
+			name: "Updated only",
+			issue: &GitHubIssue{
+				Updated: t3339("2018-01-01T00:00:00Z"),
+			},
+			want: t3339("2018-01-01T00:00:00Z"),
+		},
+		{
+			name: "Event is newer than Updated",
+			issue: &GitHubIssue{
+				Updated:      t3339("2018-01-01T00:00:00Z"),
+				eventMaxTime: t3339("2018-02-01T00:00:00Z"),
+			},
+			want: t3339("2018-02-01T00:00:00Z"),
+		},
+		{
+			name: "Comment is newer than Updated",
+			issue: &GitHubIssue{
+				Updated:            t3339("2018-01-01T00:00:00Z"),
+				commentsUpdatedTil: t3339("2018-03-01T00:00:00Z"),
+			},
+			want: t3339("2018-03-01T00:00:00Z"),
+		},
+		{
+			name: "Event is newest",
+			issue: &GitHubIssue{
+				Updated:            t3339("2018-01-01T00:00:00Z"),
+				commentsUpdatedTil: t3339("2018-02-01T00:00:00Z"),
+				eventMaxTime:       t3339("2018-04-01T00:00:00Z"),
+			},
+			want: t3339("2018-04-01T00:00:00Z"),
+		},
+		{
+			name: "Comment is newest",
+			issue: &GitHubIssue{
+				Updated:            t3339("2018-01-01T00:00:00Z"),
+				commentsUpdatedTil: t3339("2018-05-01T00:00:00Z"),
+				eventMaxTime:       t3339("2018-02-01T00:00:00Z"),
+			},
+			want: t3339("2018-05-01T00:00:00Z"),
+		},
+	}
+
+	for _, tt := range tests {
+		t.Run(tt.name, func(t *testing.T) {
+			got := tt.issue.LastModified()
+			if !got.Equal(tt.want) {
+				t.Errorf("LastModified() = %v; want %v", got, tt.want)
+			}
+		})
+	}
+}

Change information

Files:

M cmd/gerritbot/gerritbot.go
M go.mod
M go.sum
M maintner/cmd/maintserve/go.mod
M maintner/github.go
M maintner/github_test.go

Change size: M

Delta: 6 files changed, 87 insertions(+), 58 deletions(-)

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Gopher Robot (Gerrit)

unread,

Dec 2, 2025, 4:43:11 PM (11 hours ago) Dec 2

to Kevin Burke, goph...@pubsubhelper.golang.org, golang-co...@googlegroups.com

Message from Gopher Robot

Congratulations on opening your first change. Thank you for your contribution!

Next steps:
A maintainer will review your change and provide feedback. See
https://go.dev/doc/contribute#review for more info and tips to get your
patch through code review.

Most changes in the Go project go through a few rounds of revision. This can be
surprising to people new to the project. The careful, iterative review process
is our way of helping mentor contributors and ensuring that their contributions
have a lasting impact.

Open in Gerrit

Related details

Attention set is empty

Submit Requirements:

Code-Review
No-Unresolved-Comments
Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

satisfied_requirement

open

diffy

Dmitri Shuralyov (Gerrit)

unread,

Dec 2, 2025, 5:33:28 PM (10 hours ago) Dec 2

to Kevin Burke, goph...@pubsubhelper.golang.org, Dmitri Shuralyov, Carlos Amedee, Gopher Robot, golang-co...@googlegroups.com

Attention needed from Kevin Burke

Dmitri Shuralyov added 2 comments

Patchset-level comments

File-level comment, Patchset 1 (Latest):

Dmitri Shuralyov . resolved

Thanks for preparing this CL. I haven't looked closely at the new package yet, just leaving an initial comment in go.mod.

File go.mod

Line 3, Patchset 1 (Latest):go 1.25

Dmitri Shuralyov . unresolved

We need to keep x/build's go directive at 1.24 for a few months longer, while Go both 1.25 and 1.24 are still supported by the [Go release policy](https://go.dev/doc/devel/release#policy). This can become 1.25 after Go 1.26 is released (planned for Feb 2026), as that's when we'll stop supporting 1.24.

It seems the new httpcache package has 1.25 as its minimum Go language version. So to be considered for use here, either it needs to be updated to support 1.24 too, or we'll need to wait (or use an older version?).

Open in Gerrit

Related details

Attention is currently required from:

Kevin Burke

Submit Requirements:

Code-Review

No-Unresolved-Comments

Review-Enforcement
TryBots-Pass

Inspect html for hidden footers to help with email filtering. To unsubscribe visit settings.

Gerrit

unsatisfied_requirement

open

diffy

Reply all

Reply to author

Forward

[build] maintner, cmd/gerritbot: migrate to bartventer/httpcache

Kevin Burke (Gerrit)

Kevin Burke has uploaded the change for review

Commit message

Change diff

Change information

Related details

Gopher Robot (Gerrit)

Message from Gopher Robot

Related details

Dmitri Shuralyov (Gerrit)

Dmitri Shuralyov added 2 comments

Related details