Skip to content

Commit

Permalink
Add app_url to the config so workers can create OpenAI image URLs (Al…
Browse files Browse the repository at this point in the history
…lYourBot#532)

Co-authored-by: Justin Vallelonga <jlvallelonga@gmail.com>
  • Loading branch information
krschacht and jlvallelonga authored Nov 24, 2024
1 parent 19fc3cb commit fc7a482
Show file tree
Hide file tree
Showing 21 changed files with 160 additions and 74 deletions.
6 changes: 6 additions & 0 deletions .github/workflows/rubyonrails.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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"

Expand Down
3 changes: 2 additions & 1 deletion Gemfile
Original file line number Diff line number Diff line change
Expand Up @@ -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"
Expand Down
35 changes: 26 additions & 9 deletions app/models/document.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand All @@ -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
Expand Down
4 changes: 1 addition & 3 deletions app/models/message.rb
Original file line number Diff line number Diff line change
Expand Up @@ -43,9 +43,7 @@ def finished?
(content_text.present? || content_tool_calls.present?)
end

def not_finished?
!finished?
end
def not_finished? = !finished?

private

Expand Down
17 changes: 3 additions & 14 deletions app/models/message/document_image.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
4 changes: 4 additions & 0 deletions app/models/setting.rb
Original file line number Diff line number Diff line change
Expand Up @@ -27,5 +27,9 @@ def require_keys!(*keys)
end
end
end

def key_set?(key)
send(key).present?
end
end
end
2 changes: 1 addition & 1 deletion app/services/ai_backend/open_ai.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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

{
Expand Down
8 changes: 4 additions & 4 deletions app/views/messages/_message.html.erb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down Expand Up @@ -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",
Expand Down
20 changes: 20 additions & 0 deletions config/application.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
4 changes: 3 additions & 1 deletion config/environments/development.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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)
Expand Down
4 changes: 3 additions & 1 deletion config/environments/production.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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" } }
Expand Down
3 changes: 3 additions & 0 deletions config/options.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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"] %>
Expand Down
3 changes: 3 additions & 0 deletions docker-compose.yml
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
15 changes: 13 additions & 2 deletions lib/active_storage/service/postgresql_service.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
1 change: 0 additions & 1 deletion test/lib/active_storage/service/public_postgresql_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
9 changes: 9 additions & 0 deletions test/lib/active_storage/service/shared_service_tests.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
Expand Down
28 changes: 17 additions & 11 deletions test/models/document_test.rb
Original file line number Diff line number Diff line change
@@ -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
Expand Down Expand Up @@ -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
Expand Down
13 changes: 6 additions & 7 deletions test/models/message/document_image_test.rb
Original file line number Diff line number Diff line change
Expand Up @@ -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
35 changes: 35 additions & 0 deletions test/support/config_helpers.rb
Original file line number Diff line number Diff line change
@@ -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
Loading

0 comments on commit fc7a482

Please sign in to comment.