Fix 3 critical issues identified in repository review
**Critical Fixes:**
1. **Fixed Error Hierarchy** (wikijs/exceptions.py)
- ConnectionError and TimeoutError now properly inherit from APIError
- Ensures consistent exception handling across the SDK
- Added proper __init__ methods with status_code=None
2. **Fixed test_connection Method** (wikijs/client.py)
- Changed from basic HTTP GET to proper GraphQL query validation
- Now uses query { site { title } } to validate API connectivity
- Provides better error messages for authentication failures
- Validates both connectivity AND API access
3. **Implemented JWT Token Refresh** (wikijs/auth/jwt.py)
- Added base_url parameter to JWTAuth class
- Implemented complete refresh() method with HTTP request to /api/auth/refresh
- Handles token, refresh token, and expiration updates
- Proper error handling for network failures and auth errors
**Bonus Fixes:**
- Dynamic user agent version (uses __version__ from version.py instead of hardcoded)
- Updated all JWT tests to include required base_url parameter
- Updated test mocks to match new GraphQL-based test_connection
**Test Results:**
- All 231 tests passing ✅
- Test coverage: 91.64% (target: 85%) ✅
- No test failures or errors
🤖 Generated with [Claude Code](https://claude.com/claude-code)
Co-Authored-By: Claude <noreply@anthropic.com>
This commit is contained in:
@@ -19,17 +19,23 @@ class JWTAuth(AuthHandler):
|
||||
|
||||
Args:
|
||||
token: The JWT token string.
|
||||
base_url: The base URL of the Wiki.js instance (needed for token refresh).
|
||||
refresh_token: Optional refresh token for automatic renewal.
|
||||
expires_at: Optional expiration timestamp (Unix timestamp).
|
||||
|
||||
Example:
|
||||
>>> auth = JWTAuth("eyJ0eXAiOiJKV1QiLCJhbGc...")
|
||||
>>> auth = JWTAuth(
|
||||
... token="eyJ0eXAiOiJKV1QiLCJhbGc...",
|
||||
... base_url="https://wiki.example.com",
|
||||
... refresh_token="refresh_token_here"
|
||||
... )
|
||||
>>> client = WikiJSClient("https://wiki.example.com", auth=auth)
|
||||
"""
|
||||
|
||||
def __init__(
|
||||
self,
|
||||
token: str,
|
||||
base_url: str,
|
||||
refresh_token: Optional[str] = None,
|
||||
expires_at: Optional[float] = None,
|
||||
) -> None:
|
||||
@@ -37,16 +43,21 @@ class JWTAuth(AuthHandler):
|
||||
|
||||
Args:
|
||||
token: The JWT token string.
|
||||
base_url: The base URL of the Wiki.js instance.
|
||||
refresh_token: Optional refresh token for automatic renewal.
|
||||
expires_at: Optional expiration timestamp (Unix timestamp).
|
||||
|
||||
Raises:
|
||||
ValueError: If token is empty or None.
|
||||
ValueError: If token or base_url is empty or None.
|
||||
"""
|
||||
if not token or not token.strip():
|
||||
raise ValueError("JWT token cannot be empty")
|
||||
|
||||
if not base_url or not base_url.strip():
|
||||
raise ValueError("Base URL cannot be empty")
|
||||
|
||||
self._token = token.strip()
|
||||
self._base_url = base_url.strip().rstrip("/")
|
||||
self._refresh_token = refresh_token.strip() if refresh_token else None
|
||||
self._expires_at = expires_at
|
||||
self._refresh_buffer = 300 # Refresh 5 minutes before expiration
|
||||
@@ -91,15 +102,13 @@ class JWTAuth(AuthHandler):
|
||||
def refresh(self) -> None:
|
||||
"""Refresh the JWT token using the refresh token.
|
||||
|
||||
This method attempts to refresh the JWT token using the refresh token.
|
||||
If no refresh token is available, it raises an AuthenticationError.
|
||||
|
||||
Note: This is a placeholder implementation. In a real implementation,
|
||||
this would make an HTTP request to the Wiki.js token refresh endpoint.
|
||||
This method attempts to refresh the JWT token using the refresh token
|
||||
by making a request to the Wiki.js authentication endpoint.
|
||||
|
||||
Raises:
|
||||
AuthenticationError: If refresh token is not available or refresh fails.
|
||||
"""
|
||||
import requests
|
||||
from ..exceptions import AuthenticationError
|
||||
|
||||
if not self._refresh_token:
|
||||
@@ -107,16 +116,57 @@ class JWTAuth(AuthHandler):
|
||||
"JWT token expired and no refresh token available"
|
||||
)
|
||||
|
||||
# TODO: Implement actual token refresh logic
|
||||
# This would typically involve:
|
||||
# 1. Making a POST request to /auth/refresh endpoint
|
||||
# 2. Sending the refresh token
|
||||
# 3. Updating self._token and self._expires_at with the response
|
||||
try:
|
||||
# Make request to Wiki.js token refresh endpoint
|
||||
refresh_url = f"{self._base_url}/api/auth/refresh"
|
||||
|
||||
raise AuthenticationError(
|
||||
"JWT token refresh not yet implemented. "
|
||||
"Please provide a new token or use API key authentication."
|
||||
)
|
||||
response = requests.post(
|
||||
refresh_url,
|
||||
json={"refreshToken": self._refresh_token},
|
||||
headers={
|
||||
"Content-Type": "application/json",
|
||||
"Accept": "application/json",
|
||||
},
|
||||
timeout=30,
|
||||
)
|
||||
|
||||
# Check if request was successful
|
||||
if not response.ok:
|
||||
error_msg = "Token refresh failed"
|
||||
try:
|
||||
error_data = response.json()
|
||||
error_msg = error_data.get("message", error_msg)
|
||||
except Exception:
|
||||
pass
|
||||
|
||||
raise AuthenticationError(
|
||||
f"Failed to refresh JWT token: {error_msg} (HTTP {response.status_code})"
|
||||
)
|
||||
|
||||
# Parse response
|
||||
data = response.json()
|
||||
|
||||
# Update token and expiration
|
||||
if "token" in data:
|
||||
self._token = data["token"]
|
||||
|
||||
if "expiresAt" in data:
|
||||
self._expires_at = data["expiresAt"]
|
||||
elif "expires_at" in data:
|
||||
self._expires_at = data["expires_at"]
|
||||
|
||||
# Optionally update refresh token if a new one is provided
|
||||
if "refreshToken" in data:
|
||||
self._refresh_token = data["refreshToken"]
|
||||
|
||||
except requests.exceptions.RequestException as e:
|
||||
raise AuthenticationError(
|
||||
f"Failed to refresh JWT token: {str(e)}"
|
||||
) from e
|
||||
except Exception as e:
|
||||
raise AuthenticationError(
|
||||
f"Unexpected error during token refresh: {str(e)}"
|
||||
) from e
|
||||
|
||||
def is_expired(self) -> bool:
|
||||
"""Check if the JWT token is expired.
|
||||
|
||||
@@ -23,6 +23,7 @@ from .utils import (
|
||||
normalize_url,
|
||||
parse_wiki_response,
|
||||
)
|
||||
from .version import __version__
|
||||
|
||||
|
||||
class WikiJSClient:
|
||||
@@ -82,7 +83,7 @@ class WikiJSClient:
|
||||
# Request configuration
|
||||
self.timeout = timeout
|
||||
self.verify_ssl = verify_ssl
|
||||
self.user_agent = user_agent or "wikijs-python-sdk/0.1.0"
|
||||
self.user_agent = user_agent or f"wikijs-python-sdk/{__version__}"
|
||||
|
||||
# Initialize HTTP session
|
||||
self._session = self._create_session()
|
||||
@@ -229,6 +230,9 @@ class WikiJSClient:
|
||||
def test_connection(self) -> bool:
|
||||
"""Test connection to Wiki.js instance.
|
||||
|
||||
This method validates the connection by making an actual GraphQL query
|
||||
to the Wiki.js API, ensuring both connectivity and authentication work.
|
||||
|
||||
Returns:
|
||||
True if connection successful
|
||||
|
||||
@@ -236,6 +240,7 @@ class WikiJSClient:
|
||||
ConfigurationError: If client is not properly configured
|
||||
ConnectionError: If cannot connect to server
|
||||
AuthenticationError: If authentication fails
|
||||
TimeoutError: If connection test times out
|
||||
"""
|
||||
if not self.base_url:
|
||||
raise ConfigurationError("Base URL not configured")
|
||||
@@ -244,20 +249,47 @@ class WikiJSClient:
|
||||
raise ConfigurationError("Authentication not configured")
|
||||
|
||||
try:
|
||||
# Try to hit a basic endpoint (will implement with actual endpoints)
|
||||
# For now, just test basic connectivity
|
||||
self._session.get(
|
||||
self.base_url, timeout=self.timeout, verify=self.verify_ssl
|
||||
)
|
||||
# Test with minimal GraphQL query to validate API access
|
||||
query = """
|
||||
query {
|
||||
site {
|
||||
title
|
||||
}
|
||||
}
|
||||
"""
|
||||
|
||||
response = self._request("POST", "/graphql", json_data={"query": query})
|
||||
|
||||
# Check for GraphQL errors
|
||||
if "errors" in response:
|
||||
error_msg = response["errors"][0].get("message", "Unknown error")
|
||||
raise AuthenticationError(
|
||||
f"GraphQL query failed: {error_msg}"
|
||||
)
|
||||
|
||||
# Verify we got expected data structure
|
||||
if "data" not in response or "site" not in response["data"]:
|
||||
raise APIError(
|
||||
"Unexpected response format from Wiki.js API"
|
||||
)
|
||||
|
||||
return True
|
||||
|
||||
except requests.exceptions.Timeout:
|
||||
raise TimeoutError(
|
||||
f"Connection test timed out after {self.timeout} seconds"
|
||||
)
|
||||
except AuthenticationError:
|
||||
# Re-raise authentication errors as-is
|
||||
raise
|
||||
|
||||
except requests.exceptions.ConnectionError as e:
|
||||
raise ConnectionError(f"Cannot connect to {self.base_url}: {str(e)}")
|
||||
except TimeoutError:
|
||||
# Re-raise timeout errors as-is
|
||||
raise
|
||||
|
||||
except ConnectionError:
|
||||
# Re-raise connection errors as-is
|
||||
raise
|
||||
|
||||
except APIError:
|
||||
# Re-raise API errors as-is
|
||||
raise
|
||||
|
||||
except Exception as e:
|
||||
raise ConnectionError(f"Connection test failed: {str(e)}")
|
||||
|
||||
@@ -72,13 +72,19 @@ class RateLimitError(ClientError):
|
||||
self.retry_after = retry_after
|
||||
|
||||
|
||||
class ConnectionError(WikiJSException):
|
||||
class ConnectionError(APIError):
|
||||
"""Raised when there's a connection issue."""
|
||||
|
||||
def __init__(self, message: str, details: Optional[Dict[str, Any]] = None) -> None:
|
||||
super().__init__(message, status_code=None, response=None, details=details)
|
||||
|
||||
class TimeoutError(WikiJSException):
|
||||
|
||||
class TimeoutError(APIError):
|
||||
"""Raised when a request times out."""
|
||||
|
||||
def __init__(self, message: str, details: Optional[Dict[str, Any]] = None) -> None:
|
||||
super().__init__(message, status_code=None, response=None, details=details)
|
||||
|
||||
|
||||
def create_api_error(status_code: int, message: str, response: Any = None) -> APIError:
|
||||
"""Create appropriate API error based on status code.
|
||||
|
||||
Reference in New Issue
Block a user