diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 86aa2218..18bc3e1f 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -63,7 +63,7 @@ jobs: run: bundle exec rspec spec/features - name: Keep screenshots from failed system tests - uses: actions/upload-artifact@v4 + uses: actions/upload-artifact@v5 if: failure() with: name: screenshots diff --git a/Gemfile b/Gemfile index 730e2c08..ee78bfe4 100644 --- a/Gemfile +++ b/Gemfile @@ -1,6 +1,6 @@ source "https://rubygems.org" -gem "rails", "~> 8.0.3" +gem "rails", "~> 8.1.1" gem "propshaft" gem "pg" gem "puma", ">= 5.0" @@ -21,7 +21,7 @@ gem "solid_cache" gem "bootsnap", require: false gem "thruster", require: false -gem "view_component", "~> 4.0" +gem "view_component", "~> 4.1" gem "kaminari", "~> 1.2" gem "devise", "~> 4.9.4" @@ -53,11 +53,11 @@ group :test do gem "factory_bot_rails", "~> 6.5" gem "capybara" gem "capybara-email" - gem "selenium-webdriver", "~> 4.36" + gem "selenium-webdriver", "~> 4.38" end gem "bcrypt", "~> 3.1" -gem "aws-sdk-s3", "~> 1.200" # For S3 versioning support +gem "aws-sdk-s3", "~> 1.202" # For S3 versioning support gem "aws-sdk-secretsmanager" gem "aws-sdk-lambda" gem "aws-sigv4" diff --git a/Gemfile.lock b/Gemfile.lock index 27f0f394..d72e5919 100644 --- a/Gemfile.lock +++ b/Gemfile.lock @@ -1,29 +1,31 @@ GEM remote: https://rubygems.org/ specs: - actioncable (8.0.3) - actionpack (= 8.0.3) - activesupport (= 8.0.3) + action_text-trix (2.1.15) + railties + actioncable (8.1.1) + actionpack (= 8.1.1) + activesupport (= 8.1.1) nio4r (~> 2.0) websocket-driver (>= 0.6.1) zeitwerk (~> 2.6) - actionmailbox (8.0.3) - actionpack (= 8.0.3) - activejob (= 8.0.3) - activerecord (= 8.0.3) - activestorage (= 8.0.3) - activesupport (= 8.0.3) + actionmailbox (8.1.1) + actionpack (= 8.1.1) + activejob (= 8.1.1) + activerecord (= 8.1.1) + activestorage (= 8.1.1) + activesupport (= 8.1.1) mail (>= 2.8.0) - actionmailer (8.0.3) - actionpack (= 8.0.3) - actionview (= 8.0.3) - activejob (= 8.0.3) - activesupport (= 8.0.3) + actionmailer (8.1.1) + actionpack (= 8.1.1) + actionview (= 8.1.1) + activejob (= 8.1.1) + activesupport (= 8.1.1) mail (>= 2.8.0) rails-dom-testing (~> 2.2) - actionpack (8.0.3) - actionview (= 8.0.3) - activesupport (= 8.0.3) + actionpack (8.1.1) + actionview (= 8.1.1) + activesupport (= 8.1.1) nokogiri (>= 1.8.5) rack (>= 2.2.4) rack-session (>= 1.0.1) @@ -31,27 +33,28 @@ GEM rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) useragent (~> 0.16) - actiontext (8.0.3) - actionpack (= 8.0.3) - activerecord (= 8.0.3) - activestorage (= 8.0.3) - activesupport (= 8.0.3) + actiontext (8.1.1) + action_text-trix (~> 2.1.15) + actionpack (= 8.1.1) + activerecord (= 8.1.1) + activestorage (= 8.1.1) + activesupport (= 8.1.1) globalid (>= 0.6.0) nokogiri (>= 1.8.5) - actionview (8.0.3) - activesupport (= 8.0.3) + actionview (8.1.1) + activesupport (= 8.1.1) builder (~> 3.1) erubi (~> 1.11) rails-dom-testing (~> 2.2) rails-html-sanitizer (~> 1.6) - activejob (8.0.3) - activesupport (= 8.0.3) + activejob (8.1.1) + activesupport (= 8.1.1) globalid (>= 0.3.6) - activemodel (8.0.3) - activesupport (= 8.0.3) - activerecord (8.0.3) - activemodel (= 8.0.3) - activesupport (= 8.0.3) + activemodel (8.1.1) + activesupport (= 8.1.1) + activerecord (8.1.1) + activemodel (= 8.1.1) + activesupport (= 8.1.1) timeout (>= 0.4.0) activerecord-session_store (2.2.0) actionpack (>= 7.0) @@ -59,20 +62,20 @@ GEM cgi (>= 0.3.6) rack (>= 2.0.8, < 4) railties (>= 7.0) - activestorage (8.0.3) - actionpack (= 8.0.3) - activejob (= 8.0.3) - activerecord (= 8.0.3) - activesupport (= 8.0.3) + activestorage (8.1.1) + actionpack (= 8.1.1) + activejob (= 8.1.1) + activerecord (= 8.1.1) + activesupport (= 8.1.1) marcel (~> 1.0) - activesupport (8.0.3) + activesupport (8.1.1) base64 - benchmark (>= 0.3) bigdecimal concurrent-ruby (~> 1.0, >= 1.3.1) connection_pool (>= 2.2.5) drb i18n (>= 1.6, < 2) + json logger (>= 1.4.2) minitest (>= 5.1) securerandom (>= 0.3) @@ -82,8 +85,8 @@ GEM public_suffix (>= 2.0.2, < 7.0) ast (2.4.3) aws-eventstream (1.4.0) - aws-partitions (1.1173.0) - aws-sdk-core (3.233.0) + aws-partitions (1.1178.0) + aws-sdk-core (3.235.0) aws-eventstream (~> 1, >= 1.3.0) aws-partitions (~> 1, >= 1.992.0) aws-sigv4 (~> 1.9) @@ -91,27 +94,26 @@ GEM bigdecimal jmespath (~> 1, >= 1.6.1) logger - aws-sdk-kms (1.113.0) - aws-sdk-core (~> 3, >= 3.231.0) + aws-sdk-kms (1.115.0) + aws-sdk-core (~> 3, >= 3.234.0) aws-sigv4 (~> 1.5) - aws-sdk-lambda (1.161.0) - aws-sdk-core (~> 3, >= 3.231.0) + aws-sdk-lambda (1.164.0) + aws-sdk-core (~> 3, >= 3.234.0) aws-sigv4 (~> 1.5) - aws-sdk-s3 (1.200.0) - aws-sdk-core (~> 3, >= 3.231.0) + aws-sdk-s3 (1.202.0) + aws-sdk-core (~> 3, >= 3.234.0) aws-sdk-kms (~> 1) aws-sigv4 (~> 1.5) - aws-sdk-secretsmanager (1.120.0) - aws-sdk-core (~> 3, >= 3.231.0) + aws-sdk-secretsmanager (1.122.0) + aws-sdk-core (~> 3, >= 3.234.0) aws-sigv4 (~> 1.5) - aws-sdk-ses (1.90.0) - aws-sdk-core (~> 3, >= 3.231.0) + aws-sdk-ses (1.92.0) + aws-sdk-core (~> 3, >= 3.234.0) aws-sigv4 (~> 1.5) aws-sigv4 (1.12.1) aws-eventstream (~> 1, >= 1.0.2) base64 (0.3.0) bcrypt (3.1.20) - benchmark (0.4.1) better_errors (2.10.1) erubi (>= 1.0.0) rack (>= 0.9.0) @@ -148,7 +150,7 @@ GEM cssbundling-rails (1.4.3) railties (>= 6.0.0) csv (3.3.5) - date (3.4.1) + date (3.5.0) debug (1.11.0) irb (~> 1.10) reline (>= 0.3.8) @@ -184,7 +186,7 @@ GEM dry-inflector (~> 1.0) dry-logic (~> 1.4) zeitwerk (~> 2.6) - erb (5.0.3) + erb (5.1.3) erubi (1.13.1) factory_bot (6.5.5) activesupport (>= 6.1.0) @@ -219,7 +221,7 @@ GEM jmespath (1.6.2) jsbundling-rails (1.3.1) railties (>= 6.0.0) - json (2.13.2) + json (2.15.2) kaminari (1.2.2) activesupport (>= 4.1.0) kaminari-actionview (= 1.2.2) @@ -238,7 +240,8 @@ GEM loofah (2.24.1) crass (~> 1.0.2) nokogiri (>= 1.12.0) - mail (2.8.1) + mail (2.9.0) + logger mini_mime (>= 0.1.1) net-imap net-pop @@ -254,7 +257,7 @@ GEM ruby2_keywords (~> 0.0.1) mustermann-grape (1.1.0) mustermann (>= 1.0.0) - net-imap (0.5.10) + net-imap (0.5.12) date net-protocol net-pop (0.1.2) @@ -302,7 +305,7 @@ GEM pp (0.6.3) prettyprint prettyprint (0.2.0) - prism (1.4.0) + prism (1.6.0) propshaft (1.3.1) actionpack (>= 7.0.0) activesupport (>= 7.0.0) @@ -311,7 +314,7 @@ GEM date stringio public_suffix (6.0.2) - puma (7.0.4) + puma (7.1.0) nio4r (~> 2.0) racc (1.8.1) rack (3.2.3) @@ -322,20 +325,20 @@ GEM rack (>= 1.3) rackup (2.2.1) rack (>= 3) - rails (8.0.3) - actioncable (= 8.0.3) - actionmailbox (= 8.0.3) - actionmailer (= 8.0.3) - actionpack (= 8.0.3) - actiontext (= 8.0.3) - actionview (= 8.0.3) - activejob (= 8.0.3) - activemodel (= 8.0.3) - activerecord (= 8.0.3) - activestorage (= 8.0.3) - activesupport (= 8.0.3) + rails (8.1.1) + actioncable (= 8.1.1) + actionmailbox (= 8.1.1) + actionmailer (= 8.1.1) + actionpack (= 8.1.1) + actiontext (= 8.1.1) + actionview (= 8.1.1) + activejob (= 8.1.1) + activemodel (= 8.1.1) + activerecord (= 8.1.1) + activestorage (= 8.1.1) + activesupport (= 8.1.1) bundler (>= 1.15.0) - railties (= 8.0.3) + railties (= 8.1.1) rails-controller-testing (1.0.5) actionpack (>= 5.0.1.rc1) actionview (>= 5.0.1.rc1) @@ -347,9 +350,9 @@ GEM rails-html-sanitizer (1.6.2) loofah (~> 2.21) nokogiri (>= 1.15.7, != 1.16.7, != 1.16.6, != 1.16.5, != 1.16.4, != 1.16.3, != 1.16.2, != 1.16.1, != 1.16.0.rc1, != 1.16.0) - railties (8.0.3) - actionpack (= 8.0.3) - activesupport (= 8.0.3) + railties (8.1.1) + actionpack (= 8.1.1) + activesupport (= 8.1.1) irb (~> 1.13) rackup (>= 1.0.0) rake (>= 12.2) @@ -357,7 +360,7 @@ GEM tsort (>= 0.2) zeitwerk (~> 2.6) rainbow (3.1.1) - rake (13.3.0) + rake (13.3.1) rbs (3.9.4) logger rdoc (6.15.0) @@ -374,16 +377,16 @@ GEM railties (>= 5.2) rexml (3.4.4) rouge (4.6.0) - rspec (3.13.1) + rspec (3.13.2) rspec-core (~> 3.13.0) rspec-expectations (~> 3.13.0) rspec-mocks (~> 3.13.0) - rspec-core (3.13.5) + rspec-core (3.13.6) rspec-support (~> 3.13.0) rspec-expectations (3.13.5) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) - rspec-mocks (3.13.5) + rspec-mocks (3.13.6) diff-lcs (>= 1.2.0, < 2.0) rspec-support (~> 3.13.0) rspec-rails (8.0.2) @@ -394,7 +397,7 @@ GEM rspec-expectations (~> 3.13) rspec-mocks (~> 3.13) rspec-support (~> 3.13) - rspec-support (3.13.5) + rspec-support (3.13.6) rubocop (1.80.2) json (~> 2.3) language_server-protocol (~> 3.17.0.2) @@ -421,11 +424,9 @@ GEM ruby2_keywords (0.0.5) rubyzip (3.2.0) securerandom (0.4.1) - selenium-webdriver (4.36.0) + selenium-webdriver (4.38.0) base64 (~> 0.2) - json (<= 2.13.2) logger (~> 1.4) - prism (~> 1.0, < 1.5) rexml (~> 3.2, >= 3.2.5) rubyzip (>= 1.2.2, < 4.0) websocket (~> 1.0) @@ -474,7 +475,7 @@ GEM thruster (0.1.15-arm64-darwin) thruster (0.1.15-x86_64-darwin) thruster (0.1.15-x86_64-linux) - timeout (0.4.3) + timeout (0.4.4) tsort (0.2.0) turbo-rails (2.0.17) actionpack (>= 7.1.0) @@ -486,8 +487,8 @@ GEM unicode-emoji (4.1.0) uri (1.0.4) useragent (0.16.11) - view_component (4.0.2) - activesupport (>= 7.1.0, < 8.1) + view_component (4.1.0) + activesupport (>= 7.1.0, < 8.2) concurrent-ruby (~> 1) warden (1.2.9) rack (>= 2.0.9) @@ -520,7 +521,7 @@ PLATFORMS DEPENDENCIES activerecord-session_store aws-sdk-lambda - aws-sdk-s3 (~> 1.200) + aws-sdk-s3 (~> 1.202) aws-sdk-secretsmanager aws-sdk-ses aws-sigv4 @@ -549,13 +550,13 @@ DEPENDENCIES pg propshaft puma (>= 5.0) - rails (~> 8.0.3) + rails (~> 8.1.1) rails-controller-testing rspec (~> 3.13) rspec-rails (~> 8.0) ruby-lsp (~> 0.26) rubyzip - selenium-webdriver (~> 4.36) + selenium-webdriver (~> 4.38) shoulda (~> 4.0) shoulda-matchers (~> 4.5) simplecov @@ -567,7 +568,7 @@ DEPENDENCIES thruster turbo-rails tzinfo-data - view_component (~> 4.0) + view_component (~> 4.1) web-console BUNDLED WITH diff --git a/python_components/document_inference/document_inference/helpers.py b/python_components/document_inference/document_inference/helpers.py index df81c9b8..07f204d3 100644 --- a/python_components/document_inference/document_inference/helpers.py +++ b/python_components/document_inference/document_inference/helpers.py @@ -2,6 +2,7 @@ import logging import os import shutil +import time import boto3 import fitz @@ -21,6 +22,21 @@ _document_collection = {} +minimal_headers = { + "Content-Type": "application/pdf", + "Content-Disposition": "inline", +} + +browser_headers = { + "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/120.0.0.0 Safari/537.36", + "Accept": "application/pdf,application/octet-stream,*/*", + "Accept-Language": "en-US,en;q=0.9", + "Accept-Encoding": "gzip, deflate, br", + "Sec-Fetch-Dest": "document", + "Sec-Fetch-Mode": "navigate", + "Sec-Fetch-Site": "none", +} + def get_models(model_file: str): with open(model_file, "r") as f: @@ -46,23 +62,32 @@ def get_secret(secret_name: str, local_mode: bool, aws_env: str) -> str: return response["SecretString"] -def get_file(url: str, output_path: str) -> str: +def get_file(url: str, output_path: str, wait_to_retry: int = 1000) -> str: file_name = os.path.basename(url) local_path = f"{output_path}/{file_name}" - headers = { - "User-Agent": "Mozilla/5.0 (Windows NT 10.0; Win64; x64) AppleWebKit/537.36 (KHTML, like Gecko) Chrome/91.0.4472.124 Safari/537.36", - "Accept": "application/pdf,application/octet-stream,*/*", - "Accept-Language": "en-US,en;q=0.9", - "Accept-Encoding": "gzip, deflate", - "DNT": "1", - "Connection": "keep-alive", - "Upgrade-Insecure-Requests": "1", - } - with requests.get(url, headers=headers, stream=True) as response: - response.raise_for_status() - with open(f"{output_path}/{file_name}", "wb") as file: - shutil.copyfileobj(response.raw, file) - return local_path + strategies = [ + {"headers": minimal_headers, "stream": True, "verify": True}, + {"headers": browser_headers, "stream": True, "verify": True}, + {"headers": minimal_headers, "stream": True, "verify": False}, + {"headers": browser_headers, "stream": True, "verify": False}, + ] + for strategy in strategies: + try: + with requests.get(url, **strategy) as response: + if response.status_code == 200: + with open(local_path, "wb") as file: + shutil.copyfileobj(response.raw, file) + return local_path + else: + raise RuntimeError( + f"Request failed with status code: {response.status_code}" + ) + except (RuntimeError, requests.exceptions.RequestException) as e: + logger.info( + f"Download failed: {str(e)} with strategy: {json.dumps(strategy)}" + ) + time.sleep(wait_to_retry) + raise RuntimeError("Failed to download file with all strategies.") def pdf_to_attachments( diff --git a/python_components/document_inference/requirements.txt b/python_components/document_inference/requirements.txt index f4edc34c..0f7f127d 100644 --- a/python_components/document_inference/requirements.txt +++ b/python_components/document_inference/requirements.txt @@ -1,12 +1,9 @@ awslambdaric==3.0.2 backports.tarfile==1.2.0 -black==25.1.0 boto3==1.37.10 document-inference==0.0.0 -flake8==7.1.2 importlib-metadata==8.0.0 inflect==7.3.1 -isort==6.0.1 jaraco-functools==4.3.0 jaraco.collections==5.1.0 llm-anthropic==0.18 diff --git a/python_components/document_inference/tests/test_file_downloads.py b/python_components/document_inference/tests/test_file_downloads.py index 252830de..ef388ffe 100644 --- a/python_components/document_inference/tests/test_file_downloads.py +++ b/python_components/document_inference/tests/test_file_downloads.py @@ -1,6 +1,7 @@ import os from document_inference.helpers import get_file +from pytest import raises from pytest_httpserver import HTTPServer from werkzeug import Request, Response @@ -33,30 +34,6 @@ def handler(request: Request): _assert_file_contents("/tmp/test.pdf", "Great pdf content!") -def test_header_assertion(httpserver: HTTPServer): - def handler(request: Request): - try: - headers_as_text = str(request.headers) - assert ( - "python" not in headers_as_text - ), f"Headers contain 'python': {headers_as_text}" - assert ( - "urllib" not in headers_as_text - ), f"Headers contain 'urllib': {headers_as_text}" - assert ( - "Mozilla" in headers_as_text - ), f"Headers missing 'Mozilla': {headers_as_text}" - return Response("Great pdf validated by headers content!", 200) - except AssertionError as e: - print(f"Assertion failed: {e}") - return Response(f"Assertion failed: {e}", 500) - - httpserver.expect_request("/test.pdf").respond_with_handler(handler) - _remove_file_if_exists("/tmp/test.pdf") - get_file(httpserver.url_for("/test.pdf"), "/tmp") - _assert_file_contents("/tmp/test.pdf", "Great pdf validated by headers content!") - - def test_308_redirect(httpserver: HTTPServer): def redirect_handler(request: Request): return Response( @@ -78,6 +55,55 @@ def final_handler(request: Request): _assert_file_contents("/tmp/original.pdf", "Great redirected content!") +def test_first_strategy_fails_second_succeeds(httpserver: HTTPServer): + call_count = {"count": 0} + + def handler(request: Request): + call_count["count"] += 1 + # First call (minimal headers) fails, second call (browser headers) succeeds + if call_count["count"] == 1: + return Response("Forbidden", 403) + else: + return Response("Success on second try!", 200) + + httpserver.expect_request("/test.pdf").respond_with_handler(handler) + _remove_file_if_exists("/tmp/test.pdf") + get_file(httpserver.url_for("/test.pdf"), "/tmp", wait_to_retry=0) + _assert_file_contents("/tmp/test.pdf", "Success on second try!") + assert call_count["count"] == 2, "Should have tried exactly 2 strategies" + + +def test_multiple_strategies_fail_then_succeed(httpserver: HTTPServer): + call_count = {"count": 0} + + def handler(request: Request): + call_count["count"] += 1 + # First 3 calls fail, 4th succeeds + if call_count["count"] < 4: + return Response("Forbidden", 403) + else: + return Response("Success on fourth try!", 200) + + httpserver.expect_request("/test.pdf").respond_with_handler(handler) + _remove_file_if_exists("/tmp/test.pdf") + get_file(httpserver.url_for("/test.pdf"), "/tmp", wait_to_retry=0) + _assert_file_contents("/tmp/test.pdf", "Success on fourth try!") + assert call_count["count"] == 4, "Should have tried exactly 4 strategies" + + +def test_all_strategies_fail(httpserver: HTTPServer): + """Test that when all strategies fail, we raise an exception.""" + + def handler(request: Request): + return Response("Forbidden", 403) + + httpserver.expect_request("/test.pdf").respond_with_handler(handler) + _remove_file_if_exists("/tmp/test.pdf") + + with raises(RuntimeError, match="Failed to download file with all strategies"): + get_file(httpserver.url_for("/test.pdf"), "/tmp", wait_to_retry=0) + + def _remove_file_if_exists(path: str): if os.path.exists(path): os.remove(path)