[インデックス 10920] ファイルの概要

このコミットは、Goプロジェクトのダッシュボードアプリケーションにおける複数の改善を目的としています。具体的には、ログ出力の記述性を向上させ、ユーザーインターフェース（UI）に微調整を加え、認証エラーメッセージをより分かりやすく表示するように変更されています。これにより、ダッシュボードのデバッグのしやすさとユーザーエクスペリエンスが向上します。

コミット

commit 9f0e39b992bb714a8361790eee70412e64443ba6
Author: Andrew Gerrand <adg@golang.org>
Date:   Wed Dec 21 11:08:47 2011 +1100

    dashboard: more descriptive logging, ui tweaks, show better auth error
    
    R=golang-dev, dsymonds
    CC=golang-dev
    https://golang.org/cl/5505050

GitHub上でのコミットページへのリンク

https://github.com/golang/go/commit/9f0e39b992bb714a8361790eee70412e64443ba6

元コミット内容

dashboard: more descriptive logging, ui tweaks, show better auth error

R=golang-dev, dsymonds
CC=golang-dev
https://golang.org/cl/5505050

変更の背景

このコミットは、Goプロジェクトのビルドステータスなどを表示するダッシュボードアプリケーションの運用性とユーザビリティを向上させるために行われました。

ログの記述性向上: 既存のログ出力が不十分で、エラー発生時の原因特定やデバッグが困難であったと考えられます。エラーメッセージにコンテキストを追加することで、問題の追跡を容易にすることが目的です。
UIの微調整: ダッシュボードの表示が最適化されていなかったり、情報が整理されていなかったりした可能性があります。視覚的な調整により、ユーザーが情報をより効率的に把握できるように改善が図られました。
認証エラーの改善: 認証プロセスにおいて、エラーが発生した際にユーザーに提示されるメッセージが不明瞭であったため、ユーザーが問題を解決しにくい状況でした。より具体的なエラーメッセージを表示することで、認証失敗時のユーザー体験を改善することが求められました。

これらの変更は、ダッシュボードの保守性、デバッグの効率性、そしてエンドユーザーの利便性を高めることを目的としています。

前提知識の解説

このコミットを理解するためには、以下の技術的背景知識が役立ちます。

Google App Engine (GAE):
- このダッシュボードアプリケーションは、Google App Engine上で動作するように設計されています。GAEは、Googleが提供するPlatform as a Service (PaaS) であり、ウェブアプリケーションやモバイルバックエンドを構築・デプロイするためのフルマネージド環境を提供します。
- 特徴としては、スケーラビリティ、メンテナンスフリーなインフラ、そしてDatastoreのようなマネージドサービスとの統合が挙げられます。
- コード内でappengine.Contextやappengine/datastoreが使用されていることから、GAEのAPIを利用していることがわかります。
Go言語のエラーハンドリング:
- Go言語では、エラーは戻り値として明示的に扱われます。このコミットが作成された2011年当時、Goのエラーインターフェースはos.Errorという型で表現されていましたが、後に組み込みのerrorインターフェースに統合されました。
- fmt.Errorfは、フォーマットされた文字列から新しいエラーを生成するための関数です。このコミットでは、既存のエラーにコンテキスト情報を付加するために多用されており、これによりエラー発生時の原因特定が格段に容易になります。
- 例えば、単にreturn errとするのではなく、return fmt.Errorf("getting Commit: %v", err)とすることで、「コミットの取得中にエラーが発生した」という具体的な状況をログに残すことができます。
Google App Engine Datastore:
- Datastoreは、GAEが提供するNoSQLドキュメントデータベースサービスです。スケーラブルで可用性が高く、スキーマレスなデータモデルが特徴です。
- コード内では、datastore.Get（データの取得）、datastore.Put（データの保存）、datastore.RunInTransaction（トランザクション処理）などの関数が使用されており、アプリケーションの永続化層として機能しています。
HMAC (Keyed-Hash Message Authentication Code):
- HMACは、メッセージ認証コードの一種で、秘密鍵とハッシュ関数（この場合はMD5）を組み合わせてメッセージの完全性と認証性を保証するメカニズムです。
- 送信者と受信者が共有する秘密鍵を用いてメッセージのハッシュ値を計算し、そのハッシュ値（MAC）をメッセージと共に送信します。受信者は同じ秘密鍵とメッセージでMACを再計算し、受け取ったMACと比較することで、メッセージが改ざんされていないこと、および正当な送信者から送られたものであることを確認できます。
- このコミットでは、認証キーの検証にHMAC-MD5が導入されており、セキュリティの強化が図られています。
HTMLとCSS:
- ui.htmlファイルは、ダッシュボードのユーザーインターフェースを定義するHTMLテンプレートです。
- CSS（Cascading Style Sheets）は、HTML要素のスタイル（フォント、色、レイアウトなど）を定義するために使用されます。このコミットでは、CSSの変更によりUIの見た目が調整されています。
Goのjsonパッケージとhttpパッケージ:
- json.NewDecoderやjson.NewEncoderは、HTTPリクエスト/レスポンスボディのJSONデータのデコード/エンコードに使用されます。
- http.HandlerFuncやhttp.ResponseWriter、http.Requestは、Goの標準ライブラリであるnet/httpパッケージの一部であり、HTTPサーバーの構築とリクエスト処理に用いられます。

技術的詳細

このコミットは、主に以下の3つの側面で技術的な改善を行っています。

エラーハンドリングの改善とログの記述性向上:
- build.go内の多くの箇所で、単にreturn errとしていたエラー伝播が、return fmt.Errorf("メッセージ: %v", err)という形式に変更されています。
- これにより、エラーが発生した具体的な関数や処理のコンテキストがエラーメッセージに含まれるようになり、ログを解析する際に問題の発生源を特定しやすくなります。例えば、datastore.Getが失敗した場合、以前は単にDatastoreのエラーが返されるだけでしたが、変更後は「getting Commit: [Datastoreエラー]」のように、どのエンティティの取得中にエラーが発生したかが明確になります。
- appengine.ContextのErrorfやCriticalfメソッドも活用され、App Engineのログに詳細なエラー情報が出力されるようになっています。
認証メカニズムの強化とエラー表示の改善:
- AuthHandler関数において、POSTリクエストの認証キー検証ロジックが変更されました。
- 以前はsha1.New()を使用していましたが、hmac.NewMD5([]byte(secretKey))を使用するように変更されています。これは、共有秘密鍵を用いたHMAC認証を導入することで、よりセキュアなキー検証を実現しています。HMACは、単なるハッシュよりも、メッセージの認証と完全性保護において強力です。
- 認証失敗時のエラーメッセージも"invalid key"から"invalid key: " + keyへと変更され、どのキーが不正であったかを示すことで、デバッグの助けとなります。
UIの微調整とデータ表示の最適化:
- ui.htmlファイルでは、ダッシュボードの表示に関するいくつかの変更が行われています。
  - コミットハッシュのフォントサイズが9ptに設定され、視認性が向上しています。
  - ビルド時間の表示からfont-family: monospace;が削除され、一般的なフォントで表示されるようになりました。
  - 日付のフォーマットが"02 Jan 2006 15:04"から"Mon 02 Jan 15:04"へと簡略化され、よりコンパクトに表示されるようになりました。
  - <h2>Go</h2>という固定の見出しが削除され、より動的なコンテンツ表示に対応できるようになりました。
  - 「Other packages」のテーブル表示が{{if $.TipState}}という条件付きレンダリングブロックで囲まれました。これにより、TipStateが存在する場合にのみ表示されるようになり、UIの柔軟性が向上しています。
- build.goでは、commitsPerPage定数が20から30に増加しました。これにより、ダッシュボードの1ページに表示されるコミット数が多くなり、スクロールの手間が減ります。
- maxDatastoreStringLen = 500という新しい定数が導入され、コミットのDesc（説明）フィールドがDatastoreに保存される前に、この長さに切り詰められるようになりました。これは、Datastoreの文字列プロパティの最大長制限に対応するため、またはデータストアの効率を向上させるための措置と考えられます。

コアとなるコードの変更箇所

`misc/dashboard/app/build/build.go`

--- a/misc/dashboard/app/build/build.go
+++ b/misc/dashboard/app/build/build.go
@@ -9,6 +9,7 @@ import (
 	"appengine/datastore"
 	"bytes"
 	"compress/gzip"
+	"crypto/hmac"
 	"crypto/sha1"
 	"fmt"
 	"http"
@@ -18,7 +19,14 @@ import (
 	"strings"
 )
 
-const commitsPerPage = 20
+var defaultPackages = []*Package{
+	&Package{Name: "Go"},
+}
+
+const (
+	commitsPerPage        = 30
+	maxDatastoreStringLen = 500
+)
 
 // A Package describes a package that is listed on the dashboard.
 type Package struct {
@@ -111,11 +119,13 @@ func (c *Commit) Valid() os.Error {
 // It must be called from inside a datastore transaction.
 func (com *Commit) AddResult(c appengine.Context, r *Result) os.Error {
 	if err := datastore.Get(c, com.Key(c), com); err != nil {
-		return err
+		return fmt.Errorf("getting Commit: %v", err)
 	}
 	com.ResultData = append(com.ResultData, r.Data())
-	_, err := datastore.Put(c, com.Key(c), com)
-	return err
+	if _, err := datastore.Put(c, com.Key(c), com); err != nil {
+		return fmt.Errorf("putting Commit: %v", err)
+	}
+	return nil
 }
 
 // Result returns the build Result for this Commit for the given builder/goHash.
@@ -267,7 +277,7 @@ func commitHandler(r *http.Request) (interface{}, os.Error) {
 		com.PackagePath = r.FormValue("packagePath")
 		com.Hash = r.FormValue("hash")
 		if err := datastore.Get(c, com.Key(c), com); err != nil {
-			return nil, err
+			return nil, fmt.Errorf("getting Commit: %v", err)
 		}
 		return com, nil
 	}
@@ -278,10 +288,13 @@ func commitHandler(r *http.Request) (interface{}, os.Error) {
 	// POST request
 	defer r.Body.Close()
 	if err := json.NewDecoder(r.Body).Decode(com); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("decoding Body: %v", err)
+	}
+	if len(com.Desc) > maxDatastoreStringLen {
+		com.Desc = com.Desc[:maxDatastoreStringLen]
 	}
 	if err := com.Valid(); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("validating Commit: %v", err)
 	}
 	tx := func(c appengine.Context) os.Error {
 		return addCommit(c, com)
@@ -292,21 +305,24 @@ func addCommit(c appengine.Context, com *Commit) os.Error {
 // addCommit adds the Commit entity to the datastore and updates the tip Tag.
 // It must be run inside a datastore transaction.
 func addCommit(c appengine.Context, com *Commit) os.Error {
-	// if this commit is already in the datastore, do nothing
 	var tc Commit // temp value so we don't clobber com
 	err := datastore.Get(c, com.Key(c), &tc)
 	if err != datastore.ErrNoSuchEntity {
-		return err
+		// if this commit is already in the datastore, do nothing
+		if err == nil {
+			return nil
+		}
+		return fmt.Errorf("getting Commit: %v", err)
 	}
 	// get the next commit number
 	p, err := GetPackage(c, com.PackagePath)
 	if err != nil {
-		return err
+		return fmt.Errorf("GetPackage: %v", err)
 	}
 	com.Num = p.NextNum
 	p.NextNum++
 	if _, err := datastore.Put(c, p.Key(c), p); err != nil {
-		return err
+		return fmt.Errorf("putting Package: %v", err)
 	}
 	// if this isn't the first Commit test the parent commit exists
 	if com.Num > 0 {
@@ -315,7 +331,7 @@ func addCommit(c appengine.Context, com *Commit) os.Error {
 			Ancestor(p.Key(c)).
 			Count(c)
 		if err != nil {
-			return err
+			return fmt.Errorf("testing for parent Commit: %v", err)
 		}
 		if n == 0 {
 			return os.NewError("parent commit not found")
@@ -325,12 +341,14 @@ func addCommit(c appengine.Context, com *Commit) os.Error {
 	if p.Path == "" {
 		t := &Tag{Kind: "tip", Hash: com.Hash}
 		if _, err = datastore.Put(c, t.Key(c), t); err != nil {
-			return err
+			return fmt.Errorf("putting Tag: %v", err)
 		}
 	}
 	// put the Commit
-	_, err = datastore.Put(c, com.Key(c), com)
-	return err
+	if _, err = datastore.Put(c, com.Key(c), com); err != nil {
+		return fmt.Errorf("putting Commit: %v", err)
+	}
+	return nil
 }
 
 // tagHandler records a new tag. It reads a JSON-encoded Tag value from the
@@ -458,31 +476,34 @@ func resultHandler(r *http.Request) (interface{}, os.Error) {
 	res := new(Result)
 	defer r.Body.Close()
 	if err := json.NewDecoder(r.Body).Decode(res); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("decoding Body: %v", err)
 	}
 	if err := res.Valid(); err != nil {
-		return nil, err
+		return nil, fmt.Errorf("validating Result: %v", err)
 	}
 	// store the Log text if supplied
 	if len(res.Log) > 0 {
 		hash, err := PutLog(c, res.Log)
 		if err != nil {
-			return nil, err
+			return nil, fmt.Errorf("putting Log: %v", err)
 		}
 		res.LogHash = hash
 	}
 	tx := func(c appengine.Context) os.Error {
 		// check Package exists
 		if _, err := GetPackage(c, res.PackagePath); err != nil {
-			return err
+			return fmt.Errorf("GetPackage: %v", err)
 		}
 		// put Result
 		if _, err := datastore.Put(c, res.Key(c), res); err != nil {
-			return err
+			return fmt.Errorf("putting Result: %v", err)
 		}
 		// add Result to Commit
 		com := &Commit{PackagePath: res.PackagePath, Hash: res.Hash}
-		return com.AddResult(c, res)
+		if err := com.AddResult(c, res); err != nil {
+			return fmt.Errorf("AddResult: %v", err)
+		}
+		return nil
 	}
 	return nil, datastore.RunInTransaction(c, tx, nil)
 }
@@ -527,47 +548,54 @@ func (e errBadMethod) String() string {
 // supplied key and builder query parameters.
 func AuthHandler(h dashHandler) http.HandlerFunc {
 	return func(w http.ResponseWriter, r *http.Request) {
+		c := appengine.NewContext(r)
+
 		// Put the URL Query values into r.Form to avoid parsing the
 		// request body when calling r.FormValue.
 		r.Form = r.URL.Query()
 
+		var err os.Error
+		var resp interface{}
+
 		// Validate key query parameter for POST requests only.
 		key := r.FormValue("key")
-		if r.Method == "POST" && key != secretKey &&
-			!appengine.IsDevAppServer() {
-			h := sha1.New()
-			h.Write([]byte(r.FormValue("builder") + secretKey))
+		if r.Method == "POST" && key != secretKey && !appengine.IsDevAppServer() {
+			h := hmac.NewMD5([]byte(secretKey))
+			h.Write([]byte(r.FormValue("builder")))
 			if key != fmt.Sprintf("%x", h.Sum()) {
-				logErr(w, r, os.NewError("invalid key"))
-				return
+				err = os.NewError("invalid key: " + key)
 			}
 		}
 
 		// Call the original HandlerFunc and return the response.
-		c := appengine.NewContext(r)
-		resp, err := h(r)
-		dashResp := dashResponse{Response: resp}
+		if err == nil {
+			resp, err = h(r)
+		}
+
+		// Write JSON response.
+		dashResp := &dashResponse{Response: resp}
 		if err != nil {
 			c.Errorf("%v", err)
 			dashResp.Error = err.String()
 		}
 		w.Header().Set("Content-Type", "application/json")
 		if err = json.NewEncoder(w).Encode(dashResp); err != nil {
-			c.Criticalf("%v", err)
+			c.Criticalf("encoding response: %v", err)
 		}
 	}
 }
 
 func initHandler(w http.ResponseWriter, r *http.Request) {
 	// TODO(adg): devise a better way of bootstrapping new packages
-	var pkgs = []*Package{
-		&Package{Name: "Go"},
-		&Package{Name: "Test", Path: "code.google.com/p/go.test"},
-	}
 	c := appengine.NewContext(r)
-	for _, p := range pkgs {
-		_, err := datastore.Put(c, p.Key(c), p)
-		if err != nil {
+	for _, p := range defaultPackages {
+		if err := datastore.Get(c, p.Key(c), new(Package)); err == nil {
+			continue
+		} else if err != datastore.ErrNoSuchEntity {
+			logErr(w, r, err)
+			return
+		}
+		if _, err := datastore.Put(c, p.Key(c), p); err != nil {
 			logErr(w, r, err)
 			return
 		}

`misc/dashboard/app/build/ui.html`

--- a/misc/dashboard/app/build/ui.html
+++ b/misc/dashboard/app/build/ui.html
@@ -31,13 +31,13 @@
       }
       .build .hash {
         font-family: monospace;
+\tfont-size: 9pt;
       }
       .build .result {
         text-align: center;
         width: 50px;
       }
       .build .time {
-        font-family: monospace;
         color: #666;
       }
       .build .descr, .build .time, .build .user {
@@ -63,8 +63,6 @@
 
     <h1>Go Build Status</h1>
 
-    <h2>Go</h2>
-
   {{if $.Commits}}
 
     <table class="build">
@@ -91,7 +89,7 @@
       </td>
       {{end}}
       <td class="user">{{shortUser .User}}</td>
-      <td class="time">{{.Time.Time.Format "02 Jan 2006 15:04"}}</td>
+      <td class="time">{{.Time.Time.Format "Mon 02 Jan 15:04"}}</td>
       <td class="desc">{{shortDesc .Desc}}</td>
       </tr>
     {{end}}\
@@ -109,6 +107,7 @@
     <p>No commits to display. Hm.</p>
   {{end}}\
 
+  {{if $.TipState}}\
     <h2>Other packages</h2>
 
     <table class="packages">\
@@ -144,6 +143,7 @@
     </tr>
   {{end}}\
     </table>
+  {{end}}\
 
   </body>
 </html>

コアとなるコードの解説

`misc/dashboard/app/build/build.go`

エラーラッピングの導入:
- AddResult, commitHandler, addCommit, resultHandlerなどの関数内で、Datastore操作やJSONデコード、バリデーションなどでエラーが発生した場合に、単にエラーを返すのではなく、fmt.Errorf("メッセージ: %v", err)という形式でエラーをラップするようになりました。
- これにより、エラーが発生したコードの場所や、そのエラーがどのような操作中に発生したのかというコンテキスト情報がエラーメッセージに追加され、デバッグ時のトレースが非常に容易になります。これはGo言語におけるエラーハンドリングのベストプラクティスの一つです。
定数の変更と追加:
- commitsPerPageが20から30に増えました。これにより、ダッシュボードのページあたりのコミット表示数が増加し、ユーザーはより多くの情報を一度に確認できるようになります。
- maxDatastoreStringLen = 500という新しい定数が追加されました。これは、Datastoreの文字列プロパティの最大長を考慮したもので、commitHandler内でコミットの説明（com.Desc）がこの長さを超える場合に切り詰めるために使用されます。これにより、Datastoreへの書き込みエラーを防ぎ、データの整合性を保ちます。
認証ロジックの強化:
- AuthHandler関数において、認証キーの検証にcrypto/hmacパッケージが導入されました。以前はsha1.New()を直接使用していましたが、hmac.NewMD5([]byte(secretKey))を使用することで、秘密鍵を用いたHMAC認証が実現されています。
- HMACは、メッセージの完全性と認証性を保証するため、単なるハッシュよりもセキュリティが向上します。不正なキーが提供された場合のエラーメッセージも"invalid key: " + keyと、より詳細になりました。
initHandlerの改善:
- initHandlerは、初期パッケージをDatastoreに登録する役割を担っています。変更前は、パッケージが既に存在するかどうかを確認せずにdatastore.Putを呼び出していました。
- 変更後は、datastore.Getでパッケージの存在を確認し、既に存在する場合はスキップするようになりました。これにより、アプリケーションの再起動時などに不必要なDatastoreへの書き込みやエラー発生を防ぎ、初期化処理がより堅牢になりました。

`misc/dashboard/app/build/ui.html`

CSSの調整:
- .build .hashクラスにfont-size: 9pt;が追加され、コミットハッシュの表示が小さく、よりコンパクトになりました。
- .build .timeクラスからfont-family: monospace;が削除され、ビルド時間の表示が一般的なフォントに戻されました。
日付フォーマットの変更:
- コミット時間の表示フォーマットが"02 Jan 2006 15:04"（例: 21 Dec 2011 11:08）から"Mon 02 Jan 15:04"（例: Wed 21 Dec 11:08）へと変更されました。これにより、曜日が追加され、年が省略されることで、より簡潔で読みやすい表示になりました。
見出しの削除:
- 固定の見出し<h2>Go</h2>がHTMLから削除されました。これにより、ダッシュボードのコンテンツがより動的に生成されることを可能にし、柔軟なUIレイアウトに対応できるようになります。
条件付き表示の導入:
- 「Other packages」のテーブル全体が{{if $.TipState}}...{{end}}というGoテンプレートの条件付きブロックで囲まれました。これは、TipStateというデータが存在する場合にのみ、このセクションが表示されることを意味します。これにより、関連情報がない場合にはUIがすっきりと表示され、ユーザーエクスペリエンスが向上します。

参考にした情報源リンク

Google App Engine Documentation (Datastore, Go Standard Environment): (当時の公式ドキュメントを参照)
Go言語のエラーハンドリングに関する公式ドキュメントやブログ記事
HMAC (Keyed-Hash Message Authentication Code) の概念に関する情報源
Go言語のnet/httpおよびencoding/jsonパッケージのドキュメント
Go言語のテンプレートパッケージに関するドキュメント

comemo

[インデックス 10920] ファイルの概要

コミット

GitHub上でのコミットページへのリンク

元コミット内容

変更の背景

前提知識の解説

技術的詳細

コアとなるコードの変更箇所

`misc/dashboard/app/build/build.go`

`misc/dashboard/app/build/ui.html`

コアとなるコードの解説

`misc/dashboard/app/build/build.go`

`misc/dashboard/app/build/ui.html`

関連リンク

参考にした情報源リンク

Keyboard shortcuts

comemo