Skip to content

Commit 607b984

Browse files
committed
Merge branch 'fix/codecov-ci-configuration' into main
Fix CI configuration and integration test strategy: - Fix Codecov action parameter (file → files) - Enhance integration test conditions to run on push events, same-repo PRs, and manual triggers - Add automated feedback for fork PRs explaining security limitations - Fix test data issues by adding multi-page PDF - Improve file handling with better position management - Add validation improvements for Direct API methods This ensures integration tests run properly while maintaining security for fork PRs.
2 parents 3ce1ca3 + 0804070 commit 607b984

File tree

15 files changed

+2720
-175
lines changed

15 files changed

+2720
-175
lines changed

.github/workflows/ci.yml

Lines changed: 95 additions & 6 deletions
Original file line numberDiff line numberDiff line change
@@ -1,10 +1,20 @@
11
name: CI
22

3+
# Integration Test Strategy:
4+
# - Fork PRs: Cannot access secrets, so integration tests are skipped with informative feedback
5+
# - Same-repo PRs: Have access to secrets, integration tests run normally
6+
# - Push to main/develop: Integration tests always run to catch any issues after merge
7+
# - Manual trigger: Allows maintainers to run integration tests on demand
8+
#
9+
# This ensures security while still validating integration tests before release
10+
311
on:
412
push:
513
branches: [ main, develop ]
614
pull_request:
715
branches: [ main, develop ]
16+
# Run integration tests after PR is merged
17+
workflow_dispatch: # Allow manual trigger for integration tests
818

919
jobs:
1020
test:
@@ -47,25 +57,32 @@ jobs:
4757
run: python -m pytest tests/unit/ -v --cov=nutrient_dws --cov-report=xml --cov-report=term
4858

4959
- name: Upload coverage to Codecov
50-
uses: codecov/codecov-action@v4
60+
uses: codecov/codecov-action@v5
5161
with:
5262
token: ${{ secrets.CODECOV_TOKEN }}
53-
file: ./coverage.xml
63+
files: ./coverage.xml
5464
flags: unittests
5565
name: codecov-umbrella
5666
fail_ci_if_error: false
5767

5868
integration-test:
5969
runs-on: ubuntu-latest
60-
if: github.event_name == 'pull_request'
70+
# Run on: pushes to main/develop, PRs from same repo, and manual triggers
71+
if: |
72+
github.event_name == 'push' ||
73+
github.event_name == 'workflow_dispatch' ||
74+
(github.event_name == 'pull_request' && github.event.pull_request.head.repo.full_name == github.repository)
75+
strategy:
76+
matrix:
77+
python-version: ['3.8', '3.9', '3.10', '3.11', '3.12']
6178

6279
steps:
6380
- uses: actions/checkout@v4
6481

65-
- name: Set up Python 3.12
82+
- name: Set up Python ${{ matrix.python-version }}
6683
uses: actions/setup-python@v5
6784
with:
68-
python-version: '3.12'
85+
python-version: ${{ matrix.python-version }}
6986

7087
- name: Cache pip dependencies
7188
uses: actions/cache@v4
@@ -80,7 +97,29 @@ jobs:
8097
python -m pip install --upgrade pip
8198
pip install -e ".[dev]"
8299
100+
- name: Check for API key availability
101+
run: |
102+
if [ -z "${{ secrets.NUTRIENT_DWS_API_KEY }}" ]; then
103+
echo "::warning::NUTRIENT_DWS_API_KEY secret not found, skipping integration tests"
104+
echo "skip_tests=true" >> $GITHUB_ENV
105+
106+
# Provide context about why this might be happening
107+
if [ "${{ github.event_name }}" == "pull_request" ]; then
108+
if [ "${{ github.event.pull_request.head.repo.full_name }}" != "${{ github.repository }}" ]; then
109+
echo "::notice::This appears to be a PR from a fork. Secrets are not available for security reasons."
110+
else
111+
echo "::error::This is a PR from the same repository but the API key is missing. Please check repository secrets configuration."
112+
fi
113+
else
114+
echo "::error::Running on ${{ github.event_name }} event but API key is missing. Please configure NUTRIENT_DWS_API_KEY secret."
115+
fi
116+
else
117+
echo "::notice::API key found, integration tests will run"
118+
echo "skip_tests=false" >> $GITHUB_ENV
119+
fi
120+
83121
- name: Create integration config with API key
122+
if: env.skip_tests != 'true'
84123
run: |
85124
python -c "
86125
import os
@@ -91,8 +130,58 @@ jobs:
91130
NUTRIENT_DWS_API_KEY: ${{ secrets.NUTRIENT_DWS_API_KEY }}
92131

93132
- name: Run integration tests
133+
if: env.skip_tests != 'true'
94134
run: python -m pytest tests/integration/ -v
95135

136+
- name: Cleanup integration config
137+
if: always()
138+
run: rm -f tests/integration/integration_config.py
139+
140+
# Provide feedback for fork PRs where integration tests can't run
141+
integration-test-fork-feedback:
142+
runs-on: ubuntu-latest
143+
if: |
144+
github.event_name == 'pull_request' &&
145+
github.event.pull_request.head.repo.full_name != github.repository
146+
steps:
147+
- name: Comment on PR about integration tests
148+
uses: actions/github-script@v7
149+
with:
150+
github-token: ${{ secrets.GITHUB_TOKEN }}
151+
script: |
152+
const issue_number = context.issue.number;
153+
const owner = context.repo.owner;
154+
const repo = context.repo.repo;
155+
156+
// Check if we've already commented
157+
const comments = await github.rest.issues.listComments({
158+
owner,
159+
repo,
160+
issue_number,
161+
});
162+
163+
const botComment = comments.data.find(comment =>
164+
comment.user.type === 'Bot' &&
165+
comment.body.includes('Integration tests are skipped for pull requests from forks')
166+
);
167+
168+
if (!botComment) {
169+
await github.rest.issues.createComment({
170+
owner,
171+
repo,
172+
issue_number,
173+
body: `## Integration Tests Status\n\n` +
174+
`Integration tests are skipped for pull requests from forks due to security restrictions. ` +
175+
`These tests will run automatically after the PR is merged.\n\n` +
176+
`**What this means:**\n` +
177+
`- Unit tests, linting, and type checking have passed ✅\n` +
178+
`- Integration tests require API credentials that aren't available to fork PRs\n` +
179+
`- A maintainer will review your changes and merge if appropriate\n` +
180+
`- Integration tests will run on the main branch after merge\n\n` +
181+
`Thank you for your contribution! 🙏`
182+
});
183+
}
184+
96185
build:
97186
runs-on: ubuntu-latest
98187
needs: test
@@ -120,4 +209,4 @@ jobs:
120209
uses: actions/upload-artifact@v4
121210
with:
122211
name: dist
123-
path: dist/
212+
path: dist/

pyproject.toml

Lines changed: 3 additions & 4 deletions
Original file line numberDiff line numberDiff line change
@@ -22,7 +22,6 @@ keywords = ["nutrient", "pdf", "document", "processing", "api", "client"]
2222
classifiers = [
2323
"Development Status :: 4 - Beta",
2424
"Intended Audience :: Developers",
25-
"License :: OSI Approved :: MIT License",
2625
"Operating System :: OS Independent",
2726
"Programming Language :: Python :: 3",
2827
"Programming Language :: Python :: 3.10",
@@ -53,10 +52,10 @@ docs = [
5352
]
5453

5554
[project.urls]
56-
Homepage = "https://github.com/jdrhyne/nutrient-dws-client-python"
55+
Homepage = "https://github.com/PSPDFKit/nutrient-dws-client-python"
5756
Documentation = "https://nutrient-dws-client-python.readthedocs.io"
58-
Repository = "https://github.com/jdrhyne/nutrient-dws-client-python"
59-
"Bug Tracker" = "https://github.com/jdrhyne/nutrient-dws-client-python/issues"
57+
Repository = "https://github.com/PSPDFKit/nutrient-dws-client-python"
58+
"Bug Tracker" = "https://github.com/PSPDFKit/nutrient-dws-client-python/issues"
6059

6160
[tool.setuptools.package-data]
6261
nutrient_dws = ["py.typed"]

src/nutrient_dws/api/direct.py

Lines changed: 16 additions & 46 deletions
Original file line numberDiff line numberDiff line change
@@ -414,8 +414,9 @@ def delete_pdf_pages(
414414
415415
Args:
416416
input_file: Input PDF file.
417-
page_indexes: List of page indexes to delete (0-based).
418-
Negative indexes are supported (-1 for last page).
417+
page_indexes: List of page indexes to delete (0-based). 0 = first page.
418+
Must be unique, sorted in ascending order.
419+
Negative indexes are NOT supported.
419420
output_path: Optional path to save the output file.
420421
421422
Returns:
@@ -424,13 +425,13 @@ def delete_pdf_pages(
424425
Raises:
425426
AuthenticationError: If API key is missing or invalid.
426427
APIError: For other API errors.
427-
ValueError: If page_indexes is empty.
428+
ValueError: If page_indexes is empty or contains negative indexes.
428429
429430
Examples:
430-
# Delete first and last pages
431+
# Delete first and last pages (Note: negative indexes not supported)
431432
result = client.delete_pdf_pages(
432433
"document.pdf",
433-
page_indexes=[0, -1]
434+
page_indexes=[0, 2] # Delete pages 1 and 3
434435
)
435436
436437
# Delete specific pages (2nd and 4th pages)
@@ -452,13 +453,17 @@ def delete_pdf_pages(
452453
if not page_indexes:
453454
raise ValueError("page_indexes cannot be empty")
454455

456+
# Check for negative indexes
457+
if any(idx < 0 for idx in page_indexes):
458+
negative_indexes = [idx for idx in page_indexes if idx < 0]
459+
raise ValueError(
460+
f"Negative page indexes not yet supported for deletion: {negative_indexes}"
461+
)
462+
455463
# Prepare file for upload
456464
file_field, file_data = prepare_file_for_upload(input_file, "file")
457465
files = {file_field: file_data}
458466

459-
# Convert negative indexes to positive (we need to get document info first)
460-
# For now, we'll create the parts structure and let the API handle negative indexes
461-
462467
# Sort page indexes to handle ranges efficiently
463468
sorted_indexes = sorted(set(page_indexes)) # Remove duplicates and sort
464469

@@ -470,13 +475,6 @@ def delete_pdf_pages(
470475
current_page = 0
471476

472477
for delete_index in sorted_indexes:
473-
# Handle negative indexes by letting API process them
474-
if delete_index < 0:
475-
# For negative indexes, we can't easily calculate ranges without knowing total pages
476-
# We'll use a different approach - create parts for everything and let API handle it
477-
# This is a simplified approach that may need refinement
478-
continue
479-
480478
# Add range from current_page to delete_index (exclusive)
481479
if current_page < delete_index:
482480
parts.append(
@@ -487,39 +485,9 @@ def delete_pdf_pages(
487485
current_page = delete_index + 1
488486

489487
# Add remaining pages from current_page to end
490-
if current_page >= 0: # Always add remaining pages unless we handled negative indexes
488+
if current_page >= 0: # Always add remaining pages
491489
parts.append({"file": "file", "pages": {"start": current_page}})
492490

493-
# Handle case where we have negative indexes - use a simpler approach
494-
if any(idx < 0 for idx in page_indexes):
495-
# If we have negative indexes, we need a different strategy
496-
# For now, we'll create a request that includes all positive ranges
497-
# and excludes negative ones - this is a limitation that would need
498-
# API documentation clarification
499-
parts = []
500-
501-
# Positive indexes only for now
502-
positive_indexes = [idx for idx in sorted_indexes if idx >= 0]
503-
if positive_indexes:
504-
current_page = 0
505-
for delete_index in positive_indexes:
506-
if current_page < delete_index:
507-
parts.append(
508-
{"file": "file", "pages": {"start": current_page, "end": delete_index}}
509-
)
510-
current_page = delete_index + 1
511-
512-
# Add remaining pages
513-
parts.append({"file": "file", "pages": {"start": current_page}})
514-
515-
# Handle negative indexes separately by including a warning
516-
if any(idx < 0 for idx in page_indexes):
517-
# For now, raise an error for negative indexes as they need special handling
518-
negative_indexes = [idx for idx in page_indexes if idx < 0]
519-
raise ValueError(
520-
f"Negative page indexes not yet supported for deletion: {negative_indexes}"
521-
)
522-
523491
# If no parts (edge case), raise error
524492
if not parts:
525493
raise ValueError("No valid pages to keep after deletion")
@@ -667,6 +635,8 @@ def add_page(
667635
# Validate inputs
668636
if page_count < 1:
669637
raise ValueError("page_count must be at least 1")
638+
if page_count > 100:
639+
raise ValueError("page_count cannot exceed 100 pages")
670640
if insert_index < -1:
671641
raise ValueError("insert_index must be -1 (for end) or a non-negative insertion index")
672642

src/nutrient_dws/file_handler.py

Lines changed: 29 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,5 +1,6 @@
11
"""File handling utilities for input/output operations."""
22

3+
import contextlib
34
import io
45
import os
56
from collections.abc import Generator
@@ -40,11 +41,30 @@ def prepare_file_input(file_input: FileInput) -> tuple[bytes, str]:
4041
return file_input, "document"
4142
case _ if hasattr(file_input, "read"):
4243
# Handle file-like objects
44+
# Save current position if seekable
45+
current_pos = None
46+
if hasattr(file_input, "seek") and hasattr(file_input, "tell"):
47+
try:
48+
current_pos = file_input.tell()
49+
file_input.seek(0) # Read from beginning
50+
except (OSError, io.UnsupportedOperation):
51+
pass
52+
4353
content = file_input.read()
4454
if isinstance(content, str):
4555
content = content.encode()
56+
57+
# Restore position if we saved it
58+
if current_pos is not None:
59+
with contextlib.suppress(OSError, io.UnsupportedOperation):
60+
file_input.seek(current_pos)
61+
4662
filename = getattr(file_input, "name", "document")
47-
if hasattr(filename, "__fspath__") or isinstance(filename, str | bytes):
63+
if hasattr(filename, "__fspath__"):
64+
filename = os.path.basename(os.fspath(filename))
65+
elif isinstance(filename, bytes):
66+
filename = os.path.basename(filename.decode())
67+
elif isinstance(filename, str):
4868
filename = os.path.basename(filename)
4969
return content, str(filename)
5070
case _:
@@ -102,6 +122,10 @@ def prepare_file_for_upload(
102122
case _ if hasattr(file_input, "read"):
103123
filename = getattr(file_input, "name", "document")
104124
if hasattr(filename, "__fspath__"):
125+
filename = os.path.basename(os.fspath(filename))
126+
elif isinstance(filename, bytes):
127+
filename = os.path.basename(filename.decode())
128+
elif isinstance(filename, str):
105129
filename = os.path.basename(filename)
106130
return field_name, (str(filename), file_input, content_type) # type: ignore[return-value]
107131
case _:
@@ -158,7 +182,10 @@ def get_file_size(file_input: FileInput) -> int | None:
158182
Returns:
159183
File size in bytes, or None if size cannot be determined.
160184
"""
161-
if isinstance(file_input, str):
185+
if isinstance(file_input, Path):
186+
if file_input.exists():
187+
return file_input.stat().st_size
188+
elif isinstance(file_input, str):
162189
path = Path(file_input)
163190
if path.exists():
164191
return path.stat().st_size

0 commit comments

Comments
 (0)