Admin informations /v1/admin

[koi-chan]

/v1/version に、サーバ管理者の情報を出力できるようにしました。 提供者名とそのサーバの利用規約・連絡先を書いたURLを返すことを意図しています。

ご確認いただきますようお願い致します。

[ysakasin]

@koi-chan PRありがとうございます。

正直、この機能のサポートは否定的です。管理者が"適切に"設定すべき内容なので、結局設定されずに困るなんてことが起こりそうだからです。BCDice-APIのサーバーを立てるのは、技術に明るい人だけではないので。

以下、いくつか気になった点です。

エンドポイント

/v1/version に含めるよりも、 /v1/admin を新設してした方が適切なようにも思えますが、 しいて/v1/version に含める意図があれば教えてください。

設定方法

ファイルへの記述だけでなく、環境変数からも設定できるようにして欲しいです。 Herokuへのデプロイ時に面倒なことになってしまいます。 YAMLと環境変数が両方ある場合には環境変数を優先して欲しいです。

[ysakasin]

あと、この情報の利用用途があまり把握できてないので、利用用途について補足あればお教えください。

[ochaochaocha3]

PRの方針に賛成します。

一般に、Webサービスを運営する際は、運営者情報と利用規約をよく提示します。BCDice-APIは複数箇所に設置されるため、運営者情報と利用規約も設置者によって変わります。そのため、これらの情報を実際に動作しているサーバプログラムから何らかの手段で参照できるようにすることは、トラブル発生時に対応しやすくするという意味で、有意義です。参考情報として、RESTful APIの仕様を記述する形式であるOpen APIにも、コンタクト情報用や利用規約用のフィールドがあります。

管理者が"適切に"設定すべき内容なので、結局設定されずに困るなんてことが起こりそう

コードを見てみると、設定しなかった場合は nil になるので、運営者情報と利用規約はともにoptionalな項目（大手のサーバは設定する、身内で使うだけならば設定の必要なし）とすれば、実運用上は問題ないと思いました。

エンドポイント、および設定方法については、@ysakasin さんと同意見です。

[ochaochaocha3]

設定読み込み処理は一応動きますが、クラス定義に読み込み処理があると、再読み込みやテストが行いにくいため、設定用のクラスを作るとより良いと思います。これを構造体で作ると、#to_h での変換も自動で用意してくれるので、便利です。

（以下、動作確認済みの変更案です）

# lib/bcdice_api/administrator.rb
require 'yaml'

module BCDiceAPI
  class << self
    # 運営者情報
    # @return [Administrator]
    attr_accessor :admin
  end

  # 運営者情報を表す構造体
  # @!attribute name
  #   @return [String] 運営者名
  # @!attribute address
  #   @return [String] 連絡先のURLやメールアドレス
  Administrator = Struct.new(:name, :address) do
    # YAMLファイルと環境変数から読み込む
    # @param [String] yaml_file YAMLファイルのパス
    # @return [Administrator]
    def self.load_from_yaml_and_env(yaml_file)
      admin = new

      if File.exist?(yaml_file)
        admin_hash = YAML.load_file(yaml_file)
        admin.update_with_hash_from_yaml!(admin_hash)
      end

      admin.update_with_env!

      admin
    end

    # YAMLから変換したHashを使用して情報を更新する
    # @return [self]
    def update_with_hash_from_yaml!(hash)
      self.name = hash['admin_name']
      self.address = hash['admin_address']

      self
    end

    # 環境変数を使用して情報を更新する
    # @return [self]
    def update_with_env!
      name = ENV['BCDICE_API_ADMIN_NAME']
      self.name = name if name

      address = ENV['BCDICE_API_ADMIN_ADDRESS']
      self.address = address if address

      self
    end
  end
end

# server.rb
require 'bcdice_api/administrator'

# ...

# ファイル末尾：設定を運営者情報を読み込む
admin_yaml = File.expand_path('config/admin.yaml', __dir__)
BCDiceAPI.admin = BCDiceAPI::Administrator.load_from_yaml_and_env(admin_yaml)

# GETのどこかで BCDiceAPI.admin.to_h を使う

[koi-chan]

早速のご確認を頂きありがとうございます。 エンドポイントの変更、環境変数による設定、設定用のクラスの作成について、改良したものを準備致します。

この機能の利用用途としては、基本的に他のツールのバックエンドとして使われるため利用者からは見えにくい API サービスを広く公開して提供するとき、API 提供者の身を守る道具として機能することを意図しています。 利用者自身や API を使う他のツール作成者・提供者などが API 提供者の利益を損なうような利用をしたとき、API 提供者が身を守る手段の一つとして(読まれるかどうかは別として)利用規約や使い方など、利用者が使うに当たって注意すべき項目を掲げる行動が挙げられます。 これらの利用規約等にアクセスしやすくすることは、API プログラムとして重要ではないかと考えます。

確かに懸念されているとおり、管理者が適切に設定しなければ無用の機能ではあります。 しかし、適切な設定がされているかどうかは、ある API が公開サービスかそうでないかをツール作成者・提供者が判別しやすくしたり、利用者が(安定性や管理されているかなどを考慮して)どの API サーバを利用するかを検討したりするための材料になり得ると思います。

[ysakasin]

Structを使わずとも、ロード処理を特異メソッドに切り出せばいいと思います。

[ysakasin]

module BCDiceAPI
  def self.load_something
    ...
  end

  ADMIN = load_something
end

[ysakasin]

Discord等での議論を通じて、この機能がある方が便利であると私も理解できたので、提案を受け入れます。

[koi-chan]

再び確認頂きありがとうございます。 修正したものを push 致します。