From b79cdab300bbec5ef7ac20165d08bbec3e641e65 Mon Sep 17 00:00:00 2001 From: Melhaya Date: Tue, 2 Jul 2024 13:58:42 +0200 Subject: [PATCH 01/11] Adding an API Key per user Having an API key per user will allow users to utilize the AI capabilities within CodeHarbor (i.e. Unittest generator) - The generate unittest button is now always visible - If key is invalid or missing, an alert is shown to the user - If key is available and valid, a unittest will be created for the task --- app/controllers/tasks_controller.rb | 4 +- .../users/registrations_controller.rb | 2 +- app/errors/gpt/invalid_api_key_error.rb | 5 ++ app/models/user.rb | 1 + app/policies/task_policy.rb | 2 +- .../task_service/gpt_generate_tests.rb | 19 +++++++- app/views/tasks/show.html.slim | 7 ++- app/views/users/registrations/edit.html.slim | 8 ++++ app/views/users/show.html.slim | 9 ++++ config/initializers/open_ai.rb | 7 --- config/locales/de/controllers/tasks.yml | 1 + config/locales/de/views/tasks.yml | 1 + config/locales/de/views/users.yml | 3 ++ config/locales/en/controllers/tasks.yml | 1 + config/locales/en/views/tasks.yml | 1 + config/locales/en/views/users.yml | 3 ++ config/settings/development.yml | 1 - config/settings/production.yml | 1 - config/settings/test.yml | 1 - ...40703221801_add_openai_api_key_to_users.rb | 7 +++ db/schema.rb | 3 +- spec/controllers/tasks_controller_spec.rb | 28 +++++++---- spec/policies/task_policy_spec.rb | 48 +++++-------------- .../task_service/gpt_generate_tests_spec.rb | 42 ++++++++++++++-- 24 files changed, 140 insertions(+), 65 deletions(-) create mode 100644 app/errors/gpt/invalid_api_key_error.rb delete mode 100644 config/initializers/open_ai.rb create mode 100644 db/migrate/20240703221801_add_openai_api_key_to_users.rb diff --git a/app/controllers/tasks_controller.rb b/app/controllers/tasks_controller.rb index de5430995..d58187773 100644 --- a/app/controllers/tasks_controller.rb +++ b/app/controllers/tasks_controller.rb @@ -187,8 +187,10 @@ def export_external_confirm # rubocop:enable Metrics/AbcSize def generate_test - TaskService::GptGenerateTests.call(task: @task) + TaskService::GptGenerateTests.call(task: @task, openai_api_key: current_user.openai_api_key) flash[:notice] = I18n.t('tasks.task_service.gpt_generate_tests.successful_generation') + rescue Gpt::InvalidApiKeyError + flash[:alert] = I18n.t('tasks.task_service.gpt_generate_tests.invalid_api_key') rescue Gpt::MissingLanguageError flash[:alert] = I18n.t('tasks.task_service.gpt_generate_tests.no_language') rescue Gpt::InvalidTaskDescription diff --git a/app/controllers/users/registrations_controller.rb b/app/controllers/users/registrations_controller.rb index 23df9f4ac..e44053eaf 100644 --- a/app/controllers/users/registrations_controller.rb +++ b/app/controllers/users/registrations_controller.rb @@ -58,7 +58,7 @@ def configure_sign_up_params # If you have extra params to permit, append them to the sanitizer. def configure_account_update_params - devise_parameter_sanitizer.permit(:account_update, keys: %i[first_name last_name avatar]) + devise_parameter_sanitizer.permit(:account_update, keys: %i[first_name last_name avatar openai_api_key]) end def after_update_path_for(resource) diff --git a/app/errors/gpt/invalid_api_key_error.rb b/app/errors/gpt/invalid_api_key_error.rb new file mode 100644 index 000000000..9f281d605 --- /dev/null +++ b/app/errors/gpt/invalid_api_key_error.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +module Gpt + class InvalidApiKeyError < StandardError; end +end diff --git a/app/models/user.rb b/app/models/user.rb index 8d5886ab1..234163ef2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -17,6 +17,7 @@ class User < ApplicationRecord validates :email, presence: true, uniqueness: {case_sensitive: false} validates :first_name, :last_name, :status_group, presence: true validates :password_set, inclusion: [true, false] + validates :openai_api_key, allow_blank: true, length: {maximum: 255} has_many :tasks, dependent: :nullify diff --git a/app/policies/task_policy.rb b/app/policies/task_policy.rb index b86acaa3e..af406f700 100644 --- a/app/policies/task_policy.rb +++ b/app/policies/task_policy.rb @@ -54,7 +54,7 @@ def manage? end def generate_test? - Settings.open_ai.access_token.present? and update? + user.present? and user.openai_api_key.present? and update? end private diff --git a/app/services/task_service/gpt_generate_tests.rb b/app/services/task_service/gpt_generate_tests.rb index 72b60482e..8be7564bb 100644 --- a/app/services/task_service/gpt_generate_tests.rb +++ b/app/services/task_service/gpt_generate_tests.rb @@ -2,11 +2,13 @@ module TaskService class GptGenerateTests < ServiceBase - def initialize(task:) + def initialize(task:, openai_api_key:) super() raise Gpt::MissingLanguageError if task.programming_language&.language.blank? @task = task + @openai_api_key = openai_api_key.presence + validate_api_key! end def execute @@ -21,7 +23,7 @@ def execute private def client - @client ||= OpenAI::Client.new + @client ||= OpenAI::Client.new(access_token: @openai_api_key) end def gpt_response @@ -64,5 +66,18 @@ def training_prompts PROMPT ] end + + def validate_api_key! + if @openai_api_key.blank? + raise Gpt::InvalidApiKeyError + else + begin + response = client.models.list + raise Gpt::InvalidApiKeyError unless response['data'] + rescue Faraday::UnauthorizedError, OpenAI::Error + raise Gpt::InvalidApiKeyError + end + end + end end end diff --git a/app/views/tasks/show.html.slim b/app/views/tasks/show.html.slim index 4a8de8885..365154a79 100644 --- a/app/views/tasks/show.html.slim +++ b/app/views/tasks/show.html.slim @@ -576,7 +576,12 @@ - if policy(@task).generate_test? = link_to generate_test_task_path(@task), method: :post, class: 'btn btn-important' do i.fa-solid.fa-wand-magic-sparkles - =< t('.button.generate_test') + = t('.button.generate_test') + - else + div data-bs-toggle='tooltip' title=t('.button.api_key_required') data-bs-delay='150' + = link_to '#', method: :post, class: 'btn btn-important disabled' do + i.fa-solid.fa-wand-magic-sparkles + = t('.button.generate_test') - if current_user.present? = link_to t('common.button.back'), tasks_path, class: 'btn btn-important' diff --git a/app/views/users/registrations/edit.html.slim b/app/views/users/registrations/edit.html.slim index 70320d7e3..165faca7f 100644 --- a/app/views/users/registrations/edit.html.slim +++ b/app/views/users/registrations/edit.html.slim @@ -51,6 +51,14 @@ autocomplete: 'new-password', class: 'form-control' + .form-group.field-element + = f.label :openai_api_key, t('users.show.openai_api_key'), class: 'form-label' + = f.text_field :openai_api_key, + required: false, + placeholder: t('users.show.openai_api_key'), + autocomplete: 'off', + class: 'form-control' + = render 'avatar_form', f: - if resource.password_set? diff --git a/app/views/users/show.html.slim b/app/views/users/show.html.slim index 17f8945c9..6129318fc 100644 --- a/app/views/users/show.html.slim +++ b/app/views/users/show.html.slim @@ -46,6 +46,15 @@ | : .col.row-value = @user.email + .row + .col-auto.row-label + = t('.openai_api_key') + | : + .col.row-value + - if @user.openai_api_key.present? + = t('.provided_openai_key') + - else + = t('.not_provided_openai_key') .row.vertical .col.row-label = t('.account_links.created') diff --git a/config/initializers/open_ai.rb b/config/initializers/open_ai.rb deleted file mode 100644 index 1cfe95249..000000000 --- a/config/initializers/open_ai.rb +++ /dev/null @@ -1,7 +0,0 @@ -# frozen_string_literal: true - -OpenAI.configure do |config| - next unless Settings.open_ai - - config.access_token = Settings.open_ai.access_token -end diff --git a/config/locales/de/controllers/tasks.yml b/config/locales/de/controllers/tasks.yml index 60708c2bf..6c9480dfa 100644 --- a/config/locales/de/controllers/tasks.yml +++ b/config/locales/de/controllers/tasks.yml @@ -27,6 +27,7 @@ de: task_found: 'In der externen App wurde eine entsprechende Aufgabe gefunden. Sie können: ' task_found_no_right: 'In der externen App wurde eine entsprechende Aufgabe gefunden, aber Sie haben keine Rechte, sie zu bearbeiten. Sie können: ' gpt_generate_tests: + invalid_api_key: Der API-Schlüssel fehlt in Ihrem Profil oder ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um diese Funktion zu nutzen. invalid_description: Die angegebene Aufgabenbeschreibung stellt keine gültige Programmieraufgabe dar und kann daher nicht zum Generieren eines Unit-Tests genutzt werden. Bitte stellen Sie sicher, dass die Aufgabenbeschreibung eine klar formulierte Problemstellung enthält, die durch ein Programm gelöst werden kann. no_language: Für diese Aufgabe ist keine Programmiersprache angegeben. Bitte geben Sie die Sprache an, bevor Sie fortfahren. successful_generation: Unit-Test erfolgreich generiert. Bitte überprüfen Sie den generierten Test und vergeben Sie einen passenden Dateinamen. diff --git a/config/locales/de/views/tasks.yml b/config/locales/de/views/tasks.yml index dd81581f9..7f7a87b88 100644 --- a/config/locales/de/views/tasks.yml +++ b/config/locales/de/views/tasks.yml @@ -83,6 +83,7 @@ de: add_to_collection_hint: Speichern Sie Aufgaben für später, indem Sie sie zu einer Sammlung hinzufügen. button: add_to_collection: Zu Sammlung hinzufügen + api_key_required: OpenAI API-Schlüssel ist erforderlich, um einen Test zu erstellen create_collection: Neue Sammlung anlegen download_as_zip: Diese Aufgabe als ZIP-Datei herunterladen. export: Exportieren diff --git a/config/locales/de/views/users.yml b/config/locales/de/views/users.yml index bd6ef1292..ea74319a6 100644 --- a/config/locales/de/views/users.yml +++ b/config/locales/de/views/users.yml @@ -51,6 +51,9 @@ de: delete_modal: title: Warnung full_name: Vollständiger Name + not_provided_openai_key: Nicht eingegeben + openai_api_key: OpenAI API-Schlüssel private_information: Private Informationen + provided_openai_key: Eingegeben public_information: Öffentliche Informationen send_message: Nachricht senden diff --git a/config/locales/en/controllers/tasks.yml b/config/locales/en/controllers/tasks.yml index 319d6c9e9..04ada1a63 100644 --- a/config/locales/en/controllers/tasks.yml +++ b/config/locales/en/controllers/tasks.yml @@ -27,6 +27,7 @@ en: task_found: 'A corresponding task has been found on the external app. You can: ' task_found_no_right: 'A corresponding task has been found on external app, but you don''t have the rights to edit it. You can: ' gpt_generate_tests: + invalid_api_key: The API key is missing from your profile or is invalid. Please add the appropriate API key in your profile to use this feature. invalid_description: The task description provided does not represent a valid programming task and therefore cannot be used to generate a unit test. Please make sure that the task description contains a clearly formulated problem that can be solved by a program. no_language: Programming language is not specified for this task. Please specify the language before proceeding. successful_generation: Unit test generated successfully. Please check the generated test and assign an appropriate filename. diff --git a/config/locales/en/views/tasks.yml b/config/locales/en/views/tasks.yml index de76b8374..34ce672f4 100644 --- a/config/locales/en/views/tasks.yml +++ b/config/locales/en/views/tasks.yml @@ -83,6 +83,7 @@ en: add_to_collection_hint: Save Tasks for later by adding them to a collection. button: add_to_collection: Add to Collection + api_key_required: OpenAI API key is required to generate a test create_collection: Create new Collection download_as_zip: Download this Task as a ZIP file. export: Export diff --git a/config/locales/en/views/users.yml b/config/locales/en/views/users.yml index f423f7100..6a3675a09 100644 --- a/config/locales/en/views/users.yml +++ b/config/locales/en/views/users.yml @@ -51,6 +51,9 @@ en: delete_modal: title: Warning full_name: Full name + not_provided_openai_key: Not entered + openai_api_key: OpenAI API Key private_information: Private Information + provided_openai_key: Entered public_information: Public Information send_message: Send Message diff --git a/config/settings/development.yml b/config/settings/development.yml index 577d52f06..abc211a4f 100644 --- a/config/settings/development.yml +++ b/config/settings/development.yml @@ -14,5 +14,4 @@ omniauth: oai_pmh: admin_mail: admin@example.org open_ai: - access_token: ~ # Add a valid API key from https://platform.openai.com/api-keys model: gpt-3.5-turbo diff --git a/config/settings/production.yml b/config/settings/production.yml index 4271c5887..5ac71c988 100644 --- a/config/settings/production.yml +++ b/config/settings/production.yml @@ -15,5 +15,4 @@ omniauth: oai_pmh: admin_mail: admin@example.org open_ai: - access_token: ~ # Add a valid API key from https://platform.openai.com/api-keys model: gpt-3.5-turbo diff --git a/config/settings/test.yml b/config/settings/test.yml index 67503445e..e2c546538 100644 --- a/config/settings/test.yml +++ b/config/settings/test.yml @@ -21,5 +21,4 @@ omniauth: oai_pmh: admin_mail: admin@example.org open_ai: - access_token: ~ # Add a valid API key from https://platform.openai.com/api-keys model: gpt-3.5-turbo diff --git a/db/migrate/20240703221801_add_openai_api_key_to_users.rb b/db/migrate/20240703221801_add_openai_api_key_to_users.rb new file mode 100644 index 000000000..dba7151fc --- /dev/null +++ b/db/migrate/20240703221801_add_openai_api_key_to_users.rb @@ -0,0 +1,7 @@ +# frozen_string_literal: true + +class AddOpenaiApiKeyToUsers < ActiveRecord::Migration[7.1] + def change + add_column :users, :openai_api_key, :string + end +end diff --git a/db/schema.rb b/db/schema.rb index 7c9e8a585..8b9a81aa7 100644 --- a/db/schema.rb +++ b/db/schema.rb @@ -10,7 +10,7 @@ # # It's strongly recommended that you check this file into your version control system. -ActiveRecord::Schema[7.1].define(version: 2024_05_31_161908) do +ActiveRecord::Schema[7.1].define(version: 2024_07_03_221801) do # These are extensions that must be enabled in order to support this database enable_extension "pgcrypto" enable_extension "plpgsql" @@ -327,6 +327,7 @@ t.string "preferred_locale" t.boolean "password_set", default: true, null: false t.integer "status_group", limit: 2, default: 0, null: false, comment: "Used as enum in Rails" + t.string "openai_api_key" t.index ["confirmation_token"], name: "index_users_on_confirmation_token", unique: true t.index ["email"], name: "index_users_on_email", unique: true t.index ["reset_password_token"], name: "index_users_on_reset_password_token", unique: true diff --git a/spec/controllers/tasks_controller_spec.rb b/spec/controllers/tasks_controller_spec.rb index 42051852a..da2e602bb 100644 --- a/spec/controllers/tasks_controller_spec.rb +++ b/spec/controllers/tasks_controller_spec.rb @@ -1,6 +1,7 @@ # frozen_string_literal: true require 'rails_helper' +require 'webmock/rspec' RSpec.describe TasksController do render_views @@ -9,7 +10,6 @@ let(:collection) { create(:collection, users: [user], tasks: []) } let(:valid_attributes) { {user:, access_level:} } let(:access_level) { :private } - let(:invalid_attributes) { {title: ''} } describe 'GET #index' do @@ -1043,17 +1043,12 @@ end describe 'POST #generate_test' do - let(:task_user) { create(:user) } + let(:task_user) { create(:user, openai_api_key: 'valid_api_key') } let(:access_level) { :public } let(:task) { create(:task, user: task_user, access_level:) } before do sign_in task_user - Settings.open_ai.access_token = 'access_token' - end - - after do - Settings.open_ai.access_token = nil end context 'when GptGenerateTests is successful' do @@ -1062,8 +1057,8 @@ post :generate_test, params: {id: task.id} end - it 'calls the GptGenerateTests service with the correct task' do - expect(TaskService::GptGenerateTests).to have_received(:call).with(task:) + it 'calls the GptGenerateTests service with the correct parameters' do + expect(TaskService::GptGenerateTests).to have_received(:call).with(task:, openai_api_key: 'valid_api_key') end it 'redirects to the task show page' do @@ -1104,5 +1099,20 @@ expect(flash[:alert]).to eq(I18n.t('tasks.task_service.gpt_generate_tests.invalid_description')) end end + + context 'when GptGenerateTests raises InvalidApiKeyError' do + before do + allow(TaskService::GptGenerateTests).to receive(:call).and_raise(Gpt::InvalidApiKeyError) + post :generate_test, params: {id: task.id} + end + + it 'redirects to the task show page' do + expect(response).to redirect_to(task_path(task)) + end + + it 'sets flash to the appropriate message' do + expect(flash[:alert]).to eq(I18n.t('tasks.task_service.gpt_generate_tests.invalid_api_key')) + end + end end end diff --git a/spec/policies/task_policy_spec.rb b/spec/policies/task_policy_spec.rb index 3fccd05f7..b12998031 100644 --- a/spec/policies/task_policy_spec.rb +++ b/spec/policies/task_policy_spec.rb @@ -9,6 +9,7 @@ let(:groups) { [] } let(:access_level) { :private } let(:task) { create(:task, user: task_user, access_level:, groups:) } + let(:openai_api_key) { nil } context 'without a user' do let(:user) { nil } @@ -27,26 +28,18 @@ end context 'with a user' do - let(:user) { create(:user) } + let(:user) { create(:user, openai_api_key:) } let(:generic_user_permissions) { %i[index new import_start import_confirm import_uuid_check import_external] } it { is_expected.to permit_only_actions(generic_user_permissions) } context 'when user is admin' do - let(:user) { create(:admin) } + let(:user) { create(:admin, openai_api_key:) } - context 'without gpt access token' do - it { is_expected.to forbid_only_actions %i[generate_test] } - end + it { is_expected.to forbid_only_actions %i[generate_test] } context 'with gpt access token' do - before do - Settings.open_ai.access_token = 'access_token' - end - - after do - Settings.open_ai.access_token = nil - end + let(:openai_api_key) { 'access_token' } it { is_expected.to permit_all_actions } end @@ -55,18 +48,10 @@ context 'when task is from user' do let(:task_user) { user } - context 'without gpt access token' do - it { is_expected.to forbid_only_actions %i[generate_test] } - end + it { is_expected.to forbid_only_actions %i[generate_test] } context 'with gpt access token' do - before do - Settings.open_ai.access_token = 'access_token' - end - - after do - Settings.open_ai.access_token = nil - end + let(:openai_api_key) { 'access_token' } it { is_expected.to permit_all_actions } end @@ -81,7 +66,7 @@ context 'when task is "private" and in same group' do let(:access_level) { :private } - let(:user) { create(:user) } + let(:user) { create(:user, openai_api_key:) } let(:role) { :confirmed_member } let(:group_memberships) { [build(:group_membership, :with_admin), build(:group_membership, user:, role:)] } @@ -92,22 +77,13 @@ context 'when user is group-admin' do let(:role) { :admin } - context 'without gpt access token' do - it { is_expected.to permit_only_actions(group_member_permissions) } - end + it { is_expected.to permit_only_actions(group_member_permissions) } context 'with gpt access token' do - let(:group_member_permissions) { generic_user_permissions + %i[edit update duplicate show export_external_start export_external_check export_external_confirm download add_to_collection duplicate generate_test] } - - before do - Settings.open_ai.access_token = 'access_token' - end - - after do - Settings.open_ai.access_token = nil - end + let(:openai_api_key) { 'access_token' } + let(:group_member_permissions_with_generate_test) { group_member_permissions + %i[generate_test] } - it { is_expected.to permit_only_actions(group_member_permissions) } + it { is_expected.to permit_only_actions(group_member_permissions_with_generate_test) } end end end diff --git a/spec/services/task_service/gpt_generate_tests_spec.rb b/spec/services/task_service/gpt_generate_tests_spec.rb index 85d135e53..b2e3e5888 100644 --- a/spec/services/task_service/gpt_generate_tests_spec.rb +++ b/spec/services/task_service/gpt_generate_tests_spec.rb @@ -2,18 +2,29 @@ # frozen_string_literal: true require 'rails_helper' +require 'webmock/rspec' RSpec.describe TaskService::GptGenerateTests do describe '.new' do - subject(:gpt_generate_tests_service) { described_class.new(task:) } + subject(:gpt_generate_tests_service) { described_class.new(task:, openai_api_key:) } let(:programming_language) { build(:programming_language, :ruby) } let(:task) { build(:task, description: 'Sample Task', programming_language:) } + let(:openai_api_key) { 'valid_api_key' } + let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } + + before do + allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models)) + end it 'assigns task' do expect(gpt_generate_tests_service.instance_variable_get(:@task)).to be task end + it 'assigns openai_api_key' do + expect(gpt_generate_tests_service.instance_variable_get(:@openai_api_key)).to eq openai_api_key + end + context 'when language is missing' do let(:task) { build(:task, description: 'Sample Task', programming_language: nil) } @@ -21,17 +32,42 @@ expect { gpt_generate_tests_service }.to raise_error(Gpt::MissingLanguageError) end end + + context 'when API key is missing' do + let(:openai_api_key) { nil } + + it 'raises InvalidApiKeyError' do + expect { gpt_generate_tests_service }.to raise_error(Gpt::InvalidApiKeyError) + end + end + + context 'when API key is invalid' do + let(:openai_api_key) { 'invalid_api_key' } + let(:mock_models_invalid) { instance_double(OpenAI::Models) } + + before do + allow(mock_models_invalid).to receive(:list).and_raise(Faraday::UnauthorizedError) + allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models_invalid)) + end + + it 'raises InvalidApiKeyError' do + expect { gpt_generate_tests_service }.to raise_error(Gpt::InvalidApiKeyError) + end + end end describe '#call' do - subject(:gpt_generate_tests) { described_class.call(task:) } + subject(:gpt_generate_tests) { described_class.call(task:, openai_api_key:) } let(:programming_language) { create(:programming_language, :python) } let(:task) { create(:task, description: 'Create a Python script.', programming_language:) } + let(:openai_api_key) { 'valid_api_key' } let(:mock_client) { instance_double(OpenAI::Client) } + let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'text-davinci-002'}]}) } before do allow(OpenAI::Client).to receive(:new).and_return(mock_client) + allow(mock_client).to receive(:models).and_return(mock_models) end context 'when the response includes valid code blocks' do @@ -59,7 +95,7 @@ allow(mock_client).to receive(:chat).and_return({'choices' => [{'message' => {'content' => 'Python script should assert true without any code block.'}}]}) end - it 'raises InvalidTaskDescription when response does not contain backticks' do + it 'raises InvalidTaskDescription' do expect { gpt_generate_tests }.to raise_error(Gpt::InvalidTaskDescription) end end From eb9b0ea5a5b372462caf752170e29c45a35145de Mon Sep 17 00:00:00 2001 From: Karol Date: Tue, 9 Jul 2024 23:32:22 +0200 Subject: [PATCH 02/11] Added a hint for openAI entry Added tests in user.rb remove redundant code in spec files --- app/models/user.rb | 15 +++++- app/policies/task_policy.rb | 2 +- .../task_service/gpt_generate_tests.rb | 2 +- app/views/tasks/show.html.slim | 12 +++-- app/views/users/registrations/edit.html.slim | 2 + app/views/users/show.html.slim | 4 +- config/locales/de/controllers/tasks.yml | 2 +- config/locales/de/models.yml | 1 + config/locales/de/views/users.yml | 5 +- config/locales/en/controllers/tasks.yml | 2 +- config/locales/en/models.yml | 1 + config/locales/en/views/users.yml | 5 +- spec/controllers/tasks_controller_spec.rb | 19 ++------ spec/models/user_spec.rb | 47 +++++++++++++++++++ spec/policies/task_policy_spec.rb | 28 +++++------ .../task_service/gpt_generate_tests_spec.rb | 1 - 16 files changed, 100 insertions(+), 48 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 234163ef2..9e00691c2 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -17,7 +17,7 @@ class User < ApplicationRecord validates :email, presence: true, uniqueness: {case_sensitive: false} validates :first_name, :last_name, :status_group, presence: true validates :password_set, inclusion: [true, false] - validates :openai_api_key, allow_blank: true, length: {maximum: 255} + validate :validate_openai_api_key, if: -> { openai_api_key.present? } has_many :tasks, dependent: :nullify @@ -147,6 +147,19 @@ def to_s name end + def validate_openai_api_key + return unless openai_api_key_changed? + + client = OpenAI::Client.new(access_token: openai_api_key) + + begin + response = client.models.list + errors.add(:base, :invalid_api_key) unless response['data'] + rescue Faraday::UnauthorizedError, OpenAI::Error + errors.add(:base, :invalid_api_key) + end + end + private def avatar_format diff --git a/app/policies/task_policy.rb b/app/policies/task_policy.rb index af406f700..2c0f00815 100644 --- a/app/policies/task_policy.rb +++ b/app/policies/task_policy.rb @@ -54,7 +54,7 @@ def manage? end def generate_test? - user.present? and user.openai_api_key.present? and update? + user&.openai_api_key.present? and update? end private diff --git a/app/services/task_service/gpt_generate_tests.rb b/app/services/task_service/gpt_generate_tests.rb index 8be7564bb..d1c1fc9db 100644 --- a/app/services/task_service/gpt_generate_tests.rb +++ b/app/services/task_service/gpt_generate_tests.rb @@ -7,7 +7,7 @@ def initialize(task:, openai_api_key:) raise Gpt::MissingLanguageError if task.programming_language&.language.blank? @task = task - @openai_api_key = openai_api_key.presence + @openai_api_key = openai_api_key validate_api_key! end diff --git a/app/views/tasks/show.html.slim b/app/views/tasks/show.html.slim index 365154a79..e25ed40de 100644 --- a/app/views/tasks/show.html.slim +++ b/app/views/tasks/show.html.slim @@ -574,12 +574,16 @@ =< t('common.button.duplicate') - if policy(@task).generate_test? - = link_to generate_test_task_path(@task), method: :post, class: 'btn btn-important' do + = link_to generate_test_task_path(@task), + method: :post, + class: 'btn btn-important' do i.fa-solid.fa-wand-magic-sparkles = t('.button.generate_test') - - else - div data-bs-toggle='tooltip' title=t('.button.api_key_required') data-bs-delay='150' - = link_to '#', method: :post, class: 'btn btn-important disabled' do + - elsif policy(@task).update? + div data-bs-toggle='tooltip' title=t('.button.api_key_required') data-bs-delay=150 + = link_to generate_test_task_path(@task), + method: :post, + class: 'btn btn-important disabled' do i.fa-solid.fa-wand-magic-sparkles = t('.button.generate_test') diff --git a/app/views/users/registrations/edit.html.slim b/app/views/users/registrations/edit.html.slim index 165faca7f..52a87473b 100644 --- a/app/views/users/registrations/edit.html.slim +++ b/app/views/users/registrations/edit.html.slim @@ -58,6 +58,8 @@ placeholder: t('users.show.openai_api_key'), autocomplete: 'off', class: 'form-control' + small.form-text.text-body-secondary + = t('.openai_api_key_usage_html', openai_api_link: 'https://platform.openai.com/api-keys') = render 'avatar_form', f: diff --git a/app/views/users/show.html.slim b/app/views/users/show.html.slim index 6129318fc..2356b00ae 100644 --- a/app/views/users/show.html.slim +++ b/app/views/users/show.html.slim @@ -52,9 +52,9 @@ | : .col.row-value - if @user.openai_api_key.present? - = t('.provided_openai_key') + = t('.openai_api_key_provided') - else - = t('.not_provided_openai_key') + = t('.openai_api_key_not_provided') .row.vertical .col.row-label = t('.account_links.created') diff --git a/config/locales/de/controllers/tasks.yml b/config/locales/de/controllers/tasks.yml index 6c9480dfa..05e0f0519 100644 --- a/config/locales/de/controllers/tasks.yml +++ b/config/locales/de/controllers/tasks.yml @@ -27,7 +27,7 @@ de: task_found: 'In der externen App wurde eine entsprechende Aufgabe gefunden. Sie können: ' task_found_no_right: 'In der externen App wurde eine entsprechende Aufgabe gefunden, aber Sie haben keine Rechte, sie zu bearbeiten. Sie können: ' gpt_generate_tests: - invalid_api_key: Der API-Schlüssel fehlt in Ihrem Profil oder ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um diese Funktion zu nutzen. + invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um die KI-Funktionen zu nutzen. invalid_description: Die angegebene Aufgabenbeschreibung stellt keine gültige Programmieraufgabe dar und kann daher nicht zum Generieren eines Unit-Tests genutzt werden. Bitte stellen Sie sicher, dass die Aufgabenbeschreibung eine klar formulierte Problemstellung enthält, die durch ein Programm gelöst werden kann. no_language: Für diese Aufgabe ist keine Programmiersprache angegeben. Bitte geben Sie die Sprache an, bevor Sie fortfahren. successful_generation: Unit-Test erfolgreich generiert. Bitte überprüfen Sie den generierten Test und vergeben Sie einen passenden Dateinamen. diff --git a/config/locales/de/models.yml b/config/locales/de/models.yml index 1067db551..fd73f77c7 100644 --- a/config/locales/de/models.yml +++ b/config/locales/de/models.yml @@ -118,6 +118,7 @@ de: avatar: not_an_image: muss ein Bild sein size_over_10_mb: Größe muss kleiner als 10MB sein + invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um die KI-Funktionen zu nutzen. models: account_link: one: Account-Link diff --git a/config/locales/de/views/users.yml b/config/locales/de/views/users.yml index ea74319a6..08e12321b 100644 --- a/config/locales/de/views/users.yml +++ b/config/locales/de/views/users.yml @@ -28,6 +28,7 @@ de: add_identity: Account mit %{kind} verknüpfen cannot_remove_last_identity: Account-Verknüpfung zu %{kind} kann nicht entfernt werden, da weder ein Passwort gesetzt noch eine andere Identität verknüpft ist manage_omniauth: Verknüpfte Accounts verwalten + openai_api_key_usage_html: Geben Sie hier Ihren OpenAI-API-Schlüssel ein, um verschiedene KI-Funktionen innerhalb der Plattform zu nutzen. Sie können einen API-Schlüssel auf der OpenAI-Website erstellen. remove_identity: Account-Verknüpfung zu %{kind} entfernen shared: notification_modal: @@ -51,9 +52,9 @@ de: delete_modal: title: Warnung full_name: Vollständiger Name - not_provided_openai_key: Nicht eingegeben openai_api_key: OpenAI API-Schlüssel + openai_api_key_not_provided: Nicht eingegeben + openai_api_key_provided: Eingegeben private_information: Private Informationen - provided_openai_key: Eingegeben public_information: Öffentliche Informationen send_message: Nachricht senden diff --git a/config/locales/en/controllers/tasks.yml b/config/locales/en/controllers/tasks.yml index 04ada1a63..0de2004a2 100644 --- a/config/locales/en/controllers/tasks.yml +++ b/config/locales/en/controllers/tasks.yml @@ -27,7 +27,7 @@ en: task_found: 'A corresponding task has been found on the external app. You can: ' task_found_no_right: 'A corresponding task has been found on external app, but you don''t have the rights to edit it. You can: ' gpt_generate_tests: - invalid_api_key: The API key is missing from your profile or is invalid. Please add the appropriate API key in your profile to use this feature. + invalid_api_key: The API key in your profile is invalid. Please add the appropriate API key in your profile to use the AI features. invalid_description: The task description provided does not represent a valid programming task and therefore cannot be used to generate a unit test. Please make sure that the task description contains a clearly formulated problem that can be solved by a program. no_language: Programming language is not specified for this task. Please specify the language before proceeding. successful_generation: Unit test generated successfully. Please check the generated test and assign an appropriate filename. diff --git a/config/locales/en/models.yml b/config/locales/en/models.yml index abfe61180..12c304832 100644 --- a/config/locales/en/models.yml +++ b/config/locales/en/models.yml @@ -118,6 +118,7 @@ en: avatar: not_an_image: needs to be an image size_over_10_mb: size needs to be less than 10MB + invalid_api_key: The API key in your profile is invalid. Please add the appropriate API key in your profile to use the AI features. models: account_link: one: Account link diff --git a/config/locales/en/views/users.yml b/config/locales/en/views/users.yml index 6a3675a09..4912ce059 100644 --- a/config/locales/en/views/users.yml +++ b/config/locales/en/views/users.yml @@ -28,6 +28,7 @@ en: add_identity: Link %{kind} account cannot_remove_last_identity: Cannot remove account link to %{kind} because neither a password is set nor another identity is linked manage_omniauth: Manage linked accounts + openai_api_key_usage_html: Enter your OpenAI API key here to use various AI features within the platform. You can create an API key on the OpenAI website. remove_identity: Remove account link to %{kind} shared: notification_modal: @@ -51,9 +52,9 @@ en: delete_modal: title: Warning full_name: Full name - not_provided_openai_key: Not entered openai_api_key: OpenAI API Key + openai_api_key_not_provided: Not entered + openai_api_key_provided: Entered private_information: Private Information - provided_openai_key: Entered public_information: Public Information send_message: Send Message diff --git a/spec/controllers/tasks_controller_spec.rb b/spec/controllers/tasks_controller_spec.rb index da2e602bb..e061fc506 100644 --- a/spec/controllers/tasks_controller_spec.rb +++ b/spec/controllers/tasks_controller_spec.rb @@ -1,7 +1,6 @@ # frozen_string_literal: true require 'rails_helper' -require 'webmock/rspec' RSpec.describe TasksController do render_views @@ -10,6 +9,7 @@ let(:collection) { create(:collection, users: [user], tasks: []) } let(:valid_attributes) { {user:, access_level:} } let(:access_level) { :private } + let(:invalid_attributes) { {title: ''} } describe 'GET #index' do @@ -1046,8 +1046,10 @@ let(:task_user) { create(:user, openai_api_key: 'valid_api_key') } let(:access_level) { :public } let(:task) { create(:task, user: task_user, access_level:) } + let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } before do + allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models)) sign_in task_user end @@ -1099,20 +1101,5 @@ expect(flash[:alert]).to eq(I18n.t('tasks.task_service.gpt_generate_tests.invalid_description')) end end - - context 'when GptGenerateTests raises InvalidApiKeyError' do - before do - allow(TaskService::GptGenerateTests).to receive(:call).and_raise(Gpt::InvalidApiKeyError) - post :generate_test, params: {id: task.id} - end - - it 'redirects to the task show page' do - expect(response).to redirect_to(task_path(task)) - end - - it 'sets flash to the appropriate message' do - expect(flash[:alert]).to eq(I18n.t('tasks.task_service.gpt_generate_tests.invalid_api_key')) - end - end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index f864ef6e8..e7888c237 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -50,6 +50,53 @@ expect(user.errors[:avatar]).to include(I18n.t('activerecord.errors.models.user.attributes.avatar.size_over_10_mb')) end end + + context 'when openai_api_key is present and valid' do + before do + allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models)) + user.openai_api_key = 'valid_key' + end + + let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } + + it 'is valid' do + expect(user).to be_valid + end + end + + context 'when openai_api_key is present and invalid' do + before do + allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models)) + allow(mock_models).to receive(:list).and_raise(OpenAI::Error) + user.openai_api_key = 'invalid_key' + user.valid? + end + + let(:mock_models) { instance_double(OpenAI::Models) } + + it 'is not valid' do + expect(user).not_to be_valid + end + + it 'adds an error for invalid api key' do + expect(user.errors[:base]).to include(I18n.t('activerecord.errors.models.user.invalid_api_key')) + end + end + + context 'when openai_api_key remains the same' do + before do + allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models)) + user.openai_api_key = 'same_key' + user.save! + user.valid? + end + + let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } + + it 'does not trigger API validation' do + expect(user.errors[:base]).not_to include(I18n.t('activerecord.errors.models.user.invalid_api_key')) + end + end end describe '#destroy' do diff --git a/spec/policies/task_policy_spec.rb b/spec/policies/task_policy_spec.rb index b12998031..03d008876 100644 --- a/spec/policies/task_policy_spec.rb +++ b/spec/policies/task_policy_spec.rb @@ -36,24 +36,22 @@ context 'when user is admin' do let(:user) { create(:admin, openai_api_key:) } - it { is_expected.to forbid_only_actions %i[generate_test] } + context 'without gpt access token' do + let(:openai_api_key) { nil } - context 'with gpt access token' do - let(:openai_api_key) { 'access_token' } - - it { is_expected.to permit_all_actions } + it { is_expected.to forbid_actions(%i[generate_test]) } + it { is_expected.to permit_actions(generic_user_permissions) } end end context 'when task is from user' do let(:task_user) { user } - it { is_expected.to forbid_only_actions %i[generate_test] } - - context 'with gpt access token' do - let(:openai_api_key) { 'access_token' } + context 'without gpt access token' do + let(:openai_api_key) { nil } - it { is_expected.to permit_all_actions } + it { is_expected.to forbid_actions(%i[generate_test]) } + it { is_expected.to permit_actions(generic_user_permissions + %i[edit update show export_external_start export_external_check export_external_confirm download add_to_collection duplicate]) } end end @@ -77,13 +75,11 @@ context 'when user is group-admin' do let(:role) { :admin } - it { is_expected.to permit_only_actions(group_member_permissions) } - - context 'with gpt access token' do - let(:openai_api_key) { 'access_token' } - let(:group_member_permissions_with_generate_test) { group_member_permissions + %i[generate_test] } + context 'without gpt access token' do + let(:openai_api_key) { nil } - it { is_expected.to permit_only_actions(group_member_permissions_with_generate_test) } + it { is_expected.to forbid_actions(%i[generate_test]) } + it { is_expected.to permit_actions(group_member_permissions) } end end end diff --git a/spec/services/task_service/gpt_generate_tests_spec.rb b/spec/services/task_service/gpt_generate_tests_spec.rb index b2e3e5888..3a33fd01c 100644 --- a/spec/services/task_service/gpt_generate_tests_spec.rb +++ b/spec/services/task_service/gpt_generate_tests_spec.rb @@ -2,7 +2,6 @@ # frozen_string_literal: true require 'rails_helper' -require 'webmock/rspec' RSpec.describe TaskService::GptGenerateTests do describe '.new' do From 574dcc4c0c833bea1d69f38456067fd32067c093 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Fri, 12 Jul 2024 09:53:47 +0200 Subject: [PATCH 03/11] Refactor OpenAI API key validation --- app/models/user.rb | 15 +++---- .../task_service/gpt_generate_tests.rb | 32 +++++++-------- spec/models/user_spec.rb | 39 +++++++++++-------- .../task_service/gpt_generate_tests_spec.rb | 19 +++++---- 4 files changed, 54 insertions(+), 51 deletions(-) diff --git a/app/models/user.rb b/app/models/user.rb index 9e00691c2..a14861d9c 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -147,21 +147,16 @@ def to_s name end + private + def validate_openai_api_key return unless openai_api_key_changed? - client = OpenAI::Client.new(access_token: openai_api_key) - - begin - response = client.models.list - errors.add(:base, :invalid_api_key) unless response['data'] - rescue Faraday::UnauthorizedError, OpenAI::Error - errors.add(:base, :invalid_api_key) - end + TaskService::GptGenerateTests.new_client! openai_api_key + rescue Gpt::InvalidApiKeyError + errors.add(:base, :invalid_api_key) end - private - def avatar_format avatar_blob = avatar.blob if avatar_blob.content_type.start_with? 'image/' diff --git a/app/services/task_service/gpt_generate_tests.rb b/app/services/task_service/gpt_generate_tests.rb index d1c1fc9db..77e597a21 100644 --- a/app/services/task_service/gpt_generate_tests.rb +++ b/app/services/task_service/gpt_generate_tests.rb @@ -7,8 +7,7 @@ def initialize(task:, openai_api_key:) raise Gpt::MissingLanguageError if task.programming_language&.language.blank? @task = task - @openai_api_key = openai_api_key - validate_api_key! + @client = self.class.new_client! openai_api_key end def execute @@ -20,12 +19,16 @@ def execute @task.tests << test end - private + def self.new_client!(access_token) + raise Gpt::InvalidApiKeyError if access_token.blank? - def client - @client ||= OpenAI::Client.new(access_token: @openai_api_key) + client = OpenAI::Client.new(access_token:) + validate! client + client end + private + def gpt_response # train client with some prompts messages = training_prompts.map do |prompt| @@ -36,7 +39,7 @@ def gpt_response messages << {role: 'user', content: @task.description} # create gpt client - response = client.chat( + response = @client.chat( parameters: { model: Settings.open_ai.model, messages:, @@ -67,17 +70,12 @@ def training_prompts ] end - def validate_api_key! - if @openai_api_key.blank? - raise Gpt::InvalidApiKeyError - else - begin - response = client.models.list - raise Gpt::InvalidApiKeyError unless response['data'] - rescue Faraday::UnauthorizedError, OpenAI::Error - raise Gpt::InvalidApiKeyError - end - end + def self.validate!(client) + response = client.models.list + raise Gpt::InvalidApiKeyError unless response['data'] + rescue Faraday::UnauthorizedError, OpenAI::Error + raise Gpt::InvalidApiKeyError end + private_class_method :validate! end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index e7888c237..4f8c4dd7f 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -52,49 +52,54 @@ end context 'when openai_api_key is present and valid' do + let(:openai_client) { instance_double(OpenAI::Client) } + let(:openai_api_key) { 'valid_key' } + before do - allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models)) - user.openai_api_key = 'valid_key' + allow(TaskService::GptGenerateTests).to receive(:new_client!).with(openai_api_key).and_return(openai_client) + user.update(openai_api_key:) end - let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } - it 'is valid' do expect(user).to be_valid end end context 'when openai_api_key is present and invalid' do + let(:openai_client) { instance_double(OpenAI::Client) } + let(:openai_api_key) { 'invalid_key' } + before do - allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models)) - allow(mock_models).to receive(:list).and_raise(OpenAI::Error) - user.openai_api_key = 'invalid_key' - user.valid? + allow(TaskService::GptGenerateTests).to receive(:new_client!).with(openai_api_key).and_raise(Gpt::InvalidApiKeyError) + user.update(openai_api_key:) end - let(:mock_models) { instance_double(OpenAI::Models) } - it 'is not valid' do expect(user).not_to be_valid end it 'adds an error for invalid api key' do + user.valid? expect(user.errors[:base]).to include(I18n.t('activerecord.errors.models.user.invalid_api_key')) end end context 'when openai_api_key remains the same' do + let(:openai_client) { instance_double(OpenAI::Client) } + let(:openai_api_key) { 'same_key' } + before do - allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models)) - user.openai_api_key = 'same_key' - user.save! - user.valid? + allow(TaskService::GptGenerateTests).to receive(:new_client!).with(openai_api_key).and_return(openai_client) + user.update(openai_api_key:) end - let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } - it 'does not trigger API validation' do - expect(user.errors[:base]).not_to include(I18n.t('activerecord.errors.models.user.invalid_api_key')) + expect(TaskService::GptGenerateTests).not_to receive(:new_client!) + expect { user.update(openai_api_key:) }.not_to change(user, :openai_api_key) + end + + it 'is valid' do + expect(user).to be_valid end end end diff --git a/spec/services/task_service/gpt_generate_tests_spec.rb b/spec/services/task_service/gpt_generate_tests_spec.rb index 3a33fd01c..c2187419f 100644 --- a/spec/services/task_service/gpt_generate_tests_spec.rb +++ b/spec/services/task_service/gpt_generate_tests_spec.rb @@ -11,17 +11,23 @@ let(:task) { build(:task, description: 'Sample Task', programming_language:) } let(:openai_api_key) { 'valid_api_key' } let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } + let(:openai_client) { OpenAI::Client.new(access_token: openai_api_key) } before do - allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models)) + allow(OpenAI::Client).to receive(:new).and_return(openai_client) + allow(openai_client).to receive(:models).and_return(mock_models) end - it 'assigns task' do + it 'assigns the task' do expect(gpt_generate_tests_service.instance_variable_get(:@task)).to be task end - it 'assigns openai_api_key' do - expect(gpt_generate_tests_service.instance_variable_get(:@openai_api_key)).to eq openai_api_key + it 'assigns the client for OpenAI' do + expect(gpt_generate_tests_service.instance_variable_get(:@client)).to be openai_client + end + + it 'stores the OpenAI API key in the client' do + expect(gpt_generate_tests_service.instance_variable_get(:@client).access_token).to eq openai_api_key end context 'when language is missing' do @@ -42,11 +48,10 @@ context 'when API key is invalid' do let(:openai_api_key) { 'invalid_api_key' } - let(:mock_models_invalid) { instance_double(OpenAI::Models) } + let(:mock_models) { instance_double(OpenAI::Models) } before do - allow(mock_models_invalid).to receive(:list).and_raise(Faraday::UnauthorizedError) - allow(OpenAI::Client).to receive(:new).and_return(instance_double(OpenAI::Client, models: mock_models_invalid)) + allow(mock_models).to receive(:list).and_raise(Faraday::UnauthorizedError) end it 'raises InvalidApiKeyError' do From 78541bea03e6e4e5bc7fff3a9a4c6f679b6d8d44 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Fri, 12 Jul 2024 10:06:22 +0200 Subject: [PATCH 04/11] Simplify specs for TaskService::GptGenerateTests --- .../task_service/gpt_generate_tests_spec.rb | 42 ++++++++----------- 1 file changed, 17 insertions(+), 25 deletions(-) diff --git a/spec/services/task_service/gpt_generate_tests_spec.rb b/spec/services/task_service/gpt_generate_tests_spec.rb index c2187419f..0f6b622a3 100644 --- a/spec/services/task_service/gpt_generate_tests_spec.rb +++ b/spec/services/task_service/gpt_generate_tests_spec.rb @@ -4,19 +4,20 @@ require 'rails_helper' RSpec.describe TaskService::GptGenerateTests do - describe '.new' do - subject(:gpt_generate_tests_service) { described_class.new(task:, openai_api_key:) } + let(:openai_api_key) { 'valid_api_key' } + let(:openai_client) { OpenAI::Client.new(access_token: openai_api_key) } + let(:openai_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } - let(:programming_language) { build(:programming_language, :ruby) } - let(:task) { build(:task, description: 'Sample Task', programming_language:) } - let(:openai_api_key) { 'valid_api_key' } - let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } - let(:openai_client) { OpenAI::Client.new(access_token: openai_api_key) } + let(:programming_language) { create(:programming_language, :python) } + let(:task) { create(:task, description: 'Create a Python script.', programming_language:) } - before do - allow(OpenAI::Client).to receive(:new).and_return(openai_client) - allow(openai_client).to receive(:models).and_return(mock_models) - end + before do + allow(OpenAI::Client).to receive(:new).and_return(openai_client) + allow(openai_client).to receive(:models).and_return(openai_models) + end + + describe '.new' do + subject(:gpt_generate_tests_service) { described_class.new(task:, openai_api_key:) } it 'assigns the task' do expect(gpt_generate_tests_service.instance_variable_get(:@task)).to be task @@ -31,7 +32,7 @@ end context 'when language is missing' do - let(:task) { build(:task, description: 'Sample Task', programming_language: nil) } + let(:programming_language) { nil } it 'raises MissingLanguageError' do expect { gpt_generate_tests_service }.to raise_error(Gpt::MissingLanguageError) @@ -48,10 +49,9 @@ context 'when API key is invalid' do let(:openai_api_key) { 'invalid_api_key' } - let(:mock_models) { instance_double(OpenAI::Models) } before do - allow(mock_models).to receive(:list).and_raise(Faraday::UnauthorizedError) + allow(openai_models).to receive(:list).and_raise(Faraday::UnauthorizedError) end it 'raises InvalidApiKeyError' do @@ -63,20 +63,14 @@ describe '#call' do subject(:gpt_generate_tests) { described_class.call(task:, openai_api_key:) } - let(:programming_language) { create(:programming_language, :python) } - let(:task) { create(:task, description: 'Create a Python script.', programming_language:) } - let(:openai_api_key) { 'valid_api_key' } - let(:mock_client) { instance_double(OpenAI::Client) } - let(:mock_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'text-davinci-002'}]}) } + let(:chat_response) { {'choices' => [{'message' => {'content' => "```Python\ndef test_script():\n assert true```"}}]} } before do - allow(OpenAI::Client).to receive(:new).and_return(mock_client) - allow(mock_client).to receive(:models).and_return(mock_models) + allow(openai_client).to receive(:chat).and_return(chat_response) end context 'when the response includes valid code blocks' do before do - allow(mock_client).to receive(:chat).and_return('choices' => [{'message' => {'content' => "```Python\ndef test_script():\n assert true```"}}]) gpt_generate_tests end @@ -95,9 +89,7 @@ end context 'when the response does not contain backticks' do - before do - allow(mock_client).to receive(:chat).and_return({'choices' => [{'message' => {'content' => 'Python script should assert true without any code block.'}}]}) - end + let(:chat_response) { {'choices' => [{'message' => {'content' => 'Python script should assert true without any code block.'}}]} } it 'raises InvalidTaskDescription' do expect { gpt_generate_tests }.to raise_error(Gpt::InvalidTaskDescription) From 3609649b23fc0677910bf77a1a6b7b025faa68c1 Mon Sep 17 00:00:00 2001 From: Melhaya Date: Fri, 12 Jul 2024 12:40:59 +0200 Subject: [PATCH 05/11] Better handling of OpenAI server down error --- app/controllers/tasks_controller.rb | 17 +++++++++++------ app/errors/gpt/openai_server_down_error.rb | 5 +++++ app/services/task_service/gpt_generate_tests.rb | 2 ++ config/locales/de/controllers/tasks.yml | 1 + config/locales/en/controllers/tasks.yml | 1 + 5 files changed, 20 insertions(+), 6 deletions(-) create mode 100644 app/errors/gpt/openai_server_down_error.rb diff --git a/app/controllers/tasks_controller.rb b/app/controllers/tasks_controller.rb index d58187773..244e221fa 100644 --- a/app/controllers/tasks_controller.rb +++ b/app/controllers/tasks_controller.rb @@ -189,18 +189,23 @@ def export_external_confirm def generate_test TaskService::GptGenerateTests.call(task: @task, openai_api_key: current_user.openai_api_key) flash[:notice] = I18n.t('tasks.task_service.gpt_generate_tests.successful_generation') - rescue Gpt::InvalidApiKeyError - flash[:alert] = I18n.t('tasks.task_service.gpt_generate_tests.invalid_api_key') - rescue Gpt::MissingLanguageError - flash[:alert] = I18n.t('tasks.task_service.gpt_generate_tests.no_language') - rescue Gpt::InvalidTaskDescription - flash[:alert] = I18n.t('tasks.task_service.gpt_generate_tests.invalid_description') + rescue Gpt::InvalidApiKeyError, Gpt::MissingLanguageError, Gpt::InvalidTaskDescription, Gpt::OpenaiServerDownError => e + flash[:alert] = flash_message_for(e) ensure redirect_to @task end private + def flash_message_for(exception) + { + Gpt::InvalidApiKeyError => I18n.t('tasks.task_service.gpt_generate_tests.invalid_api_key'), + Gpt::MissingLanguageError => I18n.t('tasks.task_service.gpt_generate_tests.no_language'), + Gpt::InvalidTaskDescription => I18n.t('tasks.task_service.gpt_generate_tests.invalid_description'), + Gpt::OpenaiServerDownError => I18n.t('tasks.task_service.gpt_generate_tests.openai_server_down'), + }[exception.class] + end + def load_and_authorize_task @task = Task.find(params[:id]) authorize @task diff --git a/app/errors/gpt/openai_server_down_error.rb b/app/errors/gpt/openai_server_down_error.rb new file mode 100644 index 000000000..0e206285d --- /dev/null +++ b/app/errors/gpt/openai_server_down_error.rb @@ -0,0 +1,5 @@ +# frozen_string_literal: true + +module Gpt + class OpenaiServerDownError < StandardError; end +end diff --git a/app/services/task_service/gpt_generate_tests.rb b/app/services/task_service/gpt_generate_tests.rb index 77e597a21..7a07625b8 100644 --- a/app/services/task_service/gpt_generate_tests.rb +++ b/app/services/task_service/gpt_generate_tests.rb @@ -75,6 +75,8 @@ def self.validate!(client) raise Gpt::InvalidApiKeyError unless response['data'] rescue Faraday::UnauthorizedError, OpenAI::Error raise Gpt::InvalidApiKeyError + rescue Faraday::ServerError + raise Gpt::OpenaiServerDownError end private_class_method :validate! end diff --git a/config/locales/de/controllers/tasks.yml b/config/locales/de/controllers/tasks.yml index 05e0f0519..16f3ae7e5 100644 --- a/config/locales/de/controllers/tasks.yml +++ b/config/locales/de/controllers/tasks.yml @@ -30,4 +30,5 @@ de: invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um die KI-Funktionen zu nutzen. invalid_description: Die angegebene Aufgabenbeschreibung stellt keine gültige Programmieraufgabe dar und kann daher nicht zum Generieren eines Unit-Tests genutzt werden. Bitte stellen Sie sicher, dass die Aufgabenbeschreibung eine klar formulierte Problemstellung enthält, die durch ein Programm gelöst werden kann. no_language: Für diese Aufgabe ist keine Programmiersprache angegeben. Bitte geben Sie die Sprache an, bevor Sie fortfahren. + openai_server_down: Der OpenAI-Server ist derzeit nicht verfügbar. Bitte versuchen Sie es später erneut, wenn das Problem behoben ist. successful_generation: Unit-Test erfolgreich generiert. Bitte überprüfen Sie den generierten Test und vergeben Sie einen passenden Dateinamen. diff --git a/config/locales/en/controllers/tasks.yml b/config/locales/en/controllers/tasks.yml index 0de2004a2..ac84e860e 100644 --- a/config/locales/en/controllers/tasks.yml +++ b/config/locales/en/controllers/tasks.yml @@ -30,4 +30,5 @@ en: invalid_api_key: The API key in your profile is invalid. Please add the appropriate API key in your profile to use the AI features. invalid_description: The task description provided does not represent a valid programming task and therefore cannot be used to generate a unit test. Please make sure that the task description contains a clearly formulated problem that can be solved by a program. no_language: Programming language is not specified for this task. Please specify the language before proceeding. + openai_server_down: The OpenAI server is currently experiencing an outage. Please try again later when the issue is fixed. successful_generation: Unit test generated successfully. Please check the generated test and assign an appropriate filename. From 46268bece9b7f6e9ca732a68f8d9a30f4cbfdd32 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Fri, 12 Jul 2024 17:04:18 +0200 Subject: [PATCH 06/11] Refactor Gpt::Errors - Introduce a common base class - Move error descriptions to dedicated locale - Catch more network-related errors --- app/controllers/tasks_controller.rb | 13 +---- app/errors/gpt/error.rb | 17 +++++++ app/errors/gpt/invalid_api_key_error.rb | 5 -- app/errors/gpt/invalid_task_description.rb | 5 -- app/errors/gpt/missing_language_error.rb | 5 -- app/errors/gpt/openai_server_down_error.rb | 5 -- app/models/user.rb | 2 +- .../task_service/gpt_generate_tests.rb | 22 +++++---- config/locales/de/controllers/tasks.yml | 4 -- config/locales/de/errors.yml | 9 ++++ config/locales/en/controllers/tasks.yml | 4 -- config/locales/en/errors.yml | 9 ++++ spec/controllers/tasks_controller_spec.rb | 12 ++--- spec/models/user_spec.rb | 2 +- .../task_service/gpt_generate_tests_spec.rb | 48 +++++++++++++++++-- 15 files changed, 103 insertions(+), 59 deletions(-) create mode 100644 app/errors/gpt/error.rb delete mode 100644 app/errors/gpt/invalid_api_key_error.rb delete mode 100644 app/errors/gpt/invalid_task_description.rb delete mode 100644 app/errors/gpt/missing_language_error.rb delete mode 100644 app/errors/gpt/openai_server_down_error.rb create mode 100644 config/locales/de/errors.yml create mode 100644 config/locales/en/errors.yml diff --git a/app/controllers/tasks_controller.rb b/app/controllers/tasks_controller.rb index 244e221fa..04b8f1058 100644 --- a/app/controllers/tasks_controller.rb +++ b/app/controllers/tasks_controller.rb @@ -189,23 +189,14 @@ def export_external_confirm def generate_test TaskService::GptGenerateTests.call(task: @task, openai_api_key: current_user.openai_api_key) flash[:notice] = I18n.t('tasks.task_service.gpt_generate_tests.successful_generation') - rescue Gpt::InvalidApiKeyError, Gpt::MissingLanguageError, Gpt::InvalidTaskDescription, Gpt::OpenaiServerDownError => e - flash[:alert] = flash_message_for(e) + rescue Gpt::Error => e + flash[:alert] = e.localized_message ensure redirect_to @task end private - def flash_message_for(exception) - { - Gpt::InvalidApiKeyError => I18n.t('tasks.task_service.gpt_generate_tests.invalid_api_key'), - Gpt::MissingLanguageError => I18n.t('tasks.task_service.gpt_generate_tests.no_language'), - Gpt::InvalidTaskDescription => I18n.t('tasks.task_service.gpt_generate_tests.invalid_description'), - Gpt::OpenaiServerDownError => I18n.t('tasks.task_service.gpt_generate_tests.openai_server_down'), - }[exception.class] - end - def load_and_authorize_task @task = Task.find(params[:id]) authorize @task diff --git a/app/errors/gpt/error.rb b/app/errors/gpt/error.rb new file mode 100644 index 000000000..0b2fcd16a --- /dev/null +++ b/app/errors/gpt/error.rb @@ -0,0 +1,17 @@ +# frozen_string_literal: true + +module Gpt + class Error < ApplicationError + class InternalServerError < Error; end + + class InvalidApiKey < Error; end + + class InvalidTaskDescription < Error; end + + class MissingLanguage < Error; end + + def localized_message + I18n.t("errors.gpt.#{self.class.name&.demodulize&.underscore}") + end + end +end diff --git a/app/errors/gpt/invalid_api_key_error.rb b/app/errors/gpt/invalid_api_key_error.rb deleted file mode 100644 index 9f281d605..000000000 --- a/app/errors/gpt/invalid_api_key_error.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -module Gpt - class InvalidApiKeyError < StandardError; end -end diff --git a/app/errors/gpt/invalid_task_description.rb b/app/errors/gpt/invalid_task_description.rb deleted file mode 100644 index 458851869..000000000 --- a/app/errors/gpt/invalid_task_description.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -module Gpt - class InvalidTaskDescription < StandardError; end -end diff --git a/app/errors/gpt/missing_language_error.rb b/app/errors/gpt/missing_language_error.rb deleted file mode 100644 index 57d9f6a7b..000000000 --- a/app/errors/gpt/missing_language_error.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -module Gpt - class MissingLanguageError < StandardError; end -end diff --git a/app/errors/gpt/openai_server_down_error.rb b/app/errors/gpt/openai_server_down_error.rb deleted file mode 100644 index 0e206285d..000000000 --- a/app/errors/gpt/openai_server_down_error.rb +++ /dev/null @@ -1,5 +0,0 @@ -# frozen_string_literal: true - -module Gpt - class OpenaiServerDownError < StandardError; end -end diff --git a/app/models/user.rb b/app/models/user.rb index a14861d9c..799ae3774 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -153,7 +153,7 @@ def validate_openai_api_key return unless openai_api_key_changed? TaskService::GptGenerateTests.new_client! openai_api_key - rescue Gpt::InvalidApiKeyError + rescue Gpt::Error::InvalidApiKey errors.add(:base, :invalid_api_key) end diff --git a/app/services/task_service/gpt_generate_tests.rb b/app/services/task_service/gpt_generate_tests.rb index 7a07625b8..55c499d91 100644 --- a/app/services/task_service/gpt_generate_tests.rb +++ b/app/services/task_service/gpt_generate_tests.rb @@ -4,7 +4,7 @@ module TaskService class GptGenerateTests < ServiceBase def initialize(task:, openai_api_key:) super() - raise Gpt::MissingLanguageError if task.programming_language&.language.blank? + raise Gpt::Error::MissingLanguage if task.programming_language&.language.blank? @task = task @client = self.class.new_client! openai_api_key @@ -20,7 +20,7 @@ def execute end def self.new_client!(access_token) - raise Gpt::InvalidApiKeyError if access_token.blank? + raise Gpt::Error::InvalidApiKey if access_token.blank? client = OpenAI::Client.new(access_token:) validate! client @@ -29,7 +29,7 @@ def self.new_client!(access_token) private - def gpt_response + def gpt_response # rubocop:disable Metrics/AbcSize # train client with some prompts messages = training_prompts.map do |prompt| {role: 'system', content: prompt} @@ -51,9 +51,13 @@ def gpt_response raw_response = response.dig('choices', 0, 'message', 'content') # check for ``` in the response and extract the text between the first set - raise Gpt::InvalidTaskDescription unless raw_response.include?('```') + raise Gpt::Error::InvalidTaskDescription unless raw_response.include?('```') raw_response[/```(.*?)```/m, 1].lines[1..]&.join&.strip + rescue Faraday::Error => e + raise Gpt::Error::InternalServerError.new("Could not communicate with OpenAI due to #{e.inspect}") + rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError + raise Gpt::Error.new(e) end def training_prompts @@ -72,11 +76,13 @@ def training_prompts def self.validate!(client) response = client.models.list - raise Gpt::InvalidApiKeyError unless response['data'] + raise Gpt::Error::InvalidApiKey unless response['data'] rescue Faraday::UnauthorizedError, OpenAI::Error - raise Gpt::InvalidApiKeyError - rescue Faraday::ServerError - raise Gpt::OpenaiServerDownError + raise Gpt::Error::InvalidApiKey + rescue Faraday::Error => e + raise Gpt::Error::InternalServerError.new("Could not communicate with OpenAI due to #{e.inspect}") + rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError + raise Gpt::Error end private_class_method :validate! end diff --git a/config/locales/de/controllers/tasks.yml b/config/locales/de/controllers/tasks.yml index 16f3ae7e5..fe59423f0 100644 --- a/config/locales/de/controllers/tasks.yml +++ b/config/locales/de/controllers/tasks.yml @@ -27,8 +27,4 @@ de: task_found: 'In der externen App wurde eine entsprechende Aufgabe gefunden. Sie können:
  • Überschreiben Sie die Aufgabe in der externen App. Dadurch werden alle Änderungen, die in CodeHarbor vorgenommen wurden, auf die externe App übertragen.
    Vorsicht: Dadurch werden alle potenziellen Änderungen in der externen App überschrieben. Dadurch wird die Aufgabe geändert (und möglicherweise zerstört), auch wenn sie derzeit in einem Kurs verwendet wird.
  • Erstellen Sie eine neue Aufgabe, die den aktuellen Zustand dieser Aufgabe kopiert. Dadurch wird eine Kopie dieser Aufgabe in CodeHarbor erstellt, die dann als völlig neue Aufgabe in der externen App exportiert wird.
' task_found_no_right: 'In der externen App wurde eine entsprechende Aufgabe gefunden, aber Sie haben keine Rechte, sie zu bearbeiten. Sie können:
  • Eine neue Aufgabe erstellen, die den aktuellen Zustand dieser Aufgabe kopiert. Dadurch wird eine Kopie dieser Aufgabe in CodeHarbor erstellt, die dann als völlig neue Aufgabe in der externen App exportiert wird.
' gpt_generate_tests: - invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um die KI-Funktionen zu nutzen. - invalid_description: Die angegebene Aufgabenbeschreibung stellt keine gültige Programmieraufgabe dar und kann daher nicht zum Generieren eines Unit-Tests genutzt werden. Bitte stellen Sie sicher, dass die Aufgabenbeschreibung eine klar formulierte Problemstellung enthält, die durch ein Programm gelöst werden kann. - no_language: Für diese Aufgabe ist keine Programmiersprache angegeben. Bitte geben Sie die Sprache an, bevor Sie fortfahren. - openai_server_down: Der OpenAI-Server ist derzeit nicht verfügbar. Bitte versuchen Sie es später erneut, wenn das Problem behoben ist. successful_generation: Unit-Test erfolgreich generiert. Bitte überprüfen Sie den generierten Test und vergeben Sie einen passenden Dateinamen. diff --git a/config/locales/de/errors.yml b/config/locales/de/errors.yml new file mode 100644 index 000000000..64ef0c9fc --- /dev/null +++ b/config/locales/de/errors.yml @@ -0,0 +1,9 @@ +--- +de: + errors: + gpt: + error: Beim Generieren des Unit-Tests ist ein Fehler aufgetreten. Bitte versuchen Sie es später erneut. + internal_server_error: Der OpenAI-Server ist derzeit nicht verfügbar. Bitte versuchen Sie es später erneut, wenn das Problem behoben wurde. + invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um die KI-Funktionen zu nutzen. + invalid_task_description: Die angegebene Aufgabenbeschreibung stellt keine gültige Programmieraufgabe dar und kann daher nicht zum Generieren eines Unit-Tests genutzt werden. Bitte stellen Sie sicher, dass die Aufgabenbeschreibung eine klar formulierte Problemstellung enthält, die durch ein Programm gelöst werden kann. + missing_language: Für diese Aufgabe ist keine Programmiersprache angegeben. Bitte geben Sie die Sprache an, bevor Sie fortfahren. diff --git a/config/locales/en/controllers/tasks.yml b/config/locales/en/controllers/tasks.yml index ac84e860e..1668cf6fd 100644 --- a/config/locales/en/controllers/tasks.yml +++ b/config/locales/en/controllers/tasks.yml @@ -27,8 +27,4 @@ en: task_found: 'A corresponding task has been found on the external app. You can:
  • Overwrite the task on the external app. This will transfer all changes made on CodeHarbor to the external app.
    Careful: This will overwrite all potential changes made on the external app. This will change (and might break) the task, even if it is currently in use by a course.
  • Create a new task which copies the current state of this task. This will create a copy of this task on CodeHarbor, which will then be exported as a completely new exercise to the external app.
' task_found_no_right: 'A corresponding task has been found on external app, but you don''t have the rights to edit it. You can:
  • Create a new task which copies the current state of this task. This will create a copy of this task on CodeHarbor, which will then be exported as a completely new task to the external app.
' gpt_generate_tests: - invalid_api_key: The API key in your profile is invalid. Please add the appropriate API key in your profile to use the AI features. - invalid_description: The task description provided does not represent a valid programming task and therefore cannot be used to generate a unit test. Please make sure that the task description contains a clearly formulated problem that can be solved by a program. - no_language: Programming language is not specified for this task. Please specify the language before proceeding. - openai_server_down: The OpenAI server is currently experiencing an outage. Please try again later when the issue is fixed. successful_generation: Unit test generated successfully. Please check the generated test and assign an appropriate filename. diff --git a/config/locales/en/errors.yml b/config/locales/en/errors.yml new file mode 100644 index 000000000..b57d00916 --- /dev/null +++ b/config/locales/en/errors.yml @@ -0,0 +1,9 @@ +--- +en: + errors: + gpt: + error: An error occurred while generating the unit test. Please try again later. + internal_server_error: The OpenAI server is currently experiencing an outage. Please try again later when the issue is fixed. + invalid_api_key: The API key in your profile is invalid. Please add the appropriate API key in your profile to use the AI features. + invalid_task_description: The task description provided does not represent a valid programming task and therefore cannot be used to generate a unit test. Please make sure that the task description contains a clearly formulated problem that can be solved by a program. + missing_language: Programming language is not specified for this task. Please specify the language before proceeding. diff --git a/spec/controllers/tasks_controller_spec.rb b/spec/controllers/tasks_controller_spec.rb index e061fc506..f254c7f0b 100644 --- a/spec/controllers/tasks_controller_spec.rb +++ b/spec/controllers/tasks_controller_spec.rb @@ -1072,9 +1072,9 @@ end end - context 'when GptGenerateTests raises MissingLanguageError' do + context 'when GptGenerateTests raises Gpt::Error::MissingLanguage' do before do - allow(TaskService::GptGenerateTests).to receive(:call).and_raise(Gpt::MissingLanguageError) + allow(TaskService::GptGenerateTests).to receive(:call).and_raise(Gpt::Error::MissingLanguage) post :generate_test, params: {id: task.id} end @@ -1083,13 +1083,13 @@ end it 'sets flash to the appropriate message' do - expect(flash[:alert]).to eq(I18n.t('tasks.task_service.gpt_generate_tests.no_language')) + expect(flash[:alert]).to eq(I18n.t('errors.gpt.missing_language')) end end - context 'when GptGenerateTests raises InvalidTaskDescription' do + context 'when GptGenerateTests raises Gpt::Error::InvalidTaskDescription' do before do - allow(TaskService::GptGenerateTests).to receive(:call).and_raise(Gpt::InvalidTaskDescription) + allow(TaskService::GptGenerateTests).to receive(:call).and_raise(Gpt::Error::InvalidTaskDescription) post :generate_test, params: {id: task.id} end @@ -1098,7 +1098,7 @@ end it 'sets flash to the appropriate message' do - expect(flash[:alert]).to eq(I18n.t('tasks.task_service.gpt_generate_tests.invalid_description')) + expect(flash[:alert]).to eq(I18n.t('errors.gpt.invalid_task_description')) end end end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index 4f8c4dd7f..fc7e012d7 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -70,7 +70,7 @@ let(:openai_api_key) { 'invalid_key' } before do - allow(TaskService::GptGenerateTests).to receive(:new_client!).with(openai_api_key).and_raise(Gpt::InvalidApiKeyError) + allow(TaskService::GptGenerateTests).to receive(:new_client!).with(openai_api_key).and_raise(Gpt::Error::InvalidApiKey) user.update(openai_api_key:) end diff --git a/spec/services/task_service/gpt_generate_tests_spec.rb b/spec/services/task_service/gpt_generate_tests_spec.rb index 0f6b622a3..46d0dfb80 100644 --- a/spec/services/task_service/gpt_generate_tests_spec.rb +++ b/spec/services/task_service/gpt_generate_tests_spec.rb @@ -35,7 +35,7 @@ let(:programming_language) { nil } it 'raises MissingLanguageError' do - expect { gpt_generate_tests_service }.to raise_error(Gpt::MissingLanguageError) + expect { gpt_generate_tests_service }.to raise_error(Gpt::Error::MissingLanguage) end end @@ -43,7 +43,7 @@ let(:openai_api_key) { nil } it 'raises InvalidApiKeyError' do - expect { gpt_generate_tests_service }.to raise_error(Gpt::InvalidApiKeyError) + expect { gpt_generate_tests_service }.to raise_error(Gpt::Error::InvalidApiKey) end end @@ -55,7 +55,27 @@ end it 'raises InvalidApiKeyError' do - expect { gpt_generate_tests_service }.to raise_error(Gpt::InvalidApiKeyError) + expect { gpt_generate_tests_service }.to raise_error(Gpt::Error::InvalidApiKey) + end + end + + context 'when OpenAI is not responding' do + before do + allow(openai_models).to receive(:list).and_raise(Faraday::Error) + end + + it 'raises InternalServerError' do + expect { gpt_generate_tests_service }.to raise_error(Gpt::Error::InternalServerError) + end + end + + context 'when the network connection is broken' do + before do + allow(openai_models).to receive(:list).and_raise(EOFError) + end + + it 'raises an error' do + expect { gpt_generate_tests_service }.to raise_error(Gpt::Error) end end end @@ -92,7 +112,27 @@ let(:chat_response) { {'choices' => [{'message' => {'content' => 'Python script should assert true without any code block.'}}]} } it 'raises InvalidTaskDescription' do - expect { gpt_generate_tests }.to raise_error(Gpt::InvalidTaskDescription) + expect { gpt_generate_tests }.to raise_error(Gpt::Error::InvalidTaskDescription) + end + end + + context 'when OpenAI is not responding' do + before do + allow(openai_client).to receive(:chat).and_raise(Faraday::Error) + end + + it 'raises InternalServerError' do + expect { gpt_generate_tests }.to raise_error(Gpt::Error::InternalServerError) + end + end + + context 'when the network connection is broken' do + before do + allow(openai_client).to receive(:chat).and_raise(EOFError) + end + + it 'raises an error' do + expect { gpt_generate_tests }.to raise_error(Gpt::Error) end end end From 8d381a8c23597f876af81a53e674fac324194a59 Mon Sep 17 00:00:00 2001 From: Melhaya Date: Tue, 30 Jul 2024 22:14:17 +0200 Subject: [PATCH 07/11] Add UnauthorizedError for better error handling --- app/errors/gpt/error.rb | 2 ++ app/services/task_service/gpt_generate_tests.rb | 4 +++- 2 files changed, 5 insertions(+), 1 deletion(-) diff --git a/app/errors/gpt/error.rb b/app/errors/gpt/error.rb index 0b2fcd16a..e57ffb524 100644 --- a/app/errors/gpt/error.rb +++ b/app/errors/gpt/error.rb @@ -10,6 +10,8 @@ class InvalidTaskDescription < Error; end class MissingLanguage < Error; end + class UnauthorizedError < Error; end + def localized_message I18n.t("errors.gpt.#{self.class.name&.demodulize&.underscore}") end diff --git a/app/services/task_service/gpt_generate_tests.rb b/app/services/task_service/gpt_generate_tests.rb index 55c499d91..63bf86eeb 100644 --- a/app/services/task_service/gpt_generate_tests.rb +++ b/app/services/task_service/gpt_generate_tests.rb @@ -54,9 +54,11 @@ def gpt_response # rubocop:disable Metrics/AbcSize raise Gpt::Error::InvalidTaskDescription unless raw_response.include?('```') raw_response[/```(.*?)```/m, 1].lines[1..]&.join&.strip + rescue Faraday::UnauthorizedError => e + raise Gpt::Error::UnauthorizedError.new("Unauthorized access to OpenAI: #{e.message}") rescue Faraday::Error => e raise Gpt::Error::InternalServerError.new("Could not communicate with OpenAI due to #{e.inspect}") - rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError + rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError => e raise Gpt::Error.new(e) end From 77365ebd3be8d78ec33704b063dbb9cede4b96d5 Mon Sep 17 00:00:00 2001 From: Melhaya Date: Wed, 31 Jul 2024 10:38:23 +0200 Subject: [PATCH 08/11] Add error translation and test for their presence --- config/locales/de/errors.yml | 1 + config/locales/en/errors.yml | 1 + spec/services/task_service/gpt_generate_tests_spec.rb | 11 +++++++++++ 3 files changed, 13 insertions(+) diff --git a/config/locales/de/errors.yml b/config/locales/de/errors.yml index 64ef0c9fc..8b0a28e41 100644 --- a/config/locales/de/errors.yml +++ b/config/locales/de/errors.yml @@ -7,3 +7,4 @@ de: invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um die KI-Funktionen zu nutzen. invalid_task_description: Die angegebene Aufgabenbeschreibung stellt keine gültige Programmieraufgabe dar und kann daher nicht zum Generieren eines Unit-Tests genutzt werden. Bitte stellen Sie sicher, dass die Aufgabenbeschreibung eine klar formulierte Problemstellung enthält, die durch ein Programm gelöst werden kann. missing_language: Für diese Aufgabe ist keine Programmiersprache angegeben. Bitte geben Sie die Sprache an, bevor Sie fortfahren. + unauthorized_error: Unbefugter Zugriff auf OpenAI erkannt. Dies geschieht normalerweise, wenn der API-Schlüssel falsch oder abgelaufen ist. Bitte überprüfen Sie Ihren API-Schlüssel und versuchen Sie es erneut. diff --git a/config/locales/en/errors.yml b/config/locales/en/errors.yml index b57d00916..604cd8b10 100644 --- a/config/locales/en/errors.yml +++ b/config/locales/en/errors.yml @@ -7,3 +7,4 @@ en: invalid_api_key: The API key in your profile is invalid. Please add the appropriate API key in your profile to use the AI features. invalid_task_description: The task description provided does not represent a valid programming task and therefore cannot be used to generate a unit test. Please make sure that the task description contains a clearly formulated problem that can be solved by a program. missing_language: Programming language is not specified for this task. Please specify the language before proceeding. + unauthorized_error: Unauthorized access to OpenAI detected. This usually happens when the API key is incorrect or expired. Please check your API key and try again. diff --git a/spec/services/task_service/gpt_generate_tests_spec.rb b/spec/services/task_service/gpt_generate_tests_spec.rb index 46d0dfb80..2f9ddadef 100644 --- a/spec/services/task_service/gpt_generate_tests_spec.rb +++ b/spec/services/task_service/gpt_generate_tests_spec.rb @@ -136,4 +136,15 @@ end end end + + describe 'localized messages for all error classes' do + it 'has localized messages for all error classes' do + error_classes = Gpt::Error.descendants + sample_errors = error_classes.map(&:new) + + sample_errors.each do |error| + expect { error.localized_message }.not_to raise_error, "Missing translation for #{error.class.name}" + end + end + end end From 12318734c8506616dfc0d37f1e20655ec8cedf46 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 31 Jul 2024 14:56:40 +0200 Subject: [PATCH 09/11] Refactor error handling for Gpt service --- app/errors/gpt/error.rb | 2 - .../task_service/gpt_generate_tests.rb | 84 ++++++++++--------- config/locales/de/errors.yml | 3 +- config/locales/en/errors.yml | 3 +- spec/errors/gpt/error_spec.rb | 11 +++ .../task_service/gpt_generate_tests_spec.rb | 12 --- 6 files changed, 59 insertions(+), 56 deletions(-) create mode 100644 spec/errors/gpt/error_spec.rb diff --git a/app/errors/gpt/error.rb b/app/errors/gpt/error.rb index e57ffb524..0b2fcd16a 100644 --- a/app/errors/gpt/error.rb +++ b/app/errors/gpt/error.rb @@ -10,8 +10,6 @@ class InvalidTaskDescription < Error; end class MissingLanguage < Error; end - class UnauthorizedError < Error; end - def localized_message I18n.t("errors.gpt.#{self.class.name&.demodulize&.underscore}") end diff --git a/app/services/task_service/gpt_generate_tests.rb b/app/services/task_service/gpt_generate_tests.rb index 63bf86eeb..85b9fdcc7 100644 --- a/app/services/task_service/gpt_generate_tests.rb +++ b/app/services/task_service/gpt_generate_tests.rb @@ -29,37 +29,33 @@ def self.new_client!(access_token) private - def gpt_response # rubocop:disable Metrics/AbcSize - # train client with some prompts - messages = training_prompts.map do |prompt| - {role: 'system', content: prompt} + def gpt_response + wrap_api_error! do + # train client with some prompts + messages = training_prompts.map do |prompt| + {role: 'system', content: prompt} + end + + # send user message + messages << {role: 'user', content: @task.description} + + # create gpt client + response = @client.chat( + parameters: { + model: Settings.open_ai.model, + messages:, + temperature: 0.7, # Lower values insure reproducibility + } + ) + + # parse out the response + raw_response = response.dig('choices', 0, 'message', 'content') + + # check for ``` in the response and extract the text between the first set + raise Gpt::Error::InvalidTaskDescription unless raw_response.include?('```') + + raw_response[/```(.*?)```/m, 1].lines[1..]&.join&.strip end - - # send user message - messages << {role: 'user', content: @task.description} - - # create gpt client - response = @client.chat( - parameters: { - model: Settings.open_ai.model, - messages:, - temperature: 0.7, # Lower values insure reproducibility - } - ) - - # parse out the response - raw_response = response.dig('choices', 0, 'message', 'content') - - # check for ``` in the response and extract the text between the first set - raise Gpt::Error::InvalidTaskDescription unless raw_response.include?('```') - - raw_response[/```(.*?)```/m, 1].lines[1..]&.join&.strip - rescue Faraday::UnauthorizedError => e - raise Gpt::Error::UnauthorizedError.new("Unauthorized access to OpenAI: #{e.message}") - rescue Faraday::Error => e - raise Gpt::Error::InternalServerError.new("Could not communicate with OpenAI due to #{e.inspect}") - rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError => e - raise Gpt::Error.new(e) end def training_prompts @@ -76,16 +72,28 @@ def training_prompts ] end + def wrap_api_error!(...) + # Use a custom forward for the private class method :wrap_api_error! and forward all arguments + self.class.send(:wrap_api_error!, ...) + end + def self.validate!(client) - response = client.models.list - raise Gpt::Error::InvalidApiKey unless response['data'] - rescue Faraday::UnauthorizedError, OpenAI::Error - raise Gpt::Error::InvalidApiKey - rescue Faraday::Error => e - raise Gpt::Error::InternalServerError.new("Could not communicate with OpenAI due to #{e.inspect}") - rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError - raise Gpt::Error + wrap_api_error! do + response = client.models.list + raise Gpt::Error::InvalidApiKey unless response['data'] + end end private_class_method :validate! + + def self.wrap_api_error! + yield + rescue Faraday::UnauthorizedError, OpenAI::Error => e + raise Gpt::Error::InvalidApiKey.new("Could not authenticate with OpenAI: #{e.message}") + rescue Faraday::Error => e + raise Gpt::Error::InternalServerError.new("Could not communicate with OpenAI: #{e.inspect}") + rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError => e + raise Gpt::Error.new(e) + end + private_class_method :wrap_api_error! end end diff --git a/config/locales/de/errors.yml b/config/locales/de/errors.yml index 8b0a28e41..101792849 100644 --- a/config/locales/de/errors.yml +++ b/config/locales/de/errors.yml @@ -4,7 +4,6 @@ de: gpt: error: Beim Generieren des Unit-Tests ist ein Fehler aufgetreten. Bitte versuchen Sie es später erneut. internal_server_error: Der OpenAI-Server ist derzeit nicht verfügbar. Bitte versuchen Sie es später erneut, wenn das Problem behoben wurde. - invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig. Bitte fügen Sie den entsprechenden API-Schlüssel in Ihrem Profil hinzu, um die KI-Funktionen zu nutzen. + invalid_api_key: Der API-Schlüssel in Ihrem Profil ist ungültig oder abgelaufen. Bitte aktualisieren Sie den API-Schlüssel in Ihrem Profil um die KI-Funktionen zu nutzen und versuchen Sie es anschließend erneut. invalid_task_description: Die angegebene Aufgabenbeschreibung stellt keine gültige Programmieraufgabe dar und kann daher nicht zum Generieren eines Unit-Tests genutzt werden. Bitte stellen Sie sicher, dass die Aufgabenbeschreibung eine klar formulierte Problemstellung enthält, die durch ein Programm gelöst werden kann. missing_language: Für diese Aufgabe ist keine Programmiersprache angegeben. Bitte geben Sie die Sprache an, bevor Sie fortfahren. - unauthorized_error: Unbefugter Zugriff auf OpenAI erkannt. Dies geschieht normalerweise, wenn der API-Schlüssel falsch oder abgelaufen ist. Bitte überprüfen Sie Ihren API-Schlüssel und versuchen Sie es erneut. diff --git a/config/locales/en/errors.yml b/config/locales/en/errors.yml index 604cd8b10..c73caf65a 100644 --- a/config/locales/en/errors.yml +++ b/config/locales/en/errors.yml @@ -4,7 +4,6 @@ en: gpt: error: An error occurred while generating the unit test. Please try again later. internal_server_error: The OpenAI server is currently experiencing an outage. Please try again later when the issue is fixed. - invalid_api_key: The API key in your profile is invalid. Please add the appropriate API key in your profile to use the AI features. + invalid_api_key: The API key in your profile is invalid or has expired. Please update the API key in your profile to use the AI features and try again. invalid_task_description: The task description provided does not represent a valid programming task and therefore cannot be used to generate a unit test. Please make sure that the task description contains a clearly formulated problem that can be solved by a program. missing_language: Programming language is not specified for this task. Please specify the language before proceeding. - unauthorized_error: Unauthorized access to OpenAI detected. This usually happens when the API key is incorrect or expired. Please check your API key and try again. diff --git a/spec/errors/gpt/error_spec.rb b/spec/errors/gpt/error_spec.rb new file mode 100644 index 000000000..4758defdf --- /dev/null +++ b/spec/errors/gpt/error_spec.rb @@ -0,0 +1,11 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe Gpt::Error do + it 'has localized messages for all error classes' do + error_classes = described_class.descendants || [] + sample_errors = error_classes.map(&:new) + expect { sample_errors.map(&:localized_message) }.not_to raise_exception + end +end diff --git a/spec/services/task_service/gpt_generate_tests_spec.rb b/spec/services/task_service/gpt_generate_tests_spec.rb index 2f9ddadef..517f35537 100644 --- a/spec/services/task_service/gpt_generate_tests_spec.rb +++ b/spec/services/task_service/gpt_generate_tests_spec.rb @@ -1,4 +1,3 @@ -# spec/services/task_service/gpt_generate_tests_spec.rb # frozen_string_literal: true require 'rails_helper' @@ -136,15 +135,4 @@ end end end - - describe 'localized messages for all error classes' do - it 'has localized messages for all error classes' do - error_classes = Gpt::Error.descendants - sample_errors = error_classes.map(&:new) - - sample_errors.each do |error| - expect { error.localized_message }.not_to raise_error, "Missing translation for #{error.class.name}" - end - end - end end From e2a4139a0a3a4ed0f9c3fbe554233717bda15f60 Mon Sep 17 00:00:00 2001 From: Sebastian Serth Date: Wed, 31 Jul 2024 15:09:58 +0200 Subject: [PATCH 10/11] Exclude manual API key validation during execute --- app/services/task_service/gpt_generate_tests.rb | 7 ++++--- spec/services/task_service/gpt_generate_tests_spec.rb | 4 ++++ 2 files changed, 8 insertions(+), 3 deletions(-) diff --git a/app/services/task_service/gpt_generate_tests.rb b/app/services/task_service/gpt_generate_tests.rb index 85b9fdcc7..b5977cf17 100644 --- a/app/services/task_service/gpt_generate_tests.rb +++ b/app/services/task_service/gpt_generate_tests.rb @@ -7,7 +7,8 @@ def initialize(task:, openai_api_key:) raise Gpt::Error::MissingLanguage if task.programming_language&.language.blank? @task = task - @client = self.class.new_client! openai_api_key + # We can skip validating here, since an invalid API key will raise during `execute`, too. + @client = self.class.new_client! openai_api_key, validate: false end def execute @@ -19,11 +20,11 @@ def execute @task.tests << test end - def self.new_client!(access_token) + def self.new_client!(access_token, validate: true) raise Gpt::Error::InvalidApiKey if access_token.blank? client = OpenAI::Client.new(access_token:) - validate! client + validate! client if validate client end diff --git a/spec/services/task_service/gpt_generate_tests_spec.rb b/spec/services/task_service/gpt_generate_tests_spec.rb index 517f35537..4abb69bac 100644 --- a/spec/services/task_service/gpt_generate_tests_spec.rb +++ b/spec/services/task_service/gpt_generate_tests_spec.rb @@ -45,6 +45,10 @@ expect { gpt_generate_tests_service }.to raise_error(Gpt::Error::InvalidApiKey) end end + end + + describe '.new_client!' do + subject(:gpt_generate_tests_service) { described_class.new_client!(openai_api_key, validate: true) } context 'when API key is invalid' do let(:openai_api_key) { 'invalid_api_key' } From dbc61fc46fc0d9a1e93030ac91ad5970482fbdd9 Mon Sep 17 00:00:00 2001 From: kkoehn Date: Thu, 1 Aug 2024 12:52:24 +0200 Subject: [PATCH 11/11] refactor and split GptService (#1557) --- app/controllers/tasks_controller.rb | 2 +- app/models/user.rb | 2 +- .../generate_tests.rb} | 39 +-------- app/services/gpt_service/gpt_service_base.rb | 23 +++++ app/services/gpt_service/validate_api_key.rb | 22 +++++ spec/controllers/tasks_controller_spec.rb | 8 +- spec/models/user_spec.rb | 11 +-- .../generate_tests_spec.rb} | 50 +++-------- .../gpt_service/validate_api_key_spec.rb | 83 +++++++++++++++++++ 9 files changed, 154 insertions(+), 86 deletions(-) rename app/services/{task_service/gpt_generate_tests.rb => gpt_service/generate_tests.rb} (63%) create mode 100644 app/services/gpt_service/gpt_service_base.rb create mode 100644 app/services/gpt_service/validate_api_key.rb rename spec/services/{task_service/gpt_generate_tests_spec.rb => gpt_service/generate_tests_spec.rb} (79%) create mode 100644 spec/services/gpt_service/validate_api_key_spec.rb diff --git a/app/controllers/tasks_controller.rb b/app/controllers/tasks_controller.rb index d66e5bea7..2a4c7315a 100644 --- a/app/controllers/tasks_controller.rb +++ b/app/controllers/tasks_controller.rb @@ -209,7 +209,7 @@ def export_external_confirm # rubocop:enable Metrics/AbcSize def generate_test - TaskService::GptGenerateTests.call(task: @task, openai_api_key: current_user.openai_api_key) + GptService::GenerateTests.call(task: @task, openai_api_key: current_user.openai_api_key) flash[:notice] = I18n.t('tasks.task_service.gpt_generate_tests.successful_generation') rescue Gpt::Error => e flash[:alert] = e.localized_message diff --git a/app/models/user.rb b/app/models/user.rb index 799ae3774..d7867f0f6 100644 --- a/app/models/user.rb +++ b/app/models/user.rb @@ -152,7 +152,7 @@ def to_s def validate_openai_api_key return unless openai_api_key_changed? - TaskService::GptGenerateTests.new_client! openai_api_key + GptService::ValidateApiKey.call(openai_api_key:) rescue Gpt::Error::InvalidApiKey errors.add(:base, :invalid_api_key) end diff --git a/app/services/task_service/gpt_generate_tests.rb b/app/services/gpt_service/generate_tests.rb similarity index 63% rename from app/services/task_service/gpt_generate_tests.rb rename to app/services/gpt_service/generate_tests.rb index b5977cf17..fdd9e6316 100644 --- a/app/services/task_service/gpt_generate_tests.rb +++ b/app/services/gpt_service/generate_tests.rb @@ -1,14 +1,13 @@ # frozen_string_literal: true -module TaskService - class GptGenerateTests < ServiceBase +module GptService + class GenerateTests < GptServiceBase def initialize(task:, openai_api_key:) super() raise Gpt::Error::MissingLanguage if task.programming_language&.language.blank? @task = task - # We can skip validating here, since an invalid API key will raise during `execute`, too. - @client = self.class.new_client! openai_api_key, validate: false + @client = new_client! openai_api_key end def execute @@ -20,14 +19,6 @@ def execute @task.tests << test end - def self.new_client!(access_token, validate: true) - raise Gpt::Error::InvalidApiKey if access_token.blank? - - client = OpenAI::Client.new(access_token:) - validate! client if validate - client - end - private def gpt_response @@ -72,29 +63,5 @@ def training_prompts PROMPT ] end - - def wrap_api_error!(...) - # Use a custom forward for the private class method :wrap_api_error! and forward all arguments - self.class.send(:wrap_api_error!, ...) - end - - def self.validate!(client) - wrap_api_error! do - response = client.models.list - raise Gpt::Error::InvalidApiKey unless response['data'] - end - end - private_class_method :validate! - - def self.wrap_api_error! - yield - rescue Faraday::UnauthorizedError, OpenAI::Error => e - raise Gpt::Error::InvalidApiKey.new("Could not authenticate with OpenAI: #{e.message}") - rescue Faraday::Error => e - raise Gpt::Error::InternalServerError.new("Could not communicate with OpenAI: #{e.inspect}") - rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError => e - raise Gpt::Error.new(e) - end - private_class_method :wrap_api_error! end end diff --git a/app/services/gpt_service/gpt_service_base.rb b/app/services/gpt_service/gpt_service_base.rb new file mode 100644 index 000000000..7d8a46ba5 --- /dev/null +++ b/app/services/gpt_service/gpt_service_base.rb @@ -0,0 +1,23 @@ +# frozen_string_literal: true + +module GptService + class GptServiceBase < ServiceBase + def new_client!(access_token) + raise Gpt::Error::InvalidApiKey if access_token.blank? + + OpenAI::Client.new(access_token:) + end + + private + + def wrap_api_error! + yield + rescue Faraday::UnauthorizedError, OpenAI::Error => e + raise Gpt::Error::InvalidApiKey.new("Could not authenticate with OpenAI: #{e.message}") + rescue Faraday::Error => e + raise Gpt::Error::InternalServerError.new("Could not communicate with OpenAI: #{e.inspect}") + rescue Net::OpenTimeout, Net::ReadTimeout, Errno::ECONNRESET, SocketError, EOFError => e + raise Gpt::Error.new(e) + end + end +end diff --git a/app/services/gpt_service/validate_api_key.rb b/app/services/gpt_service/validate_api_key.rb new file mode 100644 index 000000000..c54697e69 --- /dev/null +++ b/app/services/gpt_service/validate_api_key.rb @@ -0,0 +1,22 @@ +# frozen_string_literal: true + +module GptService + class ValidateApiKey < GptServiceBase + def initialize(openai_api_key:) + super() + + @client = new_client! openai_api_key + end + + def execute + validate! + end + + def validate! + wrap_api_error! do + response = @client.models.list + raise Gpt::Error::InvalidApiKey unless response['data'] + end + end + end +end diff --git a/spec/controllers/tasks_controller_spec.rb b/spec/controllers/tasks_controller_spec.rb index c623018f5..04d2cb051 100644 --- a/spec/controllers/tasks_controller_spec.rb +++ b/spec/controllers/tasks_controller_spec.rb @@ -1088,12 +1088,12 @@ context 'when GptGenerateTests is successful' do before do - allow(TaskService::GptGenerateTests).to receive(:call) + allow(GptService::GenerateTests).to receive(:call) post :generate_test, params: {id: task.id} end it 'calls the GptGenerateTests service with the correct parameters' do - expect(TaskService::GptGenerateTests).to have_received(:call).with(task:, openai_api_key: 'valid_api_key') + expect(GptService::GenerateTests).to have_received(:call).with(task:, openai_api_key: 'valid_api_key') end it 'redirects to the task show page' do @@ -1107,7 +1107,7 @@ context 'when GptGenerateTests raises Gpt::Error::MissingLanguage' do before do - allow(TaskService::GptGenerateTests).to receive(:call).and_raise(Gpt::Error::MissingLanguage) + allow(GptService::GenerateTests).to receive(:call).and_raise(Gpt::Error::MissingLanguage) post :generate_test, params: {id: task.id} end @@ -1122,7 +1122,7 @@ context 'when GptGenerateTests raises Gpt::Error::InvalidTaskDescription' do before do - allow(TaskService::GptGenerateTests).to receive(:call).and_raise(Gpt::Error::InvalidTaskDescription) + allow(GptService::GenerateTests).to receive(:call).and_raise(Gpt::Error::InvalidTaskDescription) post :generate_test, params: {id: task.id} end diff --git a/spec/models/user_spec.rb b/spec/models/user_spec.rb index fc7e012d7..2ab663aa5 100644 --- a/spec/models/user_spec.rb +++ b/spec/models/user_spec.rb @@ -52,11 +52,10 @@ end context 'when openai_api_key is present and valid' do - let(:openai_client) { instance_double(OpenAI::Client) } let(:openai_api_key) { 'valid_key' } before do - allow(TaskService::GptGenerateTests).to receive(:new_client!).with(openai_api_key).and_return(openai_client) + allow(GptService::ValidateApiKey).to receive(:call).with(openai_api_key:) user.update(openai_api_key:) end @@ -66,11 +65,10 @@ end context 'when openai_api_key is present and invalid' do - let(:openai_client) { instance_double(OpenAI::Client) } let(:openai_api_key) { 'invalid_key' } before do - allow(TaskService::GptGenerateTests).to receive(:new_client!).with(openai_api_key).and_raise(Gpt::Error::InvalidApiKey) + allow(GptService::ValidateApiKey).to receive(:call).with(openai_api_key:).and_raise(Gpt::Error::InvalidApiKey) user.update(openai_api_key:) end @@ -85,16 +83,15 @@ end context 'when openai_api_key remains the same' do - let(:openai_client) { instance_double(OpenAI::Client) } let(:openai_api_key) { 'same_key' } before do - allow(TaskService::GptGenerateTests).to receive(:new_client!).with(openai_api_key).and_return(openai_client) + allow(GptService::ValidateApiKey).to receive(:call).with(openai_api_key:) user.update(openai_api_key:) end it 'does not trigger API validation' do - expect(TaskService::GptGenerateTests).not_to receive(:new_client!) + expect(GptService::ValidateApiKey).not_to receive(:call) expect { user.update(openai_api_key:) }.not_to change(user, :openai_api_key) end diff --git a/spec/services/task_service/gpt_generate_tests_spec.rb b/spec/services/gpt_service/generate_tests_spec.rb similarity index 79% rename from spec/services/task_service/gpt_generate_tests_spec.rb rename to spec/services/gpt_service/generate_tests_spec.rb index 4abb69bac..d30aec325 100644 --- a/spec/services/task_service/gpt_generate_tests_spec.rb +++ b/spec/services/gpt_service/generate_tests_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' -RSpec.describe TaskService::GptGenerateTests do +RSpec.describe GptService::GenerateTests do let(:openai_api_key) { 'valid_api_key' } let(:openai_client) { OpenAI::Client.new(access_token: openai_api_key) } let(:openai_models) { instance_double(OpenAI::Models, list: {'data' => [{'id' => 'model-id'}]}) } @@ -47,42 +47,6 @@ end end - describe '.new_client!' do - subject(:gpt_generate_tests_service) { described_class.new_client!(openai_api_key, validate: true) } - - context 'when API key is invalid' do - let(:openai_api_key) { 'invalid_api_key' } - - before do - allow(openai_models).to receive(:list).and_raise(Faraday::UnauthorizedError) - end - - it 'raises InvalidApiKeyError' do - expect { gpt_generate_tests_service }.to raise_error(Gpt::Error::InvalidApiKey) - end - end - - context 'when OpenAI is not responding' do - before do - allow(openai_models).to receive(:list).and_raise(Faraday::Error) - end - - it 'raises InternalServerError' do - expect { gpt_generate_tests_service }.to raise_error(Gpt::Error::InternalServerError) - end - end - - context 'when the network connection is broken' do - before do - allow(openai_models).to receive(:list).and_raise(EOFError) - end - - it 'raises an error' do - expect { gpt_generate_tests_service }.to raise_error(Gpt::Error) - end - end - end - describe '#call' do subject(:gpt_generate_tests) { described_class.call(task:, openai_api_key:) } @@ -138,5 +102,17 @@ expect { gpt_generate_tests }.to raise_error(Gpt::Error) end end + + context 'when API key is invalid' do + let(:openai_api_key) { 'invalid_api_key' } + + before do + allow(openai_client).to receive(:chat).and_raise(Faraday::UnauthorizedError) + end + + it 'raises InvalidApiKeyError' do + expect { gpt_generate_tests }.to raise_error(Gpt::Error::InvalidApiKey) + end + end end end diff --git a/spec/services/gpt_service/validate_api_key_spec.rb b/spec/services/gpt_service/validate_api_key_spec.rb new file mode 100644 index 000000000..58938df2e --- /dev/null +++ b/spec/services/gpt_service/validate_api_key_spec.rb @@ -0,0 +1,83 @@ +# frozen_string_literal: true + +require 'rails_helper' + +RSpec.describe GptService::ValidateApiKey do + let(:openai_api_key) { 'valid_api_key' } + let(:openai_client) { OpenAI::Client.new(access_token: openai_api_key) } + let(:openai_models) { instance_double(OpenAI::Models, list: {'data' => models_list}) } + let(:models_list) { [{'id' => 'model-id'}] } + + before do + allow(OpenAI::Client).to receive(:new).and_return(openai_client) + allow(openai_client).to receive(:models).and_return(openai_models) + end + + describe '.new' do + subject(:validate_api_key) { described_class.new(openai_api_key:) } + + it 'assigns the client for OpenAI' do + expect(validate_api_key.instance_variable_get(:@client)).to be openai_client + end + + it 'stores the OpenAI API key in the client' do + expect(validate_api_key.instance_variable_get(:@client).access_token).to eq openai_api_key + end + + context 'when API key is missing' do + let(:openai_api_key) { nil } + + it 'raises InvalidApiKeyError' do + expect { validate_api_key }.to raise_error(Gpt::Error::InvalidApiKey) + end + end + end + + describe '#call' do + subject(:validate_api_key) { described_class.call(openai_api_key:) } + + it 'does not raise an error' do + expect { validate_api_key }.not_to raise_error + end + + context 'when model list is empty' do + let(:models_list) {} + + it 'raises correct error' do + expect { validate_api_key }.to raise_error(Gpt::Error::InvalidApiKey) + end + end + + context 'when API key is invalid' do + let(:openai_api_key) { 'invalid_api_key' } + + before do + allow(openai_models).to receive(:list).and_raise(Faraday::UnauthorizedError) + end + + it 'raises InvalidApiKeyError' do + expect { validate_api_key }.to raise_error(Gpt::Error::InvalidApiKey) + end + end + + context 'when OpenAI is not responding' do + before do + allow(openai_models).to receive(:list).and_raise(Faraday::Error) + end + + it 'raises InternalServerError' do + expect { validate_api_key }.to raise_error(Gpt::Error::InternalServerError) + end + end + + context 'when the network connection is broken' do + before do + allow(openai_models).to receive(:list).and_raise(EOFError) + end + + it 'raises an error' do + expect { validate_api_key }.to raise_error(Gpt::Error) + end + end + end +end