`above_http_content_limit` の重複コードを1つのメソッドに集約
#3582 で意図せず発生した重複コードを整理し、上限超過時の副作用処理を raise_above_http_content_limit に一本化しました。
背景
直前にマージされた #3582 にて、above_http_content_limit メソッドの呼び出し箇所に重複コードが発生していました。具体的には、setup_body と write_chunk の2箇所それぞれで、同一の後処理(バッファのクリア、ステータスコードの設定、例外の送出)が繰り返されていました。
#3889 はこの重複に気づき、即座に修正を加えたものです。
技術的な変更
setup_body と write_chunk の2箇所に存在した重複コードを、raise_above_http_content_limit という新しいメソッドに集約しました。
変更前 は、above_http_content_limit で上限チェックを行い、呼び出し側がそれぞれ同じ後処理を実装していました:
# setup_body 内
if above_http_content_limit(content_length)
@buffer = nil
@body = EmptyBody
@error_status_code = 413
@env[HTTP_CONNECTION] = 'close'
raise HttpParserError, "Payload Too Large"
end
# write_chunk 内
if above_http_content_limit(@chunked_content_length + str.bytesize)
@buffer = nil
@body = EmptyBody
@error_status_code = 413
@env[HTTP_CONNECTION] = 'close'
raise HttpParserError, "Payload Too Large"
end
変更後 は、条件チェックと後処理を分離し、後処理のみを新メソッドにまとめています:
# setup_body 内
raise_above_http_content_limit if @http_content_length_limit&.< content_length
# write_chunk 内
if @http_content_length_limit&.< @chunked_content_length + str.bytesize
raise_above_http_content_limit
end
# 新メソッド
def raise_above_http_content_limit
@http_content_length_limit_exceeded = true
@buffer = nil
@body = EmptyBody
@error_status_code = 413
@env[HTTP_CONNECTION] = 'close'
raise HttpParserError, "Payload Too Large"
end
旧 above_http_content_limit メソッドが担っていた @http_content_length_limit_exceeded への代入も、新メソッド内で true の直接代入に変わっています。呼び出し元では Safe Navigation Operator(&.) を使った @http_content_length_limit&.< value という条件式が残り、@http_content_length_limit が nil(制限なし)の場合を簡潔に扱えます。
設計判断
この変更により、条件チェックのロジックと、上限超過時の例外発生・後処理を行うロジックが分離されました。
旧設計では above_http_content_limit が @http_content_length_limit_exceeded への代入という副作用を持ちつつ、戻り値で条件分岐に使われるという二重の役割を担っていました。新設計では条件式を呼び出し元に残しつつ、「上限超過時に行う処理全体」を raise_above_http_content_limit として1箇所に集めています。メソッド名に raise_ プレフィックスを付けることで、呼び出すと必ず例外が送出されることをシグネチャから読み取れます。
まとめ
5行×2箇所の重複を1つのメソッドに集約したシンプルなリファクタリングですが、今後「上限超過時の挙動を変更したい」場合の修正箇所が1箇所に絞られるという保守性の向上があります。コードの構造を客観的に見ると、条件チェックと副作用処理の責務がより明確に分かれた形になっています。