[インデックス 13302] ファイルの概要
このコミットは、Go言語のダッシュボードアプリケーションにおけるビルド失敗通知の不具合を修正するものです。具体的には、Google App EngineのDatastoreクエリの挙動変更(クエリメソッドが元のクエリを直接変更するのではなく、新しいクエリオブジェクトを返すようになった点)に対応し、ビルド失敗通知が正しく機能するように修正しています。
コミット
commit 55282111585087d35c5af57dc5bdcf9a886debe7
Author: Andrew Gerrand <adg@golang.org>
Date: Thu Jun 7 09:27:39 2012 +1000
misc/dashboard/app: fix build failure notifications
The datastore.Query methods once mutated the Query value, but now they return
a derivative query, so the Hash= and ParentHash= filters were not being
applied.
R=golang-dev, bradfitz, dsymonds
CC=golang-dev
https://golang.org/cl/6300058
GitHub上でのコミットページへのリンク
https://github.com/golang/go/commit/55282111585087d35c5af57dc5bdcf9a886debe7
元コミット内容
このコミットは、Go言語の公式リポジトリ内のmisc/dashboard/app
ディレクトリにあるビルドダッシュボードアプリケーションに関するものです。コミットメッセージによると、datastore.Query
のメソッドが以前はQuery
値を直接変更(ミューテート)していたのに対し、変更後は派生した新しいクエリを返すようになったため、Hash=
およびParentHash=
フィルターが正しく適用されていなかったという問題が指摘されています。これにより、ビルド失敗通知が適切に機能していなかったと考えられます。
変更の背景
この変更の背景には、Go言語のGoogle App Engine向けDatastoreクライアントライブラリにおけるdatastore.Query
型のメソッドの挙動変更があります。初期のGoのライブラリ設計では、メソッドがレシーバ(この場合はdatastore.Query
のインスタンス)を直接変更して状態を更新するパターンが一般的でした。しかし、より関数型プログラミングに近い、あるいはイミュータブルなデータ構造を推奨する設計思想への移行、または特定のAPI設計の改善の一環として、Query
オブジェクトを操作するメソッド(例: Filter
, Order
, Limit
など)が、元のQuery
オブジェクトを変更するのではなく、新しいQuery
オブジェクトを生成して返すように変更されました。
この変更により、既存のコードベースでq.Filter(...)
のようにメソッドを呼び出しても、その戻り値を新しいクエリ変数に代入しない限り、フィルター条件が適用されないというバグが発生しました。Goのビルドダッシュボードは、ビルドのコミット情報やその状態をDatastoreに保存しており、特定のコミット(ハッシュや親ハッシュ)に基づいてクエリを実行していました。このDatastoreクエリの挙動変更により、期待通りにフィルターが適用されず、結果としてビルドの成功/失敗通知ロジックが誤動作するようになったため、この修正が必要となりました。
前提知識の解説
Google App Engine Datastore
Google App Engine Datastoreは、Google Cloud Platformが提供するNoSQLドキュメントデータベースサービスです。スケーラブルなWebアプリケーションのバックエンドとして設計されており、大量のデータを効率的に保存・取得できます。主な特徴は以下の通りです。
- NoSQL: リレーショナルデータベースのような厳密なスキーマを持たず、柔軟なデータモデルをサポートします。
- エンティティとプロパティ: データは「エンティティ」として保存され、各エンティティはキーによって一意に識別されます。エンティティは、キーと値のペアである「プロパティ」の集合を持ちます。
- 種類(Kind): エンティティは「種類」(Kind)によって分類されます。これはリレーショナルデータベースのテーブル名に似ています。
- クエリ: Datastoreからデータを取得するためにクエリを使用します。クエリは、種類、フィルター条件、ソート順などを指定して構築されます。
- トランザクション: 複数の操作をアトミックに実行するためのトランザクションをサポートします。
Go言語におけるdatastore.Query
Go言語でGoogle App Engine Datastoreを操作する際、google.golang.org/appengine/datastore
パッケージ(またはその前身のappengine/datastore
パッケージ)が使用されます。datastore.Query
型は、Datastoreに対するクエリを構築するための構造体です。
クエリを構築する際には、通常、以下のようなメソッドチェーンを使用します。
q := datastore.NewQuery("KindName").
Filter("PropertyName =", "Value").
Order("-AnotherProperty").
Limit(10)
このコミットの修正が関連するのは、Filter
などのメソッドがどのようにq
(datastore.Query
のインスタンス)を変更するかという点です。
イミュータブル(Immutable)とミュータブル(Mutable)なオブジェクト
- ミュータブル(Mutable)なオブジェクト: オブジェクトが作成された後も、その状態(内部のデータ)を変更できるオブジェクトです。多くのプログラミング言語で、リストや配列、マップなどのデータ構造はデフォルトでミュータブルです。
- 例:
list.append(item)
のように、append
メソッドがlist
オブジェクト自体を変更する場合。
- 例:
- イミュータブル(Immutable)なオブジェクト: オブジェクトが作成された後、その状態を一切変更できないオブジェクトです。変更が必要な場合は、元のオブジェクトを基に新しいオブジェクトを作成します。
- 例:
new_string = old_string.replace("a", "b")
のように、replace
メソッドがold_string
を変更せず、新しい文字列を返す場合。
- 例:
イミュータブルなオブジェクトは、並行処理における競合状態の回避、コードの予測可能性の向上、デバッグの容易さなどの利点があります。Go言語では、スライスやマップはミュータブルですが、文字列はイミュータブルです。また、構造体のメソッドがレシーバをポインタで受け取るか値で受け取るかによって、そのメソッドがレシーバをミューテートできるかどうかが変わります。
このコミットの文脈では、datastore.Query
のメソッドが、以前はミュータブルな操作(レシーバを直接変更)だったものが、イミュータブルな操作(新しいクエリオブジェクトを返す)に変更されたことが問題の根源です。
技術的詳細
このコミットの核心は、Go言語のdatastore.Query
型におけるメソッドのセマンティクス変更です。
以前のdatastore.Query
のメソッド(例: Filter
, Order
, Limit
など)は、レシーバであるdatastore.Query
のインスタンスを直接変更していました。これは、メソッドがポインタレシーバ(例: func (q *Query) Filter(...)
)を持つか、あるいは値レシーバであっても内部的にポインタを操作するような実装であったことを示唆しています。この場合、以下のようなコードは期待通りに動作しました。
// 以前の挙動(ミュータブル)
q := datastore.NewQuery("Commit")
q.Filter("ParentHash=", com.Hash) // qが直接変更される
// ... qを使ってクエリを実行
しかし、Goのライブラリ開発の過程で、datastore.Query
のメソッドがイミュータブルな操作を行うように変更されました。つまり、これらのメソッドは元のdatastore.Query
オブジェクトを変更せず、代わりに新しいdatastore.Query
オブジェクトを生成して返すようになりました。これは、メソッドが値レシーバを持ち、そのメソッド内で新しいQuery
構造体を構築して返すような実装になったことを意味します(例: func (q Query) Filter(...) Query
)。
この変更により、以前のコードのようにメソッドの戻り値を代入しないまま呼び出すと、メソッドが返した新しいクエリオブジェクトが破棄され、元のq
オブジェクトにはフィルター条件が適用されないという問題が発生しました。
// 変更後の挙動(イミュータブル)
q := datastore.NewQuery("Commit")
q.Filter("ParentHash=", com.Hash) // 戻り値が破棄されるため、qは変更されない
// ... qを使ってクエリを実行すると、フィルターが適用されていないクエリが実行される
このコミットは、この変更に対応するため、q.Filter(...)
の呼び出し結果を再度q
変数に代入するように修正しています。
// 修正後の正しい挙動
q := datastore.NewQuery("Commit")
q = q.Filter("ParentHash=", com.Hash) // 戻り値をqに代入することで、フィルターが適用された新しいクエリがqに設定される
// ... qを使ってクエリを実行
この修正は、Go言語におけるイミュータブルなAPI設計パターンへの移行と、それに伴う既存コードの適応の必要性を示しています。このような変更は、APIの利用者がメソッドの戻り値を適切に扱うことの重要性を強調します。
コアとなるコードの変更箇所
変更はmisc/dashboard/app/build/notify.go
ファイルに集中しています。
diff --git a/misc/dashboard/app/build/notify.go b/misc/dashboard/app/build/notify.go
index f4c6733598..afcc7b2db5 100644
--- a/misc/dashboard/app/build/notify.go
+++ b/misc/dashboard/app/build/notify.go
@@ -45,7 +45,7 @@ func notifyOnFailure(c appengine.Context, com *Commit, builder string) error {
if cr.OK {
// This commit is OK. Notify if next Commit is broken.
next := new(Commit)
- q.Filter("ParentHash=", com.Hash)
+ q = q.Filter("ParentHash=", com.Hash)
if err := firstMatch(c, q, next); err != nil {
if err == datastore.ErrNoSuchEntity {
// OK at tip, no notification necessary.
@@ -61,7 +61,7 @@ func notifyOnFailure(c appengine.Context, com *Commit, builder string) error {
} else {
// This commit is broken. Notify if the previous Commit is OK.
prev := new(Commit)
- q.Filter("Hash=", com.ParentHash)
+ q = q.Filter("Hash=", com.ParentHash)
if err := firstMatch(c, q, prev); err != nil {
if err == datastore.ErrNoSuchEntity {
// No previous result, let the backfill of
具体的には、以下の2箇所でq.Filter(...)
の呼び出しがq = q.Filter(...)
に変更されています。
q.Filter("ParentHash=", com.Hash)
がq = q.Filter("ParentHash=", com.Hash)
に変更。q.Filter("Hash=", com.ParentHash)
がq = q.Filter("Hash=", com.ParentHash)
に変更。
コアとなるコードの解説
notifyOnFailure
関数は、ビルドの失敗通知ロジックを処理していると考えられます。この関数内で、datastore.Query
オブジェクトq
が初期化され、その後にFilter
メソッドを使って条件が追加されています。
変更前のコードでは、q.Filter("ParentHash=", com.Hash)
のように呼び出されていました。これは、Filter
メソッドがq
オブジェクト自体を直接変更することを期待していました。しかし、datastore.Query
のAPIが変更され、Filter
メソッドが新しいQuery
オブジェクトを返すようになったため、この呼び出しでは戻り値が破棄され、q
オブジェクトにはフィルター条件が適用されませんでした。
変更後のコードでは、q = q.Filter("ParentHash=", com.Hash)
のように、Filter
メソッドの戻り値を再度q
変数に代入しています。これにより、Filter
メソッドが返した、フィルター条件が適用された新しいQuery
オブジェクトが正しくq
に設定されるようになります。その結果、firstMatch(c, q, next)
やfirstMatch(c, q, prev)
といったDatastoreクエリの実行が、意図したフィルター条件に基づいて行われるようになり、ビルド失敗通知のロジックが正常に機能するようになります。
この修正は、Go言語のAPI設計におけるイミュータブルなパターンへの移行と、それに伴うコードの適応の典型的な例です。
関連リンク
- GitHub上のコミットページ: https://github.com/golang/go/commit/55282111585087d35c5af57dc5bdcf9a886debe7
- Go CL (Change List) 6300058: https://golang.org/cl/6300058
参考にした情報源リンク
- Google App Engine Datastore ドキュメント (Go):
- https://cloud.google.com/appengine/docs/standard/go/datastore/query-basics (現在のドキュメント。当時のAPIとは異なる可能性がありますが、概念は共通)
- Go言語におけるイミュータブルなデータ構造と関数型プログラミングの概念に関する一般的な情報源。
- Go言語の
datastore
パッケージの変更履歴や関連する議論(当時のGoのメーリングリストやIssueトラッカーなど)。- 当時の正確な変更点を特定するには、Goのソースコードリポジトリの履歴を深く掘り下げる必要がありますが、このコミットメッセージ自体が変更の性質を明確に示しています。