Add app_url to the config so workers can create OpenAI image URLs (#532)

krschacht · jlvallelonga · web-flow · commit fc7a48287067 · 2024-11-24T09:17:08.000-06:00
Co-authored-by: Justin Vallelonga &lt;jlvallelonga@gmail.com&gt;
diff --git a/.github/workflows/rubyonrails.yml b/.github/workflows/rubyonrails.yml
@@ -28,6 +28,9 @@ jobs:
     env:
       RAILS_ENV: test
       DATABASE_URL: "postgres://rails:password@localhost:5432/hostedgpt_test"
+      APP_URL_PROTOCOL: "http"
+      APP_URL_HOST: "localhost"
+      APP_URL_PORT: "3000"
     steps:
       - name: Checkout code
         uses: actions/checkout@v4
@@ -68,6 +71,9 @@ jobs:
     env:
       RAILS_ENV: test
       DATABASE_URL: "postgres://rails:password@localhost:5432/hostedgpt_test"
+      APP_URL_PROTOCOL: "http"
+      APP_URL_HOST: "localhost"
+      APP_URL_PORT: "3000"
       DISPLAY: "=:99"
       CHROME_VERSION: "127.0.6533.119"
 
diff --git a/Gemfile b/Gemfile
@@ -41,13 +41,14 @@ gem "ffi", "~> 1.15.5" # explicitly requiring 15.5 until this is resolved: https
 gem "amatch", "~> 0.4.1" # enables fuzzy comparison of strings, a tool uses this
 gem "rails_heroicon", "~> 2.2.0"
 gem "ruby-openai", "~> 7.0.1"
-gem "anthropic", "~> 0.1.0"
+gem "anthropic", "~> 0.1.0" # TODO update to the latest version
 gem "tiktoken_ruby", "~> 0.0.9"
 gem "solid_queue", "~> 1.0.0"
 gem "name_of_person"
 gem "actioncable-enhanced-postgresql-adapter" # longer paylaods w/ postgresql actioncable
 gem "aws-sdk-s3", require: false
 gem "postmark-rails"
+gem "ostruct"
 
 gem "omniauth", "~> 2.1"
 gem "omniauth-google-oauth2", "~> 1.1"
diff --git a/app/models/document.rb b/app/models/document.rb
@@ -19,18 +19,26 @@ class Document < ApplicationRecord
   validates :purpose, :filename, :bytes, presence: true
   validate :file_present
 
-  def file_data_url(variant = :large)
-    return nil if !file.attached?
+  def has_image?(variant = nil)
+    if variant.present?
+      return has_file_variant_processed?(variant)
+    end
 
-    "data:#{file.blob.content_type};base64,#{file_base64(variant)}"
+    file.attached?
   end
 
-  def file_base64(variant = :large)
-    return nil if !file.attached?
-    wait_for_file_variant_to_process!(variant.to_sym)
-
-    file_contents = file.variant(variant.to_sym).processed.download
-    base64 = Base64.strict_encode64(file_contents)
+  def image_url(variant, fallback: nil)
+    return nil unless has_image?
+
+    if Rails.application.config.x.app_url.blank?
+      file_data_url(variant)
+    elsif has_file_variant_processed?(variant)
+      fully_processed_url(variant)
+    elsif fallback.nil?
+      redirect_to_processed_path(variant)
+    else
+      fallback
+    end
   end
 
   def has_file_variant_processed?(variant)
@@ -57,6 +65,15 @@ def redirect_to_processed_path(variant)
 
   private
 
+  def file_data_url(variant = :large)
+    wait_for_file_variant_to_process!(variant)
+
+    file_contents = file.variant(variant.to_sym).processed.download
+    base64_data = Base64.strict_encode64(file_contents)
+
+    "data:#{file.blob.content_type};base64,#{base64_data}"
+  end
+
   def set_default_user
     self.user ||= message.conversation.user
   end
diff --git a/app/models/message.rb b/app/models/message.rb
@@ -43,9 +43,7 @@ def finished?
       (content_text.present? || content_tool_calls.present?)
   end
 
-  def not_finished?
-    !finished?
-  end
+  def not_finished? = !finished?
 
   private
 
diff --git a/app/models/message/document_image.rb b/app/models/message/document_image.rb
@@ -8,23 +8,12 @@ module Message::DocumentImage
   end
 
   def has_document_image?(variant = nil)
-    has_image = documents.present? && documents.first.file.attached?
-    if has_image && variant
-      has_image = documents.first.has_file_variant_processed?(variant)
-    end
-
-    !!has_image
+    documents.present? && documents.first.has_image?(variant)
   end
 
-  def document_image_path(variant, fallback: nil)
+  def document_image_url(variant, fallback: nil)
     return nil unless has_document_image?
 
-    if documents.first.has_file_variant_processed?(variant)
-      documents.first.fully_processed_url(variant)
-    elsif fallback.nil?
-      documents.first.redirect_to_processed_path(variant)
-    else
-      fallback
-    end
+    documents.first.image_url(variant, fallback: fallback)
   end
 end
diff --git a/app/models/setting.rb b/app/models/setting.rb
@@ -27,5 +27,9 @@ def require_keys!(*keys)
         end
       end
     end
+
+    def key_set?(key)
+      send(key).present?
+    end
   end
 end
diff --git a/app/services/ai_backend/open_ai.rb b/app/services/ai_backend/open_ai.rb
@@ -98,7 +98,7 @@ def preceding_conversation_messages
 
         content_with_images = [{ type: "text", text: message.content_text }]
         content_with_images += message.documents.collect do |document|
-          { type: "image_url", image_url: { url: document.file_data_url(:large) }}
+          { type: "image_url", image_url: { url: document.image_url(:large) }}
         end
 
         {
diff --git a/app/views/messages/_message.html.erb b/app/views/messages/_message.html.erb
@@ -82,10 +82,10 @@ end %>
                 role: "image-preview",
                 controller: "image-loader",
                 image_loader_message_scroller_outlet: "[data-role='inner-message']",
-                image_loader_url_value: message.document_image_path(:small),
+                image_loader_url_value: message.document_image_url(:small),
                 action: "modal#open",
             } do %>
-              <%= image_tag message.document_image_path(:small, fallback: ""),
+              <%= image_tag message.document_image_url(:small, fallback: ""),
                 class: %|
                   my-0
                   mx-auto
@@ -254,10 +254,10 @@ end %>
               class="flex flex-col md:flex-row justify-center"
               data-controller="image-loader"
               data-image-loader-message-scroller-outlet="[data-role='inner-message']"
-              data-image-loader-url-value="<%= message.document_image_path(:large) %>"
+              data-image-loader-url-value="<%= message.document_image_url(:large) %>"
               data-turbo-permanent
             >
-              <%= image_tag message.document_image_path(:large, fallback: ""),
+              <%= image_tag message.document_image_url(:large, fallback: ""),
                 class: "w-full h-auto",
                 data: {
                   image_loader_target: "image",
diff --git a/config/application.rb b/config/application.rb
@@ -37,6 +37,26 @@ class Application < Rails::Application
     config.time_zone = "Central Time (US & Canada)"
     config.eager_load_paths << Rails.root.join("lib")
 
+    url_settings = [:app_url_protocol, :app_url_host]
+    if url_settings.any?{|k| Setting.key_set?(k)}
+      Setting.require_keys!(*url_settings)
+
+      config.x.app_url_protocol = Setting.app_url_protocol
+      config.x.app_url_host = Setting.app_url_host
+
+      port = Setting.app_url_port.to_s
+      if port.blank? || port == "80"
+        config.x.app_url_port = nil
+      else
+        config.x.app_url_port = Setting.app_url_port
+      end
+
+      port_str = config.x.app_url_port.present? ? ":#{port}" : ""
+      config.x.app_url = "#{Setting.app_url_protocol}://#{Setting.app_url_host}#{port_str}"
+    else
+      config.x.app_url = nil
+    end
+
     # Active Storage
     if Feature.cloudflare_storage?
       config.active_storage.service = :cloudflare
diff --git a/config/environments/development.rb b/config/environments/development.rb
@@ -85,9 +85,11 @@
   log_polling = ENV["SOLID_QUEUE_LOG_POLLING_ON"] != "false"
   config.solid_queue.silence_polling = log_polling # NOTE: this is backwards, true means silence
 
-  config.web_console.permissions = ["192.168.0.0/16", "172.17.0.0/16"]
+  config.web_console.permissions = ["192.168.0.0/16", "172.17.0.0/16", "172.18.0.0/16"]
 
+  # TODO should we combine this with APP_URL_HOST?
   config.hosts << ENV["DEV_HOST"] if ENV["DEV_HOST"].present?
+  config.hosts << Setting.app_url_host if Setting.key_set?(:app_url_host)
 
   stdout_logger = ActiveSupport::Logger.new(STDOUT)
   tagged_logger = ActiveSupport::TaggedLogging.new(stdout_logger)
diff --git a/config/environments/production.rb b/config/environments/production.rb
@@ -97,7 +97,9 @@
   #   "example.com",     # Allow requests from example.com
   #   /.*\.example\.com/ # Allow requests from subdomains like `www.example.com`
   # ]
-  config.hosts = ENV["PRODUCTION_HOST"].split(",").map(&:strip) if ENV["PRODUCTION_HOST"]
+  # TODO should we combine this with APP_URL_HOST?
+  config.hosts << ENV["PRODUCTION_HOST"].split(",").map(&:strip) if ENV["PRODUCTION_HOST"]
+  config.hosts << Setting.app_url_host if Setting.key_set?(:app_url_host)
 
   # Skip DNS rebinding protection for the default health check endpoint.
   config.host_authorization = { exclude: ->(request) { request.path == "/up" } }
diff --git a/config/options.yml b/config/options.yml
@@ -48,6 +48,9 @@ shared:
     password_reset_email: <%= ENV["PASSWORD_RESET_EMAIL_FEATURE"] || default_to(false, except_env_test: true) %>
   settings:
     # Be sure to add these ENV to docker-compose.yml
+    app_url_protocol: <%= ENV["APP_URL_PROTOCOL"] || default_to(app_url: :protocol) %>
+    app_url_host: <%= ENV["APP_URL_HOST"] || default_to(app_url: :host) %>
+    app_url_port: <%= ENV["APP_URL_PORT"] || default_to(app_url: :port) %>
     product_name: <%= ENV["PRODUCT_NAME"] || "HostedGPT" %>
     default_openai_key: <%= ENV["DEFAULT_OPENAI_KEY"] %>
     default_anthropic_key: <%= ENV["DEFAULT_ANTHROPIC_KEY"] %>
diff --git a/docker-compose.yml b/docker-compose.yml
@@ -22,6 +22,9 @@ services:
       target: development
     environment:
       # Be sure to add environment variables to config/options.yml
+      - APP_URL_PROTOCOL
+      - APP_URL_HOST
+      - APP_URL_PORT
       - DATABASE_URL=postgres://app:secret@postgres/app_development
       - DEV_HOST=${DEV_HOST:-localhost} # Set if you want to use a different hostname
       - OVERMIND_COLORS=2,3,5
diff --git a/lib/active_storage/service/postgresql_service.rb b/lib/active_storage/service/postgresql_service.rb
@@ -148,9 +148,20 @@ def url_helpers
 
     def url_options
       if ActiveStorage::Current.respond_to?(:url_options)
-        ActiveStorage::Current.url_options
+        url_opts = ActiveStorage::Current.url_options
+        return url_opts if url_opts.is_a?(Hash)
+      end
+
+      if Rails.application.config.x.app_url.present?
+        {
+          protocol: Rails.application.config.x.app_url_protocol,
+          host: Rails.application.config.x.app_url_host,
+          port: Rails.application.config.x.app_url_port,
+        }
       else
-        { host: ActiveStorage::Current.host }
+        {
+          only_path: true, # This fixes an exception with attachment URL generation from a worker: https://github.com/AllYourBot/hostedgpt/pull/398#issuecomment-2168135853
+        }
       end
     end
   end
diff --git a/test/lib/active_storage/service/public_postgresql_test.rb b/test/lib/active_storage/service/public_postgresql_test.rb
@@ -2,7 +2,6 @@
 require "./test/lib/active_storage/service/shared_service_tests"
 
 class ActiveStorage::Service::PublicPostgresqlTest < ActiveSupport::TestCase
-  SERVICE = ActiveStorage::Service.configure(:tmp_public, { tmp_public: { service: "Postgresql", public: true }})
   include ActiveStorage::Service::SharedServiceTests
 
   test "public URL generation" do
diff --git a/test/lib/active_storage/service/shared_service_tests.rb b/test/lib/active_storage/service/shared_service_tests.rb
@@ -11,6 +11,15 @@ module ActiveStorage::Service::SharedServiceTests
 
   included do
     setup do
+      stub_custom_config_value(:app_url_protocol, "https")
+      stub_custom_config_value(:app_url_host, "example.com")
+      stub_custom_config_value(:app_url_port, nil)
+      stub_custom_config_value(:app_url, "https://example.com")
+
+      unless self.class.const_defined?(:SERVICE)
+        self.class.const_set(:SERVICE, ActiveStorage::Service.configure(:tmp_public, { tmp_public: { service: "Postgresql", public: true } }))
+      end
+
       @service = self.class.const_get(:SERVICE)
       @service.upload FIXTURE_KEY, StringIO.new(FIXTURE_DATA)
       @key = FIXTURE_KEY
diff --git a/test/models/document_test.rb b/test/models/document_test.rb
@@ -1,6 +1,12 @@
 require "test_helper"
 
 class DocumentTest < ActiveSupport::TestCase
+  setup do
+    stub_custom_config_value(:app_url_protocol, "https")
+    stub_custom_config_value(:app_url_host, "example.com")
+    stub_custom_config_value(:app_url_port, nil)
+  end
+
   test "has an associated user" do
     assert_instance_of User, documents(:cat_photo).user
   end
@@ -39,25 +45,25 @@ class DocumentTest < ActiveSupport::TestCase
     assert document.file.attached?
   end
 
-  test "file_data_url returns for a file" do
-    prefix = "data:image/png;base64,"
-    assert documents(:cat_photo).file_data_url
-    assert documents(:cat_photo).file_data_url.starts_with?(prefix)
-    assert documents(:cat_photo).file_data_url.length > 40000
-  end
+  test "image_url returns data url when app_url is not set" do
+    stub_custom_config_value(:app_url, "") do
+      url = documents(:cat_photo).image_url(:small)
 
-  test "file_base64 returns a long string" do
-    assert documents(:cat_photo).file_base64.length > 40000
+      assert url.starts_with?("data:image/png;base64,")
+      assert url.length > 40000
+    end
   end
 
   test "has_file_variant_processed?" do
     refute documents(:cat_photo).has_file_variant_processed?(:small)
   end
 
   test "fully_processed_url" do
-    assert documents(:cat_photo).fully_processed_url(:small).starts_with?("http")
-    assert documents(:cat_photo).fully_processed_url(:small).include?("rails/active_storage/postgresql")
-    assert documents(:cat_photo).fully_processed_url(:small).exclude?("/redirect")
+    stub_custom_config_value(:app_url, "https://example.com") do
+      assert documents(:cat_photo).fully_processed_url(:small).starts_with?("http")
+      assert documents(:cat_photo).fully_processed_url(:small).include?("rails/active_storage/postgresql")
+      assert documents(:cat_photo).fully_processed_url(:small).exclude?("/redirect")
+    end
   end
 
   test "redirect_to_processed_path" do
diff --git a/test/models/message/document_image_test.rb b/test/models/message/document_image_test.rb
@@ -16,12 +16,11 @@ class Message::DocumentImageTest < ActiveSupport::TestCase
     refute messages(:examine_this).has_document_image?(:small)
   end
 
-  test "document_image_path" do
-    assert messages(:examine_this).document_image_path(:small).is_a?(String)
-    assert messages(:examine_this).document_image_path(:small).starts_with?("/rails/active_storage/representations/redirect")
-  end
-
-  test "document_image_path with fallback" do
-    assert_equal "", messages(:examine_this).document_image_path(:small, fallback: "")
+  test "document_image_url with data url" do
+    stub_custom_config_value(:app_url, "") do
+      url = messages(:examine_this).document_image_url(:small)
+      assert url.is_a?(String)
+      assert url.starts_with?("data:image/png;base64,")
+    end
   end
 end
diff --git a/test/support/config_helpers.rb b/test/support/config_helpers.rb
@@ -0,0 +1,35 @@
+module ConfigHelpers
+
+  def teardown
+    super
+
+    unless @original_values.nil?
+      unstub_custom_config_values
+    end
+  end
+
+  private
+
+  def stub_custom_config_value(key, value, &block)
+    @original_values ||= {}
+
+    if block_given?
+      Rails.application.config.x.stub(key, value) do
+        yield
+      end
+    else
+      @original_values[key] = Rails.application.config.x.send(key)
+
+      Rails.application.config.x.send("#{key}=", value)
+      Rails.application.reloader.reload!
+    end
+  end
+
+  def unstub_custom_config_values
+    @original_values.each do |key, value|
+      Rails.application.config.x.send("#{key}=", value)
+    end
+    Rails.application.reloader.reload!
+    @original_values = {}
+  end
+end
diff --git a/test/support/postgresql_helper.rb b/test/support/postgresql_helper.rb
diff --git a/test/test_helper.rb b/test/test_helper.rb

Original file line number	Diff line number	Diff line change
`@@ -98,7 +98,7 @@ def preceding_conversation_messages`
`98`	`98`
`99`	`99`	`content_with_images = [{ type: "text", text: message.content_text }]`
`100`	`100`	`content_with_images += message.documents.collect do \|document\|`
`101`		`- { type: "image_url", image_url: { url: document.file_data_url(:large) }}`
	`101`	`+ { type: "image_url", image_url: { url: document.image_url(:large) }}`
`102`	`102`	`end`
`103`	`103`
`104`	`104`	`{`