Keyboard shortcuts

Press or to navigate between chapters

Press S or / to search in the book

Press ? to show this help

Press Esc to hide this help

[インデックス 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などのメソッドがどのようにqdatastore.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(...)に変更されています。

  1. q.Filter("ParentHash=", com.Hash)q = q.Filter("ParentHash=", com.Hash) に変更。
  2. 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設計におけるイミュータブルなパターンへの移行と、それに伴うコードの適応の典型的な例です。

関連リンク

参考にした情報源リンク

  • Google App Engine Datastore ドキュメント (Go):
  • Go言語におけるイミュータブルなデータ構造と関数型プログラミングの概念に関する一般的な情報源。
  • Go言語のdatastoreパッケージの変更履歴や関連する議論(当時のGoのメーリングリストやIssueトラッカーなど)。
    • 当時の正確な変更点を特定するには、Goのソースコードリポジトリの履歴を深く掘り下げる必要がありますが、このコミットメッセージ自体が変更の性質を明確に示しています。