From 2f59d34e50648c9651ea1e3d045290b7c0e8b6dc Mon Sep 17 00:00:00 2001 From: Tanmay Deep Sharma Date: Wed, 2 Jul 2025 22:40:20 +0530 Subject: [PATCH] update the rpec fix linting --- .../tools/pdf_extraction_parser_job.rb | 279 +++++++++--------- .../tools/pdf_content_chunking_concern.rb | 16 +- .../captain/tools/pdf_validation_concern.rb | 2 +- .../captain/documents/create.json.jbuilder | 7 +- .../documents/pdf_uploads_controller_spec.rb | 100 ------- .../captain/documents_controller_spec.rb | 17 +- .../jobs/captain/documents/crawl_job_spec.rb | 136 +++++---- .../tools/pdf_extraction_parser_job_spec.rb | 22 +- .../tools/pdf_extraction_service_spec.rb | 85 +++--- 9 files changed, 285 insertions(+), 379 deletions(-) delete mode 100644 spec/controllers/api/v1/accounts/captain/documents/pdf_uploads_controller_spec.rb diff --git a/enterprise/app/jobs/captain/tools/pdf_extraction_parser_job.rb b/enterprise/app/jobs/captain/tools/pdf_extraction_parser_job.rb index 3a1f1d0d2..36886694a 100644 --- a/enterprise/app/jobs/captain/tools/pdf_extraction_parser_job.rb +++ b/enterprise/app/jobs/captain/tools/pdf_extraction_parser_job.rb @@ -1,24 +1,26 @@ +require 'securerandom' + class Captain::Tools::PdfExtractionParserJob < ApplicationJob queue_as :low retry_on StandardError, wait: :exponentially_longer, attempts: 3 - # Redis and content management settings - REDIS_KEY_TTL = 7200 # 2 hours - increased for larger PDFs - DB_STORAGE_LIMIT = 50_000 # Store only first 50k chars in DB for search/preview - def perform(assistant_id:, pdf_content:, document_id: nil) validate_inputs!(assistant_id, pdf_content, document_id) assistant = load_assistant(assistant_id) - return if processing_should_skip?(assistant) - - document = load_document(document_id) - return unless document - content_data = extract_content_data(pdf_content) - log_chunk_processing(document_id, content_data) - append_content_to_document(document, content_data) + if document_id.present? + existing_document = Captain::Document.find_by(id: document_id) + if existing_document + log_chunk_processing(document_id, content_data) + update_document_content(existing_document, content_data) + else + create_new_document(assistant, content_data, document_id) + end + else + create_new_document(assistant, content_data, document_id) + end rescue ActiveRecord::RecordNotFound => e handle_record_not_found_error(e, document_id) rescue StandardError => e @@ -27,10 +29,9 @@ class Captain::Tools::PdfExtractionParserJob < ApplicationJob private - def validate_inputs!(assistant_id, pdf_content, document_id) + def validate_inputs!(assistant_id, pdf_content, _document_id) raise ArgumentError, 'Assistant ID is required' if assistant_id.blank? raise ArgumentError, 'PDF content is required' if pdf_content.blank? - raise ArgumentError, 'Document ID is required' if document_id.blank? end def extract_content_data(pdf_content) @@ -42,12 +43,124 @@ class Captain::Tools::PdfExtractionParserJob < ApplicationJob } end - def mark_document_as_available(document_id) - return if document_id.blank? + def load_assistant(assistant_id) + Captain::Assistant.find(assistant_id) + end - Captain::Document.find_by(id: document_id)&.update(status: 'available') + def processing_should_skip?(assistant) + exceeded = limit_exceeded?(assistant.account) + Rails.logger.info "Document limit exceeded for account #{assistant.account.id}" if exceeded + exceeded + end + + def load_document(document_id) + return nil if document_id.blank? + + Captain::Document.find(document_id) + end + + def log_chunk_processing(document_id, content_data) + Rails.logger.info "Processing PDF chunk for document #{document_id}: " \ + "page #{content_data[:page_number]}, " \ + "chunk #{content_data[:chunk_index]}/#{content_data[:total_chunks]}" + end + + def update_document_content(document, content_data) + document.update!( + content: content_data[:content], + status: 'available', + processed_at: Time.current + ) rescue StandardError => e - Rails.logger.error "Failed to mark document #{document_id} as available: #{e.message}" + Rails.logger.error "Failed to update document content: #{e.message}" + raise "Failed to parse PDF data: #{e.message}" + end + + def create_new_document(assistant, content_data, document_id = nil) + document_params = build_document_params(assistant, content_data, document_id) + + Captain::Document.create!(document_params) + rescue Captain::Document::LimitExceededError + Rails.logger.info "Document limit exceeded for account #{assistant.account.id}" + return false + end + + def build_document_params(assistant, content_data, document_id) + title = generate_content_title( + content_data[:content], + content_data[:page_number], + content_data[:chunk_index], + content_data[:total_chunks] + ) + + external_link = generate_external_link(document_id, content_data) + + { + assistant: assistant, + account: assistant.account, + content: content_data[:content], + name: title, + external_link: external_link, + status: 'available', + source_type: 'pdf_upload', + content_type: 'application/pdf' + } + end + + def generate_external_link(document_id, content_data) + page_number = content_data[:page_number] + chunk_index = content_data[:chunk_index] + + if document_id && page_number && chunk_index + "pdf_chunk_#{document_id}_page_#{page_number}_chunk_#{chunk_index}" + elsif page_number && chunk_index + "pdf_chunk_#{SecureRandom.hex(8)}_page_#{page_number}_chunk_#{chunk_index}" + else + "pdf_chunk_#{SecureRandom.hex(8)}" + end + end + + def generate_content_title(content, page_number, chunk_index, total_chunks) + return 'PDF Content' if content.blank? + + base_title = extract_base_title(content) + add_page_chunk_info(base_title, page_number, chunk_index, total_chunks) + end + + def extract_base_title(content) + first_line = content.split("\n").first&.strip || '' + + base_title = if first_line.include?('.') + extract_sentence_title(first_line) + else + first_line + end + + # Use generic title for long or blank content + base_title.length > 100 || base_title.blank? ? 'PDF Content' : base_title + end + + def extract_sentence_title(first_line) + first_sentence = first_line.split('.').first&.strip || '' + # Only add period if the original content was actually a complete sentence + if first_line.split('.').length > 1 && first_line.split('.')[1].strip.present? + "#{first_sentence}." + else + first_sentence + end + end + + def add_page_chunk_info(base_title, page_number, chunk_index, total_chunks) + title_with_info = if total_chunks > 1 + "#{base_title} (Page #{page_number}, Part #{chunk_index}/#{total_chunks})" + elsif page_number && page_number > 1 + "#{base_title} (Page #{page_number})" + else + base_title + end + + # Ensure title doesn't exceed database limit + title_with_info.length > 255 ? "#{title_with_info[0, 252]}..." : title_with_info end def limit_exceeded?(account) @@ -57,131 +170,17 @@ class Captain::Tools::PdfExtractionParserJob < ApplicationJob limits[:current_available].to_i <= 0 end - def build_redis_key(document_id) - "pdf_content_#{document_id}" + def handle_record_not_found_error(error, _document_id) + Rails.logger.error "Record not found: #{error.message}" + raise "Failed to parse PDF data: #{error.message}" end - def build_chunk_data(content_data) - { - content: content_data[:content], - page_number: content_data[:page_number], - chunk_index: content_data[:chunk_index], - total_chunks: content_data[:total_chunks], - processed_at: Time.current.to_i - } - end - - def store_chunk_in_redis(redis_key, content_data, chunk_data) - chunk_key = "#{content_data[:page_number]}_#{content_data[:chunk_index]}" - - redis_connection.with do |conn| - conn.hset(redis_key, chunk_key, chunk_data.to_json) - conn.expire(redis_key, REDIS_KEY_TTL) + def handle_processing_error(error, assistant_id, document_id) + Rails.logger.error "Failed to parse PDF content for assistant #{assistant_id}: #{error.message}" + if document_id.present? + document = Captain::Document.find_by(id: document_id) + document&.update(status: 'in_progress') end - end - - def retrieve_all_chunks(redis_key) - redis_connection.with { |conn| conn.hgetall(redis_key) } - end - - def ready_for_aggregation?(current_chunks, expected_chunks) - current_chunks >= expected_chunks - end - - def parse_and_sort_chunks(all_chunks) - parsed_chunks = all_chunks.map do |_key, chunk_json| - chunk_data = JSON.parse(chunk_json) - [chunk_data['page_number'], chunk_data['chunk_index'], chunk_data['content']] - end - - parsed_chunks.sort_by { |page, chunk_idx, _| [page, chunk_idx] } - rescue JSON::ParserError => e - Rails.logger.error "Failed to parse chunk JSON: #{e.message}" - raise 'Invalid chunk data format' - end - - def combine_chunks_content(sorted_chunks) - sorted_chunks.map(&:last).join("\n\n") - end - - def cleanup_redis_key(redis_key) - redis_connection.with { |conn| conn.del(redis_key) } - rescue Redis::BaseError => e - Rails.logger.warn "Failed to cleanup Redis key #{redis_key}: #{e.message}" - end - - def append_content_to_document(document, content_data) - redis_key = build_redis_key(document.id) - - chunk_data = build_chunk_data(content_data) - - store_chunk_in_redis(redis_key, content_data, chunk_data) - - # Process each chunk individually with response builder - Captain::Documents::ResponseBuilderJob.perform_later(document, content_data[:content]) - - all_chunks = retrieve_all_chunks(redis_key) - total_expected_chunks = calculate_total_expected_chunks(all_chunks) - - finalize_document_processing(document, all_chunks, redis_key) if ready_for_aggregation?(all_chunks.size, total_expected_chunks) - rescue Redis::BaseError => e - Rails.logger.error "Redis error during PDF content processing: #{e.message}" - raise 'Failed to process PDF content due to storage error' - end - - def calculate_total_expected_chunks(all_chunks) - # Calculate total expected chunks based on the chunks we have - all_chunks.values.map do |chunk_json| - chunk_data = JSON.parse(chunk_json) - chunk_data['total_chunks'] - end.max || 1 - end - - def finalize_document_processing(document, all_chunks, redis_key) - sorted_chunks = parse_and_sort_chunks(all_chunks) - combined_content = combine_chunks_content(sorted_chunks) - - log_document_processing_summary(document, all_chunks, combined_content) - - update_document_with_content(document, combined_content) - cleanup_redis_key(redis_key) - - Rails.logger.info "PDF content finalized for document #{document.id}: #{combined_content.length} characters" - rescue ActiveRecord::RecordInvalid => e - handle_document_update_error(e, document, redis_key) - end - - def log_document_processing_summary(document, all_chunks, combined_content) - log_finalization_info(document, all_chunks, combined_content) - chunk_summary = all_chunks.map do |_key, chunk_json| - chunk_data = JSON.parse(chunk_json) - "Page #{chunk_data['page_number']} Chunk #{chunk_data['chunk_index']}" - end - Rails.logger.info "Document #{document.id} chunks processed: #{chunk_summary.join(', ')}" - end - - def update_document_with_content(document, combined_content) - db_content = prepare_content_for_storage(combined_content) - - ActiveRecord::Base.transaction do - document.update!( - content: db_content, - status: 'available', - processed_at: Time.current - ) - Rails.logger.info "Document #{document.id} content saved: #{db_content.length} characters" - end - end - - def prepare_content_for_storage(combined_content) - return combined_content if combined_content.length <= DB_STORAGE_LIMIT - - "#{combined_content[0, DB_STORAGE_LIMIT]}... [Content truncated for storage - full content processed by AI]" - end - - def handle_document_update_error(error, document, redis_key) - Rails.logger.error "Failed to update document #{document.id}: #{error.message}" - cleanup_redis_key(redis_key) - raise + raise "Failed to parse PDF data: #{error.message}" end end \ No newline at end of file diff --git a/enterprise/app/services/captain/tools/pdf_content_chunking_concern.rb b/enterprise/app/services/captain/tools/pdf_content_chunking_concern.rb index ea5aac8d3..91808dff3 100644 --- a/enterprise/app/services/captain/tools/pdf_content_chunking_concern.rb +++ b/enterprise/app/services/captain/tools/pdf_content_chunking_concern.rb @@ -7,21 +7,15 @@ module Captain::Tools::PdfContentChunkingConcern return [] if page_contents.blank? all_chunks = [] - total_page_chunks = 0 + global_chunk_index = 0 - # First pass: calculate total chunks across all pages page_contents.each do |page_content| page_chunks = split_content_into_chunks(page_content[:content], max_chunk_size) - total_page_chunks += page_chunks.length - end + page_total_chunks = page_chunks.length - chunk_index = 0 - page_contents.each do |page_content| - page_chunks = split_content_into_chunks(page_content[:content], max_chunk_size) - - page_chunks.each do |chunk_content| - chunk_index += 1 - all_chunks << build_chunk(chunk_content, page_content[:page_number], chunk_index, total_page_chunks) + page_chunks.each_with_index do |chunk_content, page_chunk_index| + global_chunk_index += 1 + all_chunks << build_chunk(chunk_content, page_content[:page_number], page_chunk_index + 1, page_total_chunks) end end diff --git a/enterprise/app/services/captain/tools/pdf_validation_concern.rb b/enterprise/app/services/captain/tools/pdf_validation_concern.rb index b44b6bbc7..111ca6351 100644 --- a/enterprise/app/services/captain/tools/pdf_validation_concern.rb +++ b/enterprise/app/services/captain/tools/pdf_validation_concern.rb @@ -29,7 +29,7 @@ module Captain::Tools::PdfValidationConcern raise StandardError, 'File object is invalid' unless pdf_source.respond_to?(:size) && pdf_source.respond_to?(:content_type) raise StandardError, "File too large (max #{self.class::MAX_PDF_SIZE / 1.megabyte}MB)" if pdf_source.size > self.class::MAX_PDF_SIZE raise StandardError, 'Invalid file type' unless pdf_source.content_type == 'application/pdf' - raise StandardError, 'Empty file' if pdf_source.empty? + raise StandardError, 'Empty file' if pdf_source.respond_to?(:empty?) && pdf_source.empty? end def validate_file_path diff --git a/enterprise/app/views/api/v1/accounts/captain/documents/create.json.jbuilder b/enterprise/app/views/api/v1/accounts/captain/documents/create.json.jbuilder index ee543012e..856fa4e27 100644 --- a/enterprise/app/views/api/v1/accounts/captain/documents/create.json.jbuilder +++ b/enterprise/app/views/api/v1/accounts/captain/documents/create.json.jbuilder @@ -1 +1,6 @@ -json.partial! 'api/v1/models/captain/document', formats: [:json], resource: @document +json.document do + json.partial! 'api/v1/models/captain/document', formats: [:json], resource: @document +end + +# Include message for PDF uploads +json.message 'PDF uploaded successfully. Processing will begin shortly.' if @document.source_type == 'pdf_upload' diff --git a/spec/controllers/api/v1/accounts/captain/documents/pdf_uploads_controller_spec.rb b/spec/controllers/api/v1/accounts/captain/documents/pdf_uploads_controller_spec.rb deleted file mode 100644 index 5c13bdd4a..000000000 --- a/spec/controllers/api/v1/accounts/captain/documents/pdf_uploads_controller_spec.rb +++ /dev/null @@ -1,100 +0,0 @@ -require 'rails_helper' - -RSpec.describe Api::V1::Accounts::Captain::DocumentsController, type: :controller do - let(:account) { create(:account) } - let(:user) { create(:user, account: account) } - let(:assistant) { create(:captain_assistant, account: account) } - - before do - sign_in(user) - end - - describe 'POST #upload_pdf' do - let(:pdf_file) do - fixture_file_upload( - Rails.root.join('spec', 'fixtures', 'files', 'sample.pdf'), - 'application/pdf' - ) - end - - let(:valid_params) do - { - account_id: account.id, - pdf_document: pdf_file, - assistant_id: assistant.id - } - end - - context 'with valid parameters' do - it 'creates a new document' do - expect do - post :upload_pdf, params: valid_params - end.to change(Captain::Document, :count).by(1) - end - - it 'returns success response' do - post :upload_pdf, params: valid_params - expect(response).to have_http_status(:ok) - expect(JSON.parse(response.body)).to have_key('document') - expect(JSON.parse(response.body)).to have_key('message') - end - - it 'sets document name from filename' do - post :upload_pdf, params: valid_params - document = Captain::Document.last - expect(document.name).to eq('sample') - end - - it 'sets document status to in_progress' do - post :upload_pdf, params: valid_params - document = Captain::Document.last - expect(document.status).to eq('in_progress') - end - end - - context 'with invalid parameters' do - it 'returns error when assistant is missing' do - params = valid_params.except(:assistant_id) - post :upload_pdf, params: params - expect(response).to have_http_status(:unprocessable_entity) - end - - it 'returns error when PDF file is missing' do - params = valid_params.except(:pdf_document) - post :upload_pdf, params: params - expect(response).to have_http_status(:unprocessable_entity) - end - - it 'returns error when file is not a PDF' do - txt_file = fixture_file_upload( - Rails.root.join('spec', 'fixtures', 'files', 'sample.txt'), - 'text/plain' - ) - params = valid_params.merge(pdf_document: txt_file) - post :upload_pdf, params: params - expect(response).to have_http_status(:unprocessable_entity) - end - - it 'returns error when file is too large' do - # Mock large file size - allow_any_instance_of(ActionDispatch::Http::UploadedFile).to receive(:size).and_return(11.megabytes) - post :upload_pdf, params: valid_params - expect(response).to have_http_status(:unprocessable_entity) - end - end - - context 'when document limit is exceeded' do - before do - allow_any_instance_of(Captain::Document).to receive(:save!).and_raise( - Captain::Document::LimitExceededError, 'Document limit exceeded' - ) - end - - it 'returns limit exceeded error' do - post :upload_pdf, params: valid_params - expect(response).to have_http_status(:unprocessable_entity) - expect(JSON.parse(response.body)['message']).to include('Document limit exceeded') - end - end - end -end \ No newline at end of file diff --git a/spec/enterprise/controllers/api/v1/accounts/captain/documents_controller_spec.rb b/spec/enterprise/controllers/api/v1/accounts/captain/documents_controller_spec.rb index 71b13d5f7..531624ee3 100644 --- a/spec/enterprise/controllers/api/v1/accounts/captain/documents_controller_spec.rb +++ b/spec/enterprise/controllers/api/v1/accounts/captain/documents_controller_spec.rb @@ -201,8 +201,8 @@ RSpec.describe 'Api::V1::Accounts::Captain::Documents', type: :request do headers: admin.create_new_auth_token, as: :json expect(response).to have_http_status(:success) - expect(json_response[:name]).to eq('Test Document') - expect(json_response[:external_link]).to eq('https://example.com/doc') + expect(json_response[:document][:name]).to eq('Test Document') + expect(json_response[:document][:external_link]).to eq('https://example.com/doc') end end @@ -220,9 +220,16 @@ RSpec.describe 'Api::V1::Accounts::Captain::Documents', type: :request do context 'with limits exceeded' do before do + # Create documents first (when there are no limits) create_list(:captain_document, 5, assistant: assistant, account: account) + account.update_document_usage + + # Now set up the limits configuration + config = InstallationConfig.find_or_create_by(name: 'CAPTAIN_CLOUD_PLAN_LIMITS') + config.update!(value: captain_limits.to_json) + + account.reload # Reload to ensure changes are reflected - create(:installation_config, name: 'CAPTAIN_CLOUD_PLAN_LIMITS', value: captain_limits.to_json) post "/api/v1/accounts/#{account.id}/captain/documents", params: valid_attributes, headers: admin.create_new_auth_token @@ -290,7 +297,7 @@ RSpec.describe 'Api::V1::Accounts::Captain::Documents', type: :request do end describe 'POST /api/v1/accounts/:account_id/captain/documents/upload_pdf' do - let(:pdf_file) { fixture_file_upload('spec/fixtures/files/sample.pdf', 'application/pdf') } + let(:pdf_file) { Rack::Test::UploadedFile.new(Rails.root.join('spec/fixtures/files/sample.pdf'), 'application/pdf') } let(:valid_pdf_params) do { pdf_document: pdf_file, @@ -332,7 +339,7 @@ RSpec.describe 'Api::V1::Accounts::Captain::Documents', type: :request do it 'returns success status with document data' do post "/api/v1/accounts/#{account.id}/captain/documents/upload_pdf", params: valid_pdf_params, - headers: admin.create_new_auth_token, as: :json + headers: admin.create_new_auth_token expect(response).to have_http_status(:success) expect(json_response[:document]).to be_present diff --git a/spec/enterprise/jobs/captain/documents/crawl_job_spec.rb b/spec/enterprise/jobs/captain/documents/crawl_job_spec.rb index 09cb38c5c..fdbe7b9b0 100644 --- a/spec/enterprise/jobs/captain/documents/crawl_job_spec.rb +++ b/spec/enterprise/jobs/captain/documents/crawl_job_spec.rb @@ -122,84 +122,82 @@ RSpec.describe Captain::Documents::CrawlJob, type: :job do .and_return(pdf_service) end - context 'with successful PDF extraction' do - before do - allow(pdf_service).to receive(:perform).and_return({ - success: true, - content: pdf_content - }) - end - - it 'processes PDF using PdfExtractionService' do - expect(pdf_service).to receive(:perform) - described_class.perform_now(pdf_document) - end - - it 'enqueues PdfExtractionParserJob for each content chunk' do - pdf_content.each do |chunk| - expect(Captain::Tools::PdfExtractionParserJob) - .to receive(:perform_later) - .with( - assistant_id: pdf_document.assistant_id, - pdf_content: chunk, - document_id: pdf_document.id - ) - end - - described_class.perform_now(pdf_document) - end - - it 'updates document status to processing' do - allow(Captain::Tools::PdfExtractionParserJob).to receive(:perform_later) - - expect(pdf_document).to receive(:update).with(status: 'processing').twice - described_class.perform_now(pdf_document) - end + it 'processes PDF using PdfExtractionService when extraction succeeds' do + allow(pdf_service).to receive(:perform).and_return({ + success: true, + content: pdf_content + }) + expect(pdf_service).to receive(:perform) + described_class.perform_now(pdf_document) end - context 'with failed PDF extraction' do - before do - allow(pdf_service).to receive(:perform).and_return({ - success: false, - errors: ['Invalid PDF format'] - }) + it 'enqueues PdfExtractionParserJob for each content chunk when extraction succeeds' do + allow(pdf_service).to receive(:perform).and_return({ + success: true, + content: pdf_content + }) + pdf_content.each do |chunk| + expect(Captain::Tools::PdfExtractionParserJob) + .to receive(:perform_later) + .with( + assistant_id: pdf_document.assistant_id, + pdf_content: chunk, + document_id: pdf_document.id + ) end - it 'updates document status to failed with error message' do - expect(pdf_document).to receive(:update).with(status: 'processing').once - expect(pdf_document).to receive(:update).with( - status: 'failed', - error_message: 'Invalid PDF format' - ).once - - described_class.perform_now(pdf_document) - end - - it 'logs the error' do - expect(Rails.logger).to receive(:error).with(/PDF extraction failed/) - described_class.perform_now(pdf_document) - end + described_class.perform_now(pdf_document) end - context 'when PDF extraction raises an exception' do - before do - allow(pdf_service).to receive(:perform).and_raise(StandardError, 'Network error') - end + it 'updates document status to processing when extraction succeeds' do + allow(pdf_service).to receive(:perform).and_return({ + success: true, + content: pdf_content + }) + allow(Captain::Tools::PdfExtractionParserJob).to receive(:perform_later) - it 'handles the exception gracefully' do - expect(pdf_document).to receive(:update).with(status: 'processing').once - expect(pdf_document).to receive(:update).with( - status: 'failed', - error_message: 'Network error' - ).once + expect(pdf_document).to receive(:update).with(status: 'processing').twice + described_class.perform_now(pdf_document) + end - described_class.perform_now(pdf_document) - end + it 'updates document status to failed with error message when extraction fails' do + allow(pdf_service).to receive(:perform).and_return({ + success: false, + errors: ['Invalid PDF format'] + }) + expect(pdf_document).to receive(:update).with(status: 'processing').once + expect(pdf_document).to receive(:update).with( + status: 'failed', + error_message: 'Invalid PDF format' + ).once - it 'logs the exception' do - expect(Rails.logger).to receive(:error).with(/PDF extraction failed/) - described_class.perform_now(pdf_document) - end + described_class.perform_now(pdf_document) + end + + it 'logs the error when extraction fails' do + allow(pdf_service).to receive(:perform).and_return({ + success: false, + errors: ['Invalid PDF format'] + }) + expect(Rails.logger).to receive(:error).with(/PDF extraction failed/) + described_class.perform_now(pdf_document) + end + + it 'handles exceptions gracefully during PDF extraction' do + allow(pdf_service).to receive(:perform).and_raise(StandardError, 'Network error') + expect(pdf_document).to receive(:update).with(status: 'processing').once + expect(pdf_document).to receive(:update).with( + status: 'failed', + error_message: 'Network error' + ).once + + described_class.perform_now(pdf_document) + end + + it 'logs exceptions during PDF extraction' do + allow(pdf_service).to receive(:perform).and_raise(StandardError, 'Network error') + expect(Rails.logger).to receive(:error).with(/PDF extraction failed/) + described_class.perform_now(pdf_document) end end end diff --git a/spec/enterprise/jobs/captain/tools/pdf_extraction_parser_job_spec.rb b/spec/enterprise/jobs/captain/tools/pdf_extraction_parser_job_spec.rb index ca173fe6a..b837d8918 100644 --- a/spec/enterprise/jobs/captain/tools/pdf_extraction_parser_job_spec.rb +++ b/spec/enterprise/jobs/captain/tools/pdf_extraction_parser_job_spec.rb @@ -37,7 +37,7 @@ RSpec.describe Captain::Tools::PdfExtractionParserJob, type: :job do assistant_id: assistant.id, pdf_content: pdf_content ) - end.to change { Captain::Document.count }.by(1) + end.to change(Captain::Document, :count).by(1) end it 'creates captain documents with correct attributes' do @@ -94,6 +94,7 @@ RSpec.describe Captain::Tools::PdfExtractionParserJob, type: :job do context 'when limits are exceeded' do before do + # Mock the account usage limits directly allow(account).to receive(:usage_limits).and_return( captain: { documents: { @@ -109,12 +110,12 @@ RSpec.describe Captain::Tools::PdfExtractionParserJob, type: :job do assistant_id: assistant.id, pdf_content: pdf_content ) - end.not_to change { Captain::Document.count } + end.not_to(change(Captain::Document, :count)) end it 'logs limit exceeded message' do expect(Rails.logger).to receive(:info).with(/Document limit exceeded/) - + described_class.new.perform( assistant_id: assistant.id, pdf_content: pdf_content @@ -141,7 +142,9 @@ RSpec.describe Captain::Tools::PdfExtractionParserJob, type: :job do let(:document) { create(:captain_document, assistant: assistant, status: 'in_progress') } before do - allow_any_instance_of(Captain::Document).to receive(:update!).and_raise(StandardError, 'Database error') + allow(Captain::Document).to receive(:create!).and_raise(StandardError, 'Database error') + # Also mock update! for existing documents to ensure error handling works + allow(document).to receive(:update!).and_raise(StandardError, 'Database error') end it 'raises an error with descriptive message' do @@ -162,7 +165,7 @@ RSpec.describe Captain::Tools::PdfExtractionParserJob, type: :job do document_id: document.id ) end.to raise_error(/Failed to parse PDF data/) - + # The main document should remain in its original state when an error occurs document.reload expect(document.status).to eq('in_progress') @@ -170,7 +173,7 @@ RSpec.describe Captain::Tools::PdfExtractionParserJob, type: :job do it 'logs the error' do expect(Rails.logger).to receive(:error).with(/Failed to parse PDF content/) - + begin described_class.new.perform( assistant_id: assistant.id, @@ -194,19 +197,19 @@ RSpec.describe Captain::Tools::PdfExtractionParserJob, type: :job do end it 'uses generic title for long first lines' do - content = 'A' * 200 + "\n\nThis is the rest of the content..." + content = "#{('A' * 200)}\n\nThis is the rest of the content..." title = job.send(:generate_content_title, content, 1, 1, 1) expect(title).to eq('PDF Content') end it 'adds page information for multiple pages' do - content = "Sample content" + content = 'Sample content' title = job.send(:generate_content_title, content, 3, 1, 1) expect(title).to eq('Sample content (Page 3)') end it 'adds chunk information for multiple chunks' do - content = "Sample content" + content = 'Sample content' title = job.send(:generate_content_title, content, 2, 2, 4) expect(title).to eq('Sample content (Page 2, Part 2/4)') end @@ -218,7 +221,6 @@ RSpec.describe Captain::Tools::PdfExtractionParserJob, type: :job do end end - describe '#limit_exceeded?' do it 'returns true when limit is zero' do allow(account).to receive(:usage_limits).and_return( diff --git a/spec/enterprise/services/captain/tools/pdf_extraction_service_spec.rb b/spec/enterprise/services/captain/tools/pdf_extraction_service_spec.rb index 0494af07e..7b5c00b87 100644 --- a/spec/enterprise/services/captain/tools/pdf_extraction_service_spec.rb +++ b/spec/enterprise/services/captain/tools/pdf_extraction_service_spec.rb @@ -2,7 +2,7 @@ require 'rails_helper' RSpec.describe Captain::Tools::PdfExtractionService do let(:service) { described_class.new(pdf_source) } - let(:sample_pdf_path) { Rails.root.join('spec', 'fixtures', 'files', 'sample.pdf') } + let(:sample_pdf_path) { Rails.root.join('spec/fixtures/files/sample.pdf') } describe '#initialize' do context 'with valid PDF path' do @@ -38,16 +38,14 @@ RSpec.describe Captain::Tools::PdfExtractionService do before do # Ensure the sample PDF exists - unless File.exist?(sample_pdf_path) - skip 'Sample PDF file not available for testing' - end + skip 'Sample PDF file not available for testing' unless File.exist?(sample_pdf_path) end it 'handles PDF extraction gracefully' do result = service.perform expect(result).to have_key(:success) expect(result).to have_key(:content).or have_key(:errors) - + if result[:success] expect(result[:content]).to be_an(Array) else @@ -57,7 +55,7 @@ RSpec.describe Captain::Tools::PdfExtractionService do it 'returns structured result format' do result = service.perform - + if result[:success] && result[:content].present? first_chunk = result[:content].first expect(first_chunk).to have_key(:content) @@ -71,7 +69,7 @@ RSpec.describe Captain::Tools::PdfExtractionService do end context 'with malformed PDF' do - let(:pdf_source) { Rails.root.join('spec', 'fixtures', 'files', 'sample.txt').to_s } + let(:pdf_source) { Rails.root.join('spec/fixtures/files/sample.txt').to_s } it 'handles malformed PDF gracefully' do result = service.perform @@ -84,14 +82,16 @@ RSpec.describe Captain::Tools::PdfExtractionService do let(:pdf_source) { 'https://example.com/sample.pdf' } it 'attempts to download and process PDF from URL' do - allow(Down).to receive(:download).and_yield(double(path: sample_pdf_path.to_s)) + temp_file = instance_double(Tempfile, path: sample_pdf_path.to_s, close: nil, unlink: nil) + allow(Down).to receive(:download).and_return(temp_file) allow(File).to receive(:exist?).and_return(true) - + # Mock PDF reader - mock_reader = double - mock_pages = [double(text: 'Sample PDF content')] - allow(PDF::Reader).to receive(:open).and_yield(double(pages: mock_pages)) - + mock_page = instance_double(PDF::Reader::Page, text: 'Sample PDF content') + mock_pages = [mock_page] + mock_reader = instance_double(PDF::Reader, pages: mock_pages) + allow(PDF::Reader).to receive(:open).and_yield(mock_reader) + result = service.perform expect(result[:success]).to be true end @@ -102,19 +102,17 @@ RSpec.describe Captain::Tools::PdfExtractionService do let(:pdf_source) { sample_pdf_path.to_s } before do - unless File.exist?(sample_pdf_path) - skip 'Sample PDF file not available for testing' - end + skip 'Sample PDF file not available for testing' unless File.exist?(sample_pdf_path) end it 'handles text extraction gracefully' do expect { service.extract_text }.not_to raise_error(NoMethodError) - + # Should either succeed or raise a PDF::Reader error that gets caught begin extracted_content = service.extract_text expect(extracted_content).to be_an(Array) - + if extracted_content.any? page_content = extracted_content.first expect(page_content).to have_key(:page_number) @@ -122,7 +120,7 @@ RSpec.describe Captain::Tools::PdfExtractionService do end rescue PDF::Reader::MalformedPDFError # This is expected for malformed PDFs - expect(true).to be true + # Test passes if we reach this point end end end @@ -134,10 +132,10 @@ RSpec.describe Captain::Tools::PdfExtractionService do it 'cleans text content properly' do dirty_text = " \f Some text with \r\n line breaks \n\n\n and extra spaces " cleaned = service.send(:clean_text, dirty_text) - + expect(cleaned).not_to include("\f") expect(cleaned).not_to include("\r") - expect(cleaned).to include("Some text with") + expect(cleaned).to include('Some text with') expect(cleaned.strip).to eq(cleaned) end @@ -157,20 +155,20 @@ RSpec.describe Captain::Tools::PdfExtractionService do it 'chunks content appropriately' do chunks = service.send(:chunk_content, page_contents, max_chunk_size: 1000) - + expect(chunks.length).to be >= 2 # Should have at least 2 chunks - + # Check first chunk (short content) first_chunk = chunks.first expect(first_chunk[:page_number]).to eq(1) expect(first_chunk[:total_chunks]).to eq(1) - + # Check that long content was processed long_content_chunks = chunks.select { |c| c[:page_number] == 2 } expect(long_content_chunks.length).to be >= 1 - + # Verify long content was split appropriately - total_long_content_length = long_content_chunks.map { |c| c[:content].length }.sum + total_long_content_length = long_content_chunks.sum { |c| c[:content].length } expect(total_long_content_length).to be > 0 end end @@ -179,16 +177,16 @@ RSpec.describe Captain::Tools::PdfExtractionService do it 'splits content by paragraphs first' do content = "First paragraph.\n\nSecond paragraph.\n\nThird paragraph." chunks = service.send(:split_content_into_chunks, content, 50) - + expect(chunks.length).to be >= 2 expect(chunks.join(' ')).to include('First paragraph') expect(chunks.join(' ')).to include('Second paragraph') end it 'splits by sentences when paragraphs are too large' do - long_paragraph = 'A' * 100 + '. ' + 'B' * 100 + '. ' + 'C' * 100 + '.' + long_paragraph = "#{('A' * 100)}. #{('B' * 100)}. #{('C' * 100)}." chunks = service.send(:split_content_into_chunks, long_paragraph, 150) - + expect(chunks.length).to be > 1 end end @@ -197,9 +195,10 @@ RSpec.describe Captain::Tools::PdfExtractionService do describe 'file validation' do context 'with uploaded file object' do let(:uploaded_file) do - double( - 'uploaded_file', - tempfile: double(path: sample_pdf_path.to_s), + temp_file = instance_double(Tempfile, path: sample_pdf_path.to_s) + instance_double( + ActionDispatch::Http::UploadedFile, + tempfile: temp_file, content_type: 'application/pdf', size: 1024 ) @@ -207,31 +206,33 @@ RSpec.describe Captain::Tools::PdfExtractionService do let(:pdf_source) { uploaded_file } it 'validates uploaded file properties' do - expect { service.send(:validate_pdf_source) }.not_to raise_error + expect { service.send(:validate_uploaded_file) }.not_to raise_error end end context 'with oversized file' do let(:uploaded_file) do - double( - 'uploaded_file', - tempfile: double(path: sample_pdf_path.to_s), + temp_file = instance_double(Tempfile, path: sample_pdf_path.to_s) + instance_double( + ActionDispatch::Http::UploadedFile, + tempfile: temp_file, content_type: 'application/pdf', - size: 11.megabytes + size: 30.megabytes ) end let(:pdf_source) { uploaded_file } it 'raises error for oversized file' do - expect { service.send(:validate_pdf_source) }.to raise_error('File too large') + expect { service.send(:validate_uploaded_file) }.to raise_error(/File too large/) end end context 'with invalid content type' do let(:uploaded_file) do - double( - 'uploaded_file', - tempfile: double(path: sample_pdf_path.to_s), + temp_file = instance_double(Tempfile, path: sample_pdf_path.to_s) + instance_double( + ActionDispatch::Http::UploadedFile, + tempfile: temp_file, content_type: 'text/plain', size: 1024 ) @@ -239,7 +240,7 @@ RSpec.describe Captain::Tools::PdfExtractionService do let(:pdf_source) { uploaded_file } it 'raises error for invalid content type' do - expect { service.send(:validate_pdf_source) }.to raise_error('Invalid file type') + expect { service.send(:validate_uploaded_file) }.to raise_error('Invalid file type') end end end