From 6869ea7ad06f1eccea2fd4c2706f29665eaa9b9c Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Fri, 5 Dec 2025 09:51:03 +0100 Subject: [PATCH 01/14] Docs: expand Phase 2 auth-roles spec --- specs/feature/feat-auth-layer-apply-roles.md | 246 +++++++++++-------- 1 file changed, 140 insertions(+), 106 deletions(-) diff --git a/specs/feature/feat-auth-layer-apply-roles.md b/specs/feature/feat-auth-layer-apply-roles.md index b834d21..fb33136 100644 --- a/specs/feature/feat-auth-layer-apply-roles.md +++ b/specs/feature/feat-auth-layer-apply-roles.md @@ -1,110 +1,147 @@ # Auth Roles — Phase 2 Proposal ## TL;DR -Add role-aware authorization (user vs admin). Use the existing role column on User, add `is_admin` helper and an `admin_required` decorator, enforce admin checks server-side on admin actions (approve/reject, deletes), add a small seed/CLI for creating an initial admin, update UI to surface admin controls, and add integration tests to lock the behavior. The plan keeps changes minimal and testable. - -## Steps (high level) -- Create feature branch: `git checkout -b feat/auth-roles`. -- Add `is_admin` helper to `user.py`. -- Add an `admin_required` decorator (new file `service/authorization.py` or add to `auth_service.py`). -- Protect admin-only endpoints (proposal approvals, analysis approve/delete, any management endpoints). -- Add a CLI/utility script to create the initial admin account (`scripts/create_admin.py`). -- Update templates to show/hide admin UI elements and add server-side checks for visibility only (do not rely on UI alone). -- Add tests: integration tests to verify admin can perform admin actions and normal users cannot (403 or redirect). -- Run full test suite and fix regressions; documentation and a short release checklist. - -## Files to change (suggested) -### Add/modify: -- `user.py` — add `is_admin` property. -- `service/authorization.py` (new) — implement `admin_required` decorator. -- `proposal_route.py` — add `@admin_required` to `approve_proposal` (and any admin actions). -- `analysis_route.py` — add `@admin_required` to approve/delete actions. -- `index.html` and relevant templates — show admin links conditionally (use `current_user.is_admin`). -- `scripts/create_admin.py` (new) — CLI to create admin user through `AuthService.register_user(..., role='admin')`. -- `tests/integration/test_auth_admin_flow.py` (new) — integration tests for admin flows. - -## Minimal code examples -### User.is_admin property (add to user.py): +Add server-side role-based authorization (user vs admin). Use the existing `role` column on `User`, add a convenient `is_admin` helper and an `admin_required` decorator, protect admin actions (approve/reject, deletes, management) at the controller level, seed an initial admin account via a small CLI helper, update templates to show admin UI elements, and add integration tests to prevent regressions. Keep changes minimal, well-tested and auditable. + +## Goals +- Enforce admin-only operations server-side (not just hidden UI elements). +- Provide a simple, auditable mechanism to create an initial admin locally. +- Keep the role model simple (string role) with easy migration path to more granular RBAC later. +- Add tests exercising admin vs normal user behavior. + +## Non-Goals +- Complex RBAC (permissions table) — defer to later if needed. +- UI redesign — add minimal UI changes only (navbar badges/links). + +## Implementation Steps (concrete) +1. Branch: + - `git checkout -b feat/auth-roles` +2. Domain: `model/entity/user.py` + - Add property: + ```python + @property + def is_admin(self) -> bool: + return (self.role or '').lower() == 'admin' + ``` +3. Authorization helper: new `service/authorization.py` + - Add `admin_required` decorator: + ```python + from functools import wraps + from flask import abort + from flask_login import current_user, login_required + + def admin_required(func): + @wraps(func) + @login_required + def wrapper(*args, **kwargs): + if not getattr(current_user, "is_admin", False): + abort(403) + return func(*args, **kwargs) + return wrapper + ``` +4. Protect server routes (examples) + - `route/proposal_route.py`: + - Add `from service.authorization import admin_required` + - Annotate: + ```python + @proposal_bp.route('/approve/', methods=['POST']) + @admin_required + def approve_proposal(proposal_id): + ... + ``` + - `route/analysis_route.py`: + - Protect `approve_analysis` and `delete_analysis`. +5. Templates: show admin UI (examples) + - `templates/index.html` (navbar): + ```jinja + {% if current_user.is_authenticated and current_user.is_admin %} + Admin: Proposals + {% endif %} + ``` + - Buttons for approve/delete: keep server enforcement and render only for admin: + ```jinja + {% if current_user.is_authenticated and current_user.is_admin %} +
+ {% endif %} + ``` +6. Seed CLI (new) `scripts/create_admin.py` — minimal safe script: + ```python + # scripts/create_admin.py + import argparse + from app import app + from service.auth_service import AuthService + + def main(): + p = argparse.ArgumentParser() + p.add_argument('--email', required=True) + p.add_argument('--password', required=True) + args = p.parse_args() + + AuthService(app).register_user(email=args.email, password=args.password, role='admin') + print(f"Admin user '{args.email}' created.") + + if __name__ == '__main__': + main() + ``` + - Run locally (do not commit secrets): `python scripts/create_admin.py --email admin@example.com --password 'Strong!'` +7. Tests (new) + - `tests/integration/test_auth_admin_flow.py` (skeleton): + - Create admin via `AuthService.register_user(..., role='admin')` or create via the repo directly in test DB. + - Log in with test client, POST to admin-only endpoints and assert success and DB side-effect. + - Log in as normal user and assert 403 on admin endpoints. + - Unit tests: + - `test_user_is_admin` (verify `is_admin` behavior). + - `test_admin_required_decorator` (use Flask test_request_context and a mocked `current_user` to assert abort). +8. Audit (optional) + - Consider a simple `admin_actions` table for future auditing. Optional for MVP. + +## File Summary (exact edits) +- `model/entity/user.py` — add `is_admin` property. +- `service/authorization.py` — new. +- `route/proposal_route.py` — import decorator + protect `approve_proposal`. +- `route/analysis_route.py` — protect `approve_analysis` and `delete_analysis`. +- `templates/index.html`, `templates/proposals.html`, `templates/analysis.html` — conditional UI. +- `scripts/create_admin.py` — new seed script. +- `tests/integration/test_auth_admin_flow.py` — new integration tests. + +## Example: Protecting Analysis endpoints +In `route/analysis_route.py`: ```python -@property -def is_admin(self) -> bool: - return (self.role or '').lower() == 'admin' -``` +from service.authorization import admin_required -### admin_required decorator (new service/authorization.py): -```python -from functools import wraps -from flask import abort -from flask_login import current_user, login_required - -def admin_required(func): - @wraps(func) - @login_required - def wrapper(*args, **kwargs): - if not getattr(current_user, "is_admin", False): - abort(403) - return func(*args, **kwargs) - return wrapper -``` +@analysis_bp.route('/approve/', methods=['POST']) +@admin_required +def approve_analysis(id: int): + ... -### Protecting an endpoint: -```python -@proposal_bp.route('/approve/', methods=['POST']) +@analysis_bp.route('/delete/', methods=['POST']) @admin_required -def approve_proposal(proposal_id): +def delete_analysis(id: int): ... ``` -## Database / migration notes -- No schema change required: role already exists on users. -- If you want a DB-level constraint or an enum later, create a migration step in `migrate_db`. -- Seeding admin can be done with a small script (preferred) rather than committing secrets in migrations. +## Testing notes and expected behavior +- Admin success: approve endpoint returns redirect and DB effect (proposal created / radio source saved). +- Normal user: call returns HTTP 403 (Abort), or custom error page — tests should expect 403. +- Test fixtures: use `tests/conftest.py` test DB and call `AuthService.register_user(..., role='admin')` or create user directly via repository + hashed password. -## Seed / CLI script (recommended) -Create `scripts/create_admin.py` to be run locally/once: +## Migration & DB notes +- No DB schema change required — `role` exists. +- If you later want DB-level constraints, add a migration in `migrate_db/`. -```bash -python scripts/create_admin.py --email admin@example.com --password 'StrongPass!' -``` - -The script should: -- Import app context or call `AuthService(app)` with test/production config. -- Call `AuthService.register_user(email, password, role='admin')`. -- Print the created user's id/email (do not echo password). - -## Tests to add -`tests/integration/test_auth_admin_flow.py`: -- Register a user with `role='admin'` using the service (not the public registration page) or the seed script in test. -- Log in via test client. -- POST to `/proposal/approve/` and assert redirect/status and DB effect (proposal converted to radio source). -- Repeat same action as a normal user and assert 403 (or appropriate denial). -- Add unit tests: - - `User.is_admin` correctness. - - `admin_required` returns 403 for non-admins. - -## UI changes -- Navbar: show an "Admin" area or admin links if `current_user.is_admin`. -- Buttons for approve/delete: continue to render only when `current_user.is_admin`, but server must enforce checks regardless. -- Keep UX minimal — a top-level "Admin" dropdown linking to panels for proposals/analysis is fine. - -## Security & behavior rules -- Server-side enforcement always required — do not rely on UI hiding. -- Use `abort(403)` for unauthorized admin attempts (tests should expect 403). -- Audit: consider adding a simple audit log for admin actions (table `admin_actions` with `user_id`, `action`, `target`, `timestamp`). Optional but recommended for production. +## Security considerations +- Do not expose admin UI as the only protection — always enforce on server. +- Seed script should be run locally and never commit credentials. Consider generating a one-time password printed to stdout and requiring rotation. +- Consider adding logging for admin actions (timestamp, user id, action, target). ## Rollout checklist -- Create feature branch `feat/auth-roles`. -- Implement `is_admin` and `admin_required`. -- Protect endpoints and add tests. -- Add `scripts/create_admin.py` and run locally to create initial admin. -- Update templates to surface admin UI. -- Run full test suite: `pytest -q`. -- Commit & push branch, open PR. -- Manual smoke test: create admin, login, try approve/reject/delete flows; verify normal user cannot. -- (Optional) Add migration if you later enforce DB-level constraints. - -## Commands & recommended workflow -Create branch and run tests locally: +1. Create branch `feat/auth-roles`. +2. Implement code changes and tests. +3. Run `pytest -q` locally, fix failures. +4. Add `scripts/create_admin.py` and create an initial admin locally to test admin flows. +5. Commit and push branch; open PR with description and test run output. + +## Commands & workflow ```bash git checkout -b feat/auth-roles # implement changes... @@ -113,18 +150,15 @@ python -m pip install -r requirements.txt pytest -q ``` -Create an admin locally using the seed script (after adding it): -```bash -python scripts/create_admin.py --email admin@local.test --password 'ReplaceMe!' -``` - ## Estimated effort - Basic implementation + tests: 2–4 hours. -- With audit logging + polished admin UI: 4–8 hours. - -## Optional future enhancements -- Granular RBAC (roles + permissions table). -- Admin UI pages for user management. -- Audit trail (DB table). -- Rate-limits / throttling for admin endpoints. -- Admin-only API keys and 2FA for admin accounts. \ No newline at end of file +- With audit logging + nicer admin UI: 4–8 hours. + +## Optional extensions +- Granular permissions table. +- Admin user management UI. +- 2FA for admin accounts. + +## Final notes +- Keep changes small and test-first. Add the seed script to your local `scripts/` folder and do not check passwords into source control. +- If you'd like, I can produce the skeleton files and patches for all file edits and the tests so you can apply them in one go. \ No newline at end of file From 0a508778845dc8e9d805510a9fb8361f0ec1d46c Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Fri, 5 Dec 2025 10:24:13 +0100 Subject: [PATCH 02/14] partially created new role stage. Tests needed to complete. Create admin user. --- model/entity/user.py | 4 ++++ route/analysis_route.py | 5 +++++ route/proposal_route.py | 8 +++++++- route/radio_source_route.py | 4 ++++ service/authorization.py | 13 +++++++++++++ templates/proposal_detail.html | 3 ++- 6 files changed, 35 insertions(+), 2 deletions(-) create mode 100644 service/authorization.py diff --git a/model/entity/user.py b/model/entity/user.py index 4ed54e2..02fd0a2 100644 --- a/model/entity/user.py +++ b/model/entity/user.py @@ -27,5 +27,9 @@ def is_authenticated(self) -> bool: def is_anonymous(self) -> bool: return False + @property + def is_admin(self) -> bool: + return (self.role or '').lower() == 'admin' + def __repr__(self): return f"" diff --git a/route/analysis_route.py b/route/analysis_route.py index 078ab5f..de98a17 100644 --- a/route/analysis_route.py +++ b/route/analysis_route.py @@ -5,6 +5,7 @@ from typing import List from flask import Blueprint, request, jsonify, render_template, redirect, url_for, flash +from flask_login import login_required from model.entity.stream_analysis import StreamAnalysis from model.dto.stream_analysis import StreamAnalysisResult from model.repository.stream_analysis_repository import StreamAnalysisRepository @@ -71,6 +72,8 @@ def index(): return render_template('analysis.html', streams=streams_from_db) +# only a registered user can analyze a stream and save as proposal +@login_required @analysis_bp.route('/analyze', methods=['POST']) def analyze_url(): """Analyze a stream URL and show results.""" @@ -108,6 +111,7 @@ def analyze_url(): return redirect(url_for('analysis.index')) +@login_required @analysis_bp.route('/approve/', methods=['POST']) def approve_analysis(id: int): """Approve an analysis and creates a proposal.""" @@ -125,6 +129,7 @@ def approve_analysis(id: int): return redirect(url_for('proposal.index')) +@login_required @analysis_bp.route('/delete/', methods=['POST']) def delete_analysis(id: int): """Delete an analysis.""" diff --git a/route/proposal_route.py b/route/proposal_route.py index 0a30dd0..d5b859e 100644 --- a/route/proposal_route.py +++ b/route/proposal_route.py @@ -5,6 +5,7 @@ from typing import List from flask import Blueprint, request, render_template, redirect, url_for, flash +from flask_login import login_required from model.entity.radio_source import RadioSource from model.repository.proposal_repository import ProposalRepository @@ -12,6 +13,7 @@ from model.entity.proposal import Proposal from model.dto.validation import ProposalUpdateRequest from service.proposal_service import ProposalService +from service.authorization import admin_required proposal_bp = Blueprint('proposal', __name__, url_prefix='/proposal') @@ -60,6 +62,8 @@ def index(): return render_template('proposals.html', proposals=proposals_from_db) +# only a registered use can propose a new radio source +@login_required @proposal_bp.route('/propose', methods=['GET', 'POST']) def propose(): """Handle proposal submission form.""" @@ -98,7 +102,7 @@ def propose(): return redirect(url_for('proposal.index')) - +@login_required @proposal_bp.route('/update/', methods=['GET', 'POST']) def proposal_detail(proposal_id): if request.method == 'POST': @@ -138,6 +142,8 @@ def proposal_detail(proposal_id): return render_template('proposal_detail.html',proposal=proposal) +# only admin users can approve proposals +@admin_required @proposal_bp.route('/approve/', methods=['POST']) def approve_proposal(proposal_id): """Approve and convert proposal to radio source.""" diff --git a/route/radio_source_route.py b/route/radio_source_route.py index cb36cdb..a337c2b 100644 --- a/route/radio_source_route.py +++ b/route/radio_source_route.py @@ -7,6 +7,7 @@ from flask import Blueprint, request,render_template, redirect, url_for, flash from model.entity.stream_type import StreamType from model.repository.stream_type_repository import StreamTypeRepository +from service.authorization import admin_required from service.radio_source_service import RadioSourceService from model.repository.radio_source_repository import RadioSourceRepository from database import db @@ -40,6 +41,8 @@ def source_detail(source_id): return render_template('source_detail.html', source=source) +# only admin users can edit or delete radio sources +@admin_required @radio_source_bp.route('/edit/', methods=['GET', 'POST']) def edit_source(source_id): """Edit radio source.""" @@ -71,6 +74,7 @@ def edit_source(source_id): return render_template('edit_source.html', source=source, stream_types=stream_types) +@admin_required @radio_source_bp.route('/delete/', methods=['POST']) def delete_source(source_id): """Delete radio source.""" diff --git a/service/authorization.py b/service/authorization.py new file mode 100644 index 0000000..e3ab35f --- /dev/null +++ b/service/authorization.py @@ -0,0 +1,13 @@ +from functools import wraps +from flask import abort +from flask_login import current_user, login_required + +# decorator for role-based access control +def admin_required(func): + @wraps(func) + @login_required + def wrapper(*args, **kwargs): + if not getattr(current_user, "is_admin", False): + abort(403) + return func(*args, **kwargs) + return wrapper \ No newline at end of file diff --git a/templates/proposal_detail.html b/templates/proposal_detail.html index f2e7497..4862cd0 100644 --- a/templates/proposal_detail.html +++ b/templates/proposal_detail.html @@ -76,11 +76,12 @@
Edit Proposal
- + {% if current_user.is_authenticated %}
Return to Proposals
+ {% endif %} From 20e840534c37771d03621107e136436b73ddcd8e Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Fri, 5 Dec 2025 10:36:09 +0100 Subject: [PATCH 03/14] black formatted --- app.py | 16 +- database.py | 2 +- migrate_db/init_db.py | 1 + migrate_db/migrate.py | 30 +- model/dto/stream_analysis.py | 28 +- model/dto/stream_type.py | 9 +- model/dto/validation.py | 26 +- model/entity/proposal.py | 17 +- model/entity/radio_source.py | 15 +- model/entity/stream_analysis.py | 25 +- model/entity/stream_type.py | 17 +- model/entity/user.py | 10 +- model/repository/__init__.py | 2 +- model/repository/proposal_repository.py | 25 +- model/repository/radio_source_repository.py | 32 ++- .../repository/stream_analysis_repository.py | 41 +-- model/repository/stream_type_repository.py | 66 +++-- model/repository/user_repository.py | 6 +- route/analysis_route.py | 87 ++++-- route/auth_route.py | 100 ++++--- route/database_route.py | 66 +++-- route/listen_route.py | 3 +- route/main_route.py | 9 +- route/proposal_route.py | 87 +++--- route/radio_source_route.py | 62 ++-- scripts/smoke_auth_check.py | 14 +- service/__init__.py | 2 +- service/auth_service.py | 16 +- service/authorization.py | 4 +- service/proposal_service.py | 4 +- service/proposal_validation_service.py | 64 +++-- service/radio_source_service.py | 74 +++-- service/stream_analysis_service.py | 270 +++++++++++------- service/stream_type_service.py | 54 ++-- tests/conftest.py | 114 ++++++-- tests/integration/test_auth_flow.py | 46 +-- tests/integration/test_smoke_auth_pages.py | 20 +- .../test_validate_and_add_workflow.py | 36 ++- tests/unit/test_analysis_routes.py | 52 +++- tests/unit/test_auth_service.py | 2 +- tests/unit/test_proposal_update.py | 38 +-- .../unit/test_proposal_validation_service.py | 133 ++++++--- tests/unit/test_radio_source_service.py | 141 +++++---- tests/unit/test_stream_analysis_service.py | 152 +++++++--- tests/unit/test_stream_type_service.py | 73 ++++- 45 files changed, 1291 insertions(+), 800 deletions(-) diff --git a/app.py b/app.py index 8d377ed..0b7a834 100644 --- a/app.py +++ b/app.py @@ -4,20 +4,24 @@ app = Flask(__name__) # Configuration -app.config['SECRET_KEY'] = 'your-secret-key' # Change in production +app.config["SECRET_KEY"] = "your-secret-key" # Change in production basedir = os.path.abspath(os.path.dirname(__file__)) -instance_dir = os.path.join(basedir, 'instance') +instance_dir = os.path.join(basedir, "instance") os.makedirs(instance_dir, exist_ok=True) -app.config['SQLALCHEMY_DATABASE_URI'] = f'sqlite:///{os.path.join(instance_dir, "radio_sources.db")}' -app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False +app.config["SQLALCHEMY_DATABASE_URI"] = ( + f'sqlite:///{os.path.join(instance_dir, "radio_sources.db")}' +) +app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False # Import db from separate module from database import db + db.init_app(app) # Enable CSRF protection for forms try: from flask_wtf import CSRFProtect + csrf = CSRFProtect(app) except Exception: # If flask-wtf is not installed in the environment, app will still run @@ -46,5 +50,5 @@ # Db is created only by pyway migrations -if __name__ == '__main__': - app.run(debug=True) \ No newline at end of file +if __name__ == "__main__": + app.run(debug=True) diff --git a/database.py b/database.py index 3ea6804..a628dd1 100644 --- a/database.py +++ b/database.py @@ -4,4 +4,4 @@ from flask_sqlalchemy import SQLAlchemy -db = SQLAlchemy() \ No newline at end of file +db = SQLAlchemy() diff --git a/migrate_db/init_db.py b/migrate_db/init_db.py index 216b654..f80b780 100644 --- a/migrate_db/init_db.py +++ b/migrate_db/init_db.py @@ -9,6 +9,7 @@ DBPATH = Path(__file__).parent.parent / "instance" / "radio_sources.db" + def init_database(): """Initialize a fresh database with all migrations.""" print("🗄️ Initializing RadioChWeb database...") diff --git a/migrate_db/migrate.py b/migrate_db/migrate.py index d718686..d90d227 100644 --- a/migrate_db/migrate.py +++ b/migrate_db/migrate.py @@ -10,10 +10,13 @@ PYWAY_PATH = Path(__file__).parent.parent / ".venv" / "bin" / "pyway" + def run_command(cmd: list) -> bool: """Run a command and return success status.""" try: - result = subprocess.run(cmd, capture_output=True, text=True, cwd=Path(__file__).parent) + result = subprocess.run( + cmd, capture_output=True, text=True, cwd=Path(__file__).parent + ) if result.returncode != 0: print(f"❌ Command failed: {' '.join(cmd)}") print(f"Error: {result.stderr}") @@ -48,21 +51,26 @@ def run_migrations(): else: print("⚠️ pyway run failed, falling back to direct SQL application.") except Exception: - print("⚠️ pyway CLI not available or failed to run; falling back to direct SQL application.") + print( + "⚠️ pyway CLI not available or failed to run; falling back to direct SQL application." + ) # Fallback: apply SQL files directly import sqlite3 + db_path = Path("../instance/radio_sources.db") conn = sqlite3.connect(str(db_path)) cursor = conn.cursor() # Get list of migration files migration_dir = Path("migrations") - migration_files = sorted([f for f in migration_dir.iterdir() if f.is_file() and f.name.endswith('.sql')]) + migration_files = sorted( + [f for f in migration_dir.iterdir() if f.is_file() and f.name.endswith(".sql")] + ) for migration_file in migration_files: print(f"Applying migration: {migration_file.name}") - with open(migration_file, 'r') as f: + with open(migration_file, "r") as f: sql = f.read() cursor.executescript(sql) @@ -76,11 +84,7 @@ def show_migration_status(): """Show current migration status.""" print("\n📊 Migration Status:") - cmd = [ - PYWAY_PATH.as_posix(), - "--config", "pyway.yaml", - "info" - ] + cmd = [PYWAY_PATH.as_posix(), "--config", "pyway.yaml", "info"] run_command(cmd) @@ -89,11 +93,7 @@ def validate_migrations(): """Validate migration checksums.""" print("🔍 Validating migrations...") - cmd = [ - PYWAY_PATH.as_posix(), - "--config", "pyway.yaml", - "validate" - ] + cmd = [PYWAY_PATH.as_posix(), "--config", "pyway.yaml", "validate"] if run_command(cmd): print("✅ All migrations are valid!") @@ -113,4 +113,4 @@ def validate_migrations(): print("Usage: python migrate.py [status|validate]") sys.exit(1) else: - run_migrations() \ No newline at end of file + run_migrations() diff --git a/model/dto/stream_analysis.py b/model/dto/stream_analysis.py index ae487d8..27db0e5 100644 --- a/model/dto/stream_analysis.py +++ b/model/dto/stream_analysis.py @@ -25,14 +25,11 @@ class ErrorCode(str, Enum): class StreamAnalysisRequest(BaseModel): """Request DTO for stream analysis (spec 003).""" + url: HttpUrl timeout_seconds: int = 30 - model_config = ConfigDict( - json_encoders={ - HttpUrl: str - } - ) + model_config = ConfigDict(json_encoders={HttpUrl: str}) class StreamAnalysisResult(BaseModel): @@ -40,30 +37,35 @@ class StreamAnalysisResult(BaseModel): Data structure returned by analysis process (persisted for page proposal.html). This is the main return type from spec 003 analyze-and-classify process. """ + is_valid: bool is_secure: bool # False for HTTP, true for HTTPS stream_url: Optional[str] = None # if loaded is the url of proposal stream stream_type_id: Optional[int] = None # Foreign key to StreamType, null if invalid - stream_type_display_name: Optional[str] = None # Human-readable name of the stream type + stream_type_display_name: Optional[str] = ( + None # Human-readable name of the stream type + ) error_code: Optional[ErrorCode] = None # Null if valid detection_method: Optional[DetectionMethod] = None # How the stream was detected raw_content_type: Optional[str] = None # String from curl headers raw_ffmpeg_output: Optional[str] = None # String from ffmpeg detection - extracted_metadata: Optional[str] = None # Normalized metadata extracted from ffmpeg stderr - - @field_validator('extracted_metadata') + extracted_metadata: Optional[str] = ( + None # Normalized metadata extracted from ffmpeg stderr + ) + + @field_validator("extracted_metadata") def _clean_extracted_metadata(cls, v: Optional[str]) -> Optional[str]: if v is None: return None # remove control chars except newline and tab, trim, and enforce max length - cleaned = ''.join(ch for ch in v if (ch >= ' ' or ch in '\n\t')) + cleaned = "".join(ch for ch in v if (ch >= " " or ch in "\n\t")) cleaned = cleaned.strip() if len(cleaned) > 4096: cleaned = cleaned[:4096] return cleaned - + def is_success(self) -> bool: """Returns True if analysis was successful and stream is valid.""" return self.is_valid and self.error_code is None - - model_config = ConfigDict(from_attributes=True) \ No newline at end of file + + model_config = ConfigDict(from_attributes=True) diff --git a/model/dto/stream_type.py b/model/dto/stream_type.py index c4447b3..2fed7d1 100644 --- a/model/dto/stream_type.py +++ b/model/dto/stream_type.py @@ -7,15 +7,16 @@ class StreamTypeDTO(BaseModel): """DTO for StreamType entity.""" + id: int protocol: str # HTTP, HTTPS, HLS - format: str # MP3, AAC, OGG + format: str # MP3, AAC, OGG metadata: str # Icecast, Shoutcast, None (mapped from metadata_type) display_name: str - + @property def type_key(self) -> str: """Returns the type key in format: PROTOCOL-FORMAT-METADATA""" return f"{self.protocol}-{self.format}-{self.metadata}" - - model_config = ConfigDict(from_attributes=True) \ No newline at end of file + + model_config = ConfigDict(from_attributes=True) diff --git a/model/dto/validation.py b/model/dto/validation.py index 8462cf4..e1e92b1 100644 --- a/model/dto/validation.py +++ b/model/dto/validation.py @@ -15,6 +15,7 @@ class SecurityStatus(str, Enum): class ValidationResult(BaseModel): """Result of proposal validation.""" + is_valid: bool message: str = "" security_status: Optional[SecurityStatus] = None @@ -35,6 +36,7 @@ def add_warning(self, warning: str): class ProposalUpdateRequest(BaseModel): """Request DTO for updating proposal details.""" + name: Optional[str] = None website_url: Optional[str] = None country: Optional[str] = None @@ -45,17 +47,20 @@ class ProposalUpdateRequest(BaseModel): def has_updates(self) -> bool: """Check if any updates are provided.""" - return any([ - self.name is not None, - self.website_url is not None, - self.country is not None, - self.description is not None, - self.image is not None - ]) + return any( + [ + self.name is not None, + self.website_url is not None, + self.country is not None, + self.description is not None, + self.image is not None, + ] + ) class ProposalRequest(BaseModel): - """Data model for a proposal.""" + """Data model for a proposal.""" + id: int stream_url: str name: str @@ -65,7 +70,8 @@ class ProposalRequest(BaseModel): image_url: Optional[str] = None stream_type_id: int is_secure: bool - + model_config = ConfigDict(from_attributes=True) + def __repr__(self): - return f"" \ No newline at end of file + return f"" diff --git a/model/entity/proposal.py b/model/entity/proposal.py index aa9567f..67d9af8 100644 --- a/model/entity/proposal.py +++ b/model/entity/proposal.py @@ -1,18 +1,21 @@ from sqlalchemy import func from database import db + class Proposal(db.Model): - __tablename__ = 'proposals' + __tablename__ = "proposals" id = db.Column(db.Integer, primary_key=True, autoincrement=True) stream_url = db.Column(db.String(500), nullable=False, unique=True, index=True) name = db.Column(db.String(100), nullable=False) website_url = db.Column(db.String(500)) - + # Classification data from analysis - stream_type_id = db.Column(db.Integer, db.ForeignKey("stream_types.id"), nullable=False) + stream_type_id = db.Column( + db.Integer, db.ForeignKey("stream_types.id"), nullable=False + ) is_secure = db.Column(db.Boolean, nullable=False, default=False) - + # User-editable fields country = db.Column(db.String(50)) description = db.Column(db.Text) @@ -20,9 +23,9 @@ class Proposal(db.Model): # Timestamps created_at = db.Column(db.DateTime(timezone=True), server_default=func.now()) - + # Relationship stream_type = db.relationship("StreamType", back_populates="proposals") - + def __repr__(self): - return f"" \ No newline at end of file + return f"" diff --git a/model/entity/radio_source.py b/model/entity/radio_source.py index 790c343..177d745 100644 --- a/model/entity/radio_source.py +++ b/model/entity/radio_source.py @@ -1,23 +1,26 @@ from database import db from sqlalchemy.sql import func + class RadioSource(db.Model): - __tablename__ = 'radio_sources' + __tablename__ = "radio_sources" id = db.Column(db.Integer, primary_key=True, autoincrement=True) stream_url = db.Column(db.String(500), nullable=False, unique=True, index=True) name = db.Column(db.String(100), nullable=False) website_url = db.Column(db.String(500)) - + # Classification data - stream_type_id = db.Column(db.Integer, db.ForeignKey("stream_types.id"), nullable=False) + stream_type_id = db.Column( + db.Integer, db.ForeignKey("stream_types.id"), nullable=False + ) is_secure = db.Column(db.Boolean, nullable=False, default=False) - + # User-editable fields country = db.Column(db.String(50)) description = db.Column(db.Text) image_url = db.Column(db.String(500)) - + # Timestamps created_at = db.Column(db.DateTime(timezone=True), server_default=func.now()) @@ -25,4 +28,4 @@ class RadioSource(db.Model): stream_type = db.relationship("StreamType", back_populates="radio_sources") def __repr__(self): - return f"" \ No newline at end of file + return f"" diff --git a/model/entity/stream_analysis.py b/model/entity/stream_analysis.py index f04d1ab..8401c51 100644 --- a/model/entity/stream_analysis.py +++ b/model/entity/stream_analysis.py @@ -1,23 +1,30 @@ from database import db + class StreamAnalysis(db.Model): - __tablename__ = 'stream_analysis' + __tablename__ = "stream_analysis" - id= db.Column(db.Integer, primary_key=True, autoincrement=True) - stream_url = db.Column(db.String(200), nullable=False) - stream_type_id = db.Column(db.Integer, db.ForeignKey('stream_types.id'), nullable=True) # Foreign key to StreamType, null if invalid + id = db.Column(db.Integer, primary_key=True, autoincrement=True) + stream_url = db.Column(db.String(200), nullable=False) + stream_type_id = db.Column( + db.Integer, db.ForeignKey("stream_types.id"), nullable=True + ) # Foreign key to StreamType, null if invalid is_valid = db.Column(db.Boolean, nullable=False, default=False) - is_secure = db.Column(db.Boolean, nullable=False, default=False) # False for HTTP, true for HTTPS + is_secure = db.Column( + db.Boolean, nullable=False, default=False + ) # False for HTTP, true for HTTPS error_code = db.Column(db.String(50), nullable=True) # Null if valid - detection_method = db.Column(db.String(50), nullable=True) # How the stream was detected + detection_method = db.Column( + db.String(50), nullable=True + ) # How the stream was detected raw_content_type = db.Column(db.Text, nullable=True) # String from curl headers - raw_ffmpeg_output = db.Column(db.Text, nullable=True) # String from ffmpeg detection + raw_ffmpeg_output = db.Column( + db.Text, nullable=True + ) # String from ffmpeg detection extracted_metadata = db.Column(db.Text, nullable=True) - # Relationship with StreamTypes stream_type = db.relationship("StreamType", back_populates="stream_analysis") def __repr__(self): return f"" - diff --git a/model/entity/stream_type.py b/model/entity/stream_type.py index 126886f..16a9a86 100644 --- a/model/entity/stream_type.py +++ b/model/entity/stream_type.py @@ -1,25 +1,26 @@ from database import db + class StreamType(db.Model): - __tablename__ = 'stream_types' + __tablename__ = "stream_types" id = db.Column(db.Integer, primary_key=True, autoincrement=True) protocol = db.Column(db.String(10), nullable=False) # HTTP, HTTPS, HLS - format = db.Column(db.String(10), nullable=False) # MP3, AAC, OGG + format = db.Column(db.String(10), nullable=False) # MP3, AAC, OGG metadata_type = db.Column(db.String(15), nullable=False) # Icecast, Shoutcast, None - display_name = db.Column(db.String(100), nullable=False) # Human-readable name + display_name = db.Column(db.String(100), nullable=False) # Human-readable name - # Relationship with RadioSource + # Relationship with RadioSource radio_sources = db.relationship("RadioSource", back_populates="stream_type") # Relationship with StreamAnalysis - stream_analysis = db.relationship("StreamAnalysis", back_populates="stream_type") + stream_analysis = db.relationship("StreamAnalysis", back_populates="stream_type") # Relationship with Proposals - proposals = db.relationship("Proposal", back_populates="stream_type") - + proposals = db.relationship("Proposal", back_populates="stream_type") + def __repr__(self): return f"" @property def type_key(self): """Returns the type key in format: PROTOCOL-FORMAT-METADATA""" - return f"{self.protocol}-{self.format}-{self.metadata_type}" \ No newline at end of file + return f"{self.protocol}-{self.format}-{self.metadata_type}" diff --git a/model/entity/user.py b/model/entity/user.py index 02fd0a2..5e17f44 100644 --- a/model/entity/user.py +++ b/model/entity/user.py @@ -4,17 +4,19 @@ class User(db.Model): - __tablename__ = 'users' + __tablename__ = "users" id = db.Column(db.Integer, primary_key=True, autoincrement=True) email = db.Column(db.String(255), unique=True, nullable=False, index=True) hash_password = db.Column(db.String(512), nullable=False) - role = db.Column(db.String(20), nullable=False, default='user') + role = db.Column(db.String(20), nullable=False, default="user") is_active = db.Column(db.Boolean, nullable=False, default=True) # Timestamps: keep `created_at` and `last_modified_at` per project preference created_at = db.Column(db.DateTime(timezone=True), server_default=func.now()) - last_modified_at = db.Column(db.DateTime(timezone=True), server_default=func.now(), onupdate=func.now()) + last_modified_at = db.Column( + db.DateTime(timezone=True), server_default=func.now(), onupdate=func.now() + ) def get_id(self): return str(self.id) @@ -29,7 +31,7 @@ def is_anonymous(self) -> bool: @property def is_admin(self) -> bool: - return (self.role or '').lower() == 'admin' + return (self.role or "").lower() == "admin" def __repr__(self): return f"" diff --git a/model/repository/__init__.py b/model/repository/__init__.py index c21b104..b816725 100644 --- a/model/repository/__init__.py +++ b/model/repository/__init__.py @@ -6,4 +6,4 @@ from .radio_source_repository import RadioSourceRepository from .proposal_repository import ProposalRepository -__all__ = ["StreamTypeRepository", "RadioSourceRepository", "ProposalRepository"] \ No newline at end of file +__all__ = ["StreamTypeRepository", "RadioSourceRepository", "ProposalRepository"] diff --git a/model/repository/proposal_repository.py b/model/repository/proposal_repository.py index 4c77a04..7f31bc2 100644 --- a/model/repository/proposal_repository.py +++ b/model/repository/proposal_repository.py @@ -9,22 +9,22 @@ class ProposalRepository: """Repository for Proposal data access operations.""" - + def __init__(self, db_session: Session): self.db = db_session - + def find_by_id(self, proposal_id: int) -> Optional[Proposal]: """Get Proposal by ID.""" return self.db.query(Proposal).filter(Proposal.id == proposal_id).first() - + def find_by_url(self, url: str) -> Optional[Proposal]: """Get Proposal by URL.""" return self.db.query(Proposal).filter(Proposal.url == url).first() - + def find_all(self) -> List[Proposal]: """Get all Proposals.""" return self.db.query(Proposal).all() - + def save(self, proposal: Proposal) -> Proposal: """Save (create or update) a Proposal.""" if proposal.id is None: @@ -32,7 +32,7 @@ def save(self, proposal: Proposal) -> Proposal: self.db.commit() self.db.refresh(proposal) return proposal - + def delete(self, proposal_id: int) -> bool: """Delete a Proposal by ID.""" proposal = self.find_by_id(proposal_id) @@ -41,15 +41,18 @@ def delete(self, proposal_id: int) -> bool: self.db.commit() return True return False - + def count(self) -> int: """Count total Proposals.""" return self.db.query(Proposal).count() - + def exists_by_stream_url(self, stream_url: str) -> bool: """Check if a Proposal with the given stream URL already exists.""" - return self.db.query(Proposal).filter(Proposal.stream_url == stream_url).first() is not None - + return ( + self.db.query(Proposal).filter(Proposal.stream_url == stream_url).first() + is not None + ) + def get_all_proposals(self) -> List[Proposal]: """Retrieve all proposals from the database.""" - return self.db.query(Proposal).all() \ No newline at end of file + return self.db.query(Proposal).all() diff --git a/model/repository/radio_source_repository.py b/model/repository/radio_source_repository.py index ac885f2..b2cad5e 100644 --- a/model/repository/radio_source_repository.py +++ b/model/repository/radio_source_repository.py @@ -9,30 +9,38 @@ class RadioSourceRepository: """Repository for RadioSource data access operations.""" - + def __init__(self, db_session: Session): self.db = db_session - + def find_by_id(self, source_id: int) -> Optional[RadioSource]: """Get RadioSource by ID.""" return self.db.query(RadioSource).filter(RadioSource.id == source_id).first() - + def find_by_url(self, url: str) -> Optional[RadioSource]: """Get RadioSource by URL (for duplicate checking).""" return self.db.query(RadioSource).filter(RadioSource.stream_url == url).first() - + def find_all(self) -> List[RadioSource]: """Get all RadioSources.""" return self.db.query(RadioSource).all() - + def find_by_stream_type(self, stream_type_id: int) -> List[RadioSource]: """Get RadioSources by stream type.""" - return self.db.query(RadioSource).filter(RadioSource.stream_type_id == stream_type_id).all() - + return ( + self.db.query(RadioSource) + .filter(RadioSource.stream_type_id == stream_type_id) + .all() + ) + def search_by_name(self, name_query: str) -> List[RadioSource]: """Search RadioSources by name.""" - return self.db.query(RadioSource).filter(RadioSource.name.ilike(f'%{name_query}%')).all() - + return ( + self.db.query(RadioSource) + .filter(RadioSource.name.ilike(f"%{name_query}%")) + .all() + ) + def save(self, radio_source: RadioSource) -> RadioSource: """Save (create or update) a RadioSource.""" if radio_source.id is None: @@ -40,7 +48,7 @@ def save(self, radio_source: RadioSource) -> RadioSource: self.db.commit() self.db.refresh(radio_source) return radio_source - + def delete(self, source_id: int) -> bool: """Delete a RadioSource by ID.""" radio_source = self.find_by_id(source_id) @@ -49,7 +57,7 @@ def delete(self, source_id: int) -> bool: self.db.commit() return True return False - + def count(self) -> int: """Count total RadioSources.""" - return self.db.query(RadioSource).count() \ No newline at end of file + return self.db.query(RadioSource).count() diff --git a/model/repository/stream_analysis_repository.py b/model/repository/stream_analysis_repository.py index a419c14..633ad84 100644 --- a/model/repository/stream_analysis_repository.py +++ b/model/repository/stream_analysis_repository.py @@ -9,54 +9,57 @@ class StreamAnalysisRepository: """Repository for StreamAnalysis data access operations.""" - + def __init__(self, db_session: Session): self.db = db_session - + def find_by_id(self, id: int) -> Optional[StreamAnalysis]: """Get StreamAnalysis by ID.""" return self.db.query(StreamAnalysis).filter(StreamAnalysis.id == id).first() - + def find_all(self) -> List[StreamAnalysis]: """Get all StreamAnalysises.""" return self.db.query(StreamAnalysis).all() - + def count(self) -> int: """Count total StreamAnalysises.""" return self.db.query(StreamAnalysis).count() - + def find_by_url(self, stream_url: str) -> Optional[StreamAnalysis]: """ Find StreamAnalysis ID by stream url - + Returns: StreamAnalysis if found, None otherwise """ - stream_analysis = self.db.query(StreamAnalysis).filter( - StreamAnalysis.stream_url == stream_url - ).first() - + stream_analysis = ( + self.db.query(StreamAnalysis) + .filter(StreamAnalysis.stream_url == stream_url) + .first() + ) + return stream_analysis if stream_analysis else None - def save(self, new_analysis: StreamAnalysis) -> StreamAnalysis: """ Create a StreamAnalysis if it doesn't already exist. Used for initializing predefined types. """ - existing = self.db.query(StreamAnalysis).filter( - StreamAnalysis.stream_url == new_analysis.stream_url - ).first() - + existing = ( + self.db.query(StreamAnalysis) + .filter(StreamAnalysis.stream_url == new_analysis.stream_url) + .first() + ) + if existing: return existing - + self.db.add(new_analysis) self.db.commit() self.db.refresh(new_analysis) - + return new_analysis - + def delete(self, id: int) -> bool: """Delete a StreamAnalysis by ID.""" existing = self.find_by_id(id) @@ -64,4 +67,4 @@ def delete(self, id: int) -> bool: self.db.delete(existing) self.db.commit() return True - return False \ No newline at end of file + return False diff --git a/model/repository/stream_type_repository.py b/model/repository/stream_type_repository.py index 5822c9e..c220146 100644 --- a/model/repository/stream_type_repository.py +++ b/model/repository/stream_type_repository.py @@ -9,68 +9,80 @@ class StreamTypeRepository: """Repository for StreamType data access operations.""" - + def __init__(self, db_session: Session): self.db = db_session - + def find_by_id(self, stream_type_id: int) -> Optional[StreamType]: """Get StreamType by ID.""" return self.db.query(StreamType).filter(StreamType.id == stream_type_id).first() - + def find_all(self) -> List[StreamType]: """Get all StreamTypes.""" return self.db.query(StreamType).all() - + def count(self) -> int: """Count total StreamTypes.""" return self.db.query(StreamType).count() - - def find_by_combination(self, protocol: str, format_type: str, metadata: str) -> Optional[int]: + + def find_by_combination( + self, protocol: str, format_type: str, metadata: str + ) -> Optional[int]: """ Find StreamType ID by protocol, format, and metadata combination. - + Returns: StreamType ID if found, None otherwise """ - stream_type = self.db.query(StreamType).filter( - StreamType.protocol == protocol, - StreamType.format == format_type, - StreamType.metadata_type == metadata - ).first() - + stream_type = ( + self.db.query(StreamType) + .filter( + StreamType.protocol == protocol, + StreamType.format == format_type, + StreamType.metadata_type == metadata, + ) + .first() + ) + return stream_type.id if stream_type else None - - def create_if_not_exists(self, protocol: str, format_type: str, metadata: str, display_name: str) -> StreamType: + + def create_if_not_exists( + self, protocol: str, format_type: str, metadata: str, display_name: str + ) -> StreamType: """ Create a StreamType if it doesn't already exist. Used for initializing predefined types. """ - existing = self.db.query(StreamType).filter( - StreamType.protocol == protocol, - StreamType.format == format_type, - StreamType.metadata_type == metadata - ).first() - + existing = ( + self.db.query(StreamType) + .filter( + StreamType.protocol == protocol, + StreamType.format == format_type, + StreamType.metadata_type == metadata, + ) + .first() + ) + if existing: return existing - + new_type = StreamType( protocol=protocol, format=format_type, metadata_type=metadata, - display_name=display_name + display_name=display_name, ) - + self.db.add(new_type) self.db.commit() self.db.refresh(new_type) - + return new_type - + def get_type_key_to_id_map(self) -> Dict[str, int]: """ Get a dictionary mapping type keys (PROTOCOL-FORMAT-METADATA) to IDs. Useful for quick lookups during stream analysis. """ stream_types = self.get_all() - return {st.type_key: st.id for st in stream_types} \ No newline at end of file + return {st.type_key: st.id for st in stream_types} diff --git a/model/repository/user_repository.py b/model/repository/user_repository.py index 4279ad3..2deac01 100644 --- a/model/repository/user_repository.py +++ b/model/repository/user_repository.py @@ -12,8 +12,8 @@ def find_by_id(self, user_id: int) -> Optional[User]: def find_by_email(self, email: str) -> Optional[User]: return self.session.query(User).filter(User.email == email).first() - - def create(self, email: str, hash_password: str, role: str = 'user') -> User: + + def create(self, email: str, hash_password: str, role: str = "user") -> User: user = User(email=email, hash_password=hash_password, role=role) self.session.add(user) @@ -30,7 +30,7 @@ def update_password(self, user: User, new_hash: str) -> User: def set_role(self, user: User, role: str) -> User: user.role = role - + self.session.commit() self.session.refresh(user) return user diff --git a/route/analysis_route.py b/route/analysis_route.py index de98a17..8fde3ac 100644 --- a/route/analysis_route.py +++ b/route/analysis_route.py @@ -17,78 +17,98 @@ from service.stream_type_service import StreamTypeService -analysis_bp = Blueprint('analysis', __name__, url_prefix='/analysis') +analysis_bp = Blueprint("analysis", __name__, url_prefix="/analysis") + # Repository and service initialization functions def get_analysis_repo() -> StreamAnalysisRepository: from database import db + return StreamAnalysisRepository(db.session) + def get_proposal_repo(): from database import db + return ProposalRepository(db.session) + def get_radio_source_repo(): from database import db + return RadioSourceRepository(db.session) + def get_validation_service(): proposal_repo = get_proposal_repo() radio_source_repo = get_radio_source_repo() from service.proposal_validation_service import ProposalValidationService + return ProposalValidationService(proposal_repo, radio_source_repo) + def get_radio_source_service(): proposal_repo = get_proposal_repo() radio_source_repo = get_radio_source_repo() validation_service = get_validation_service() from service.radio_source_service import RadioSourceService + return RadioSourceService(proposal_repo, radio_source_repo, validation_service) + def get_stream_type_repo() -> StreamTypeRepository: from database import db from model.repository.stream_type_repository import StreamTypeRepository + return StreamTypeRepository(db.session) + def get_stream_analysis_service() -> StreamAnalysisService: stream_type_service = get_stream_type_service() from service.stream_analysis_service import StreamAnalysisService - return StreamAnalysisService(stream_type_service, get_proposal_repo(), get_analysis_repo()) + + return StreamAnalysisService( + stream_type_service, get_proposal_repo(), get_analysis_repo() + ) def get_stream_type_service(): stream_type_repo = get_stream_type_repo() from service.stream_type_service import StreamTypeService + return StreamTypeService(stream_type_repo) -@analysis_bp.route('/', methods=['GET']) +@analysis_bp.route("/", methods=["GET"]) def index(): """Display the analysis page with all stream analyses.""" analysis_repo = get_analysis_repo() # Get all analyses for display (pass entity objects so templates can access id) streams_from_db: List[StreamAnalysis] = analysis_repo.find_all() - return render_template('analysis.html', streams=streams_from_db) + return render_template("analysis.html", streams=streams_from_db) # only a registered user can analyze a stream and save as proposal @login_required -@analysis_bp.route('/analyze', methods=['POST']) +@analysis_bp.route("/analyze", methods=["POST"]) def analyze_url(): """Analyze a stream URL and show results.""" - url = request.form.get('url') - + url = request.form.get("url") + if not url: - flash('URL is required', 'error') - return redirect(url_for('analysis.index')) - + flash("URL is required", "error") + return redirect(url_for("analysis.index")) + try: analysis_service: StreamAnalysisService = get_stream_analysis_service() result: StreamAnalysisResult = analysis_service.analyze_stream(url) # Show the result in a simple format - flash(f'Analysis result: {result.stream_type_display_name if result.stream_type_display_name else "Unknown"}', 'info') + flash( + f'Analysis result: {result.stream_type_display_name if result.stream_type_display_name else "Unknown"}', + "info", + ) # Save detail in repo analysis_repo = get_analysis_repo() @@ -97,50 +117,55 @@ def analyze_url(): is_valid=result.is_valid, is_secure=result.is_secure, error_code=result.error_code.name if result.error_code else None, - stream_type_id=result.stream_type_id, - detection_method=result.detection_method.name if result.detection_method else None, + stream_type_id=result.stream_type_id, + detection_method=( + result.detection_method.name if result.detection_method else None + ), raw_content_type=result.raw_content_type, raw_ffmpeg_output=result.raw_ffmpeg_output, - extracted_metadata=result.extracted_metadata + extracted_metadata=result.extracted_metadata, ) - analysis_repo.save(analysis_entity) - + analysis_repo.save(analysis_entity) + except Exception as e: - flash(f'Analysis failed: {str(e)}', 'error') - - return redirect(url_for('analysis.index')) + flash(f"Analysis failed: {str(e)}", "error") + + return redirect(url_for("analysis.index")) @login_required -@analysis_bp.route('/approve/', methods=['POST']) +@analysis_bp.route("/approve/", methods=["POST"]) def approve_analysis(id: int): """Approve an analysis and creates a proposal.""" - + try: analysis_service: StreamAnalysisService = get_stream_analysis_service() success = analysis_service.save_analysis_as_proposal(id) if success: - flash('ProposalAnalysis approved and added as proposal for radio source!', 'success') + flash( + "ProposalAnalysis approved and added as proposal for radio source!", + "success", + ) else: - flash('Failed to approve stream analysis', 'error') + flash("Failed to approve stream analysis", "error") except Exception as e: - flash(f'Error approving stream analysis: {str(e)}', 'error') + flash(f"Error approving stream analysis: {str(e)}", "error") - return redirect(url_for('proposal.index')) + return redirect(url_for("proposal.index")) @login_required -@analysis_bp.route('/delete/', methods=['POST']) +@analysis_bp.route("/delete/", methods=["POST"]) def delete_analysis(id: int): """Delete an analysis.""" analysis_service: StreamAnalysisService = get_stream_analysis_service() - + try: success = analysis_service.delete_analysis(id) if success: - flash('Stream analysis deleted successfully!', 'success') + flash("Stream analysis deleted successfully!", "success") else: - flash('Failed to delete stream analysis', 'error') + flash("Failed to delete stream analysis", "error") except Exception as e: - flash(f'Error deleting stream analysis: {str(e)}', 'error') - return redirect(url_for('analysis.index')) \ No newline at end of file + flash(f"Error deleting stream analysis: {str(e)}", "error") + return redirect(url_for("analysis.index")) diff --git a/route/auth_route.py b/route/auth_route.py index 6f7d1e3..e840bd4 100644 --- a/route/auth_route.py +++ b/route/auth_route.py @@ -11,29 +11,37 @@ auth_service = AuthService() -auth_bp = Blueprint('auth', __name__, url_prefix='/auth') +auth_bp = Blueprint("auth", __name__, url_prefix="/auth") class RegisterForm(FlaskForm): - email = StringField('Email', validators=[DataRequired(), Email()]) - password = PasswordField('Password', validators=[DataRequired(), Length(min=8)]) - confirm = PasswordField('Confirm', validators=[DataRequired(), EqualTo('password')]) - submit = SubmitField('Register') + email = StringField("Email", validators=[DataRequired(), Email()]) + password = PasswordField("Password", validators=[DataRequired(), Length(min=8)]) + confirm = PasswordField("Confirm", validators=[DataRequired(), EqualTo("password")]) + submit = SubmitField("Register") + class ChangePasswordForm(FlaskForm): - old_password = PasswordField('Current password', validators=[DataRequired()]) - new_password = PasswordField('New password', validators=[DataRequired(), Length(min=8)]) - confirm = PasswordField('Confirm', validators=[DataRequired(), EqualTo('new_password')]) - submit = SubmitField('Change Password') + old_password = PasswordField("Current password", validators=[DataRequired()]) + new_password = PasswordField( + "New password", validators=[DataRequired(), Length(min=8)] + ) + confirm = PasswordField( + "Confirm", validators=[DataRequired(), EqualTo("new_password")] + ) + submit = SubmitField("Change Password") + class LoginForm(FlaskForm): - email = StringField('Email', validators=[DataRequired(), Email()]) - password = PasswordField('Password', validators=[DataRequired()]) - submit = SubmitField('Login') + email = StringField("Email", validators=[DataRequired(), Email()]) + password = PasswordField("Password", validators=[DataRequired()]) + submit = SubmitField("Login") + user_repo = UserRepository() -@auth_bp.route('/change_password', methods=['GET', 'POST']) + +@auth_bp.route("/change_password", methods=["GET", "POST"]) @login_required def change_password(): form = ChangePasswordForm() @@ -47,65 +55,71 @@ def change_password(): else: verified = bool(res) if not verified: - flash('Current password incorrect', 'error') - return render_template('user/change_password.html', form=form), 400 + flash("Current password incorrect", "error") + return render_template("user/change_password.html", form=form), 400 auth_service.change_password(user, new) - flash('Password changed successfully', 'success') - return redirect(url_for('main.index')) - return render_template('user/change_password.html', form=form) - -user_repo = UserRepository() + flash("Password changed successfully", "success") + return redirect(url_for("main.index")) + return render_template("user/change_password.html", form=form) +user_repo = UserRepository() -@auth_bp.route('/login', methods=['GET', 'POST']) +@auth_bp.route("/login", methods=["GET", "POST"]) def login(): form = LoginForm() if form.validate_on_submit(): - found_user: User = auth_service.user_repo.find_by_email(form.email.data) # adapt to your repo/service call - - if found_user and auth_service.verify_password(form.password.data, found_user.hash_password): + found_user: User = auth_service.user_repo.find_by_email( + form.email.data + ) # adapt to your repo/service call + + if found_user and auth_service.verify_password( + form.password.data, found_user.hash_password + ): login_user(found_user) - flash('Signed in successfully', 'success') - next_page = request.args.get('next') or url_for('main.index') + flash("Signed in successfully", "success") + next_page = request.args.get("next") or url_for("main.index") return redirect(next_page) verified = False if found_user: - res = auth_service.verify_password(form.password.data, found_user.hash_password) + res = auth_service.verify_password( + form.password.data, found_user.hash_password + ) if isinstance(res, tuple): verified = bool(res[0]) else: verified = bool(res) - + if not verified: - flash('Invalid email or password', 'error') - return render_template('user/login.html', form=form) + flash("Invalid email or password", "error") + return render_template("user/login.html", form=form) else: login_user(found_user) - flash('Signed in successfully', 'success') - next_page = request.args.get('next') or url_for('main.index') + flash("Signed in successfully", "success") + next_page = request.args.get("next") or url_for("main.index") return redirect(next_page) - - return render_template('user/login.html', form=form) + return render_template("user/login.html", form=form) -@auth_bp.route('/logout') + +@auth_bp.route("/logout") @login_required def logout(): logout_user() - return redirect(url_for('main.index')) + return redirect(url_for("main.index")) -@auth_bp.route('/register', methods=['GET', 'POST']) +@auth_bp.route("/register", methods=["GET", "POST"]) def register(): form = RegisterForm() if form.validate_on_submit(): try: - auth_service.register_user(email=form.email.data, password=form.password.data) - flash('Registration successful. Please login.', 'success') - return redirect(url_for('auth.login')) + auth_service.register_user( + email=form.email.data, password=form.password.data + ) + flash("Registration successful. Please login.", "success") + return redirect(url_for("auth.login")) except ValueError as e: - flash(str(e), 'error') - return render_template('user/register.html', form=form) - + flash(str(e), "error") + return render_template("user/register.html", form=form) diff --git a/route/database_route.py b/route/database_route.py index 527ce50..7526325 100644 --- a/route/database_route.py +++ b/route/database_route.py @@ -9,31 +9,36 @@ from model.repository.proposal_repository import ProposalRepository from database import db -database_bp = Blueprint('database', __name__, url_prefix='/database') +database_bp = Blueprint("database", __name__, url_prefix="/database") + # Repository initialization functions def get_radio_source_repo(): from database import db + return RadioSourceRepository(db.session) + def get_stream_type_repo(): from database import db + return StreamTypeRepository(db.session) + def get_proposal_repo(): from database import db - return ProposalRepository(db.session) + return ProposalRepository(db.session) -@database_bp.route('/sources') +@database_bp.route("/sources") def list_sources(): """List all radio sources with filtering options.""" radio_source_repo = get_radio_source_repo() stream_type_repo = get_stream_type_repo() - - stream_type_filter = request.args.get('stream_type') - search_query = request.args.get('search') + + stream_type_filter = request.args.get("stream_type") + search_query = request.args.get("search") if stream_type_filter: sources = radio_source_repo.find_by_stream_type(int(stream_type_filter)) @@ -44,52 +49,57 @@ def list_sources(): stream_types = stream_type_repo.find_all() - return render_template('sources.html', - sources=sources, - stream_types=stream_types, - current_filter=stream_type_filter, - search_query=search_query) + return render_template( + "sources.html", + sources=sources, + stream_types=stream_types, + current_filter=stream_type_filter, + search_query=search_query, + ) -@database_bp.route('/proposals') +@database_bp.route("/proposals") def list_proposals(): """List all pending proposals.""" proposal_repo = get_proposal_repo() proposals = proposal_repo.find_all() - return render_template('proposals.html', proposals=proposals) + return render_template("proposals.html", proposals=proposals) -@database_bp.route('/database') +@database_bp.route("/database") def index(): """Database management page with overview of all entities.""" radio_source_repo = get_radio_source_repo() stream_type_repo = get_stream_type_repo() proposal_repo = get_proposal_repo() - + stream_types = stream_type_repo.find_all() proposals = proposal_repo.find_all() radio_sources = radio_source_repo.find_all() - - return render_template('database.html', - stream_types=stream_types, - proposals=proposals, - radio_sources=radio_sources) + return render_template( + "database.html", + stream_types=stream_types, + proposals=proposals, + radio_sources=radio_sources, + ) -@database_bp.route('/api/stats') + +@database_bp.route("/api/stats") def get_stats(): """API endpoint to get database statistics.""" radio_source_repo = get_radio_source_repo() proposal_repo = get_proposal_repo() stream_type_repo = get_stream_type_repo() - + source_count = radio_source_repo.count() proposal_count = proposal_repo.count() stream_type_count = stream_type_repo.count() - return jsonify({ - 'total_sources': source_count, - 'total_proposals': proposal_count, - 'total_stream_types': stream_type_count - }) - + return jsonify( + { + "total_sources": source_count, + "total_proposals": proposal_count, + "total_stream_types": stream_type_count, + } + ) diff --git a/route/listen_route.py b/route/listen_route.py index a41022f..54e3ee6 100644 --- a/route/listen_route.py +++ b/route/listen_route.py @@ -4,6 +4,7 @@ listen_bp = Blueprint("listen", __name__, url_prefix="/listen") + @listen_bp.route("/") def player(source_id: int): repo = RadioSourceRepository(db.session) @@ -11,4 +12,4 @@ def player(source_id: int): if source is None: abort(404) # Ensure stream_url exists and is a string; further validation/sanitization can be added here - return render_template("listen_player.html", source=source) \ No newline at end of file + return render_template("listen_player.html", source=source) diff --git a/route/main_route.py b/route/main_route.py index 3224d09..4f4fc57 100644 --- a/route/main_route.py +++ b/route/main_route.py @@ -1,19 +1,20 @@ - from flask import Blueprint, render_template from model.repository.radio_source_repository import RadioSourceRepository -main_bp = Blueprint('main', __name__, url_prefix='/') +main_bp = Blueprint("main", __name__, url_prefix="/") + # Repository initialization functions def get_radio_source_repo(): from database import db + return RadioSourceRepository(db.session) -@main_bp.route('/') +@main_bp.route("/") def index(): """Main index page - list all radio sources.""" radio_source_repo = get_radio_source_repo() sources = radio_source_repo.find_all() - return render_template('index.html', sources=sources) \ No newline at end of file + return render_template("index.html", sources=sources) diff --git a/route/proposal_route.py b/route/proposal_route.py index d5b859e..6ac66d5 100644 --- a/route/proposal_route.py +++ b/route/proposal_route.py @@ -16,73 +16,80 @@ from service.authorization import admin_required -proposal_bp = Blueprint('proposal', __name__, url_prefix='/proposal') +proposal_bp = Blueprint("proposal", __name__, url_prefix="/proposal") def get_proposal_repo(): from database import db + return ProposalRepository(db.session) + def get_radio_source_repo(): from database import db + return RadioSourceRepository(db.session) + def get_validation_service(): proposal_repo = get_proposal_repo() radio_source_repo = get_radio_source_repo() from service.proposal_validation_service import ProposalValidationService + return ProposalValidationService(proposal_repo, radio_source_repo) + def get_radio_source_service(): proposal_repo = get_proposal_repo() radio_source_repo = get_radio_source_repo() validation_service = get_validation_service() from service.radio_source_service import RadioSourceService + return RadioSourceService(proposal_repo, radio_source_repo, validation_service) + def get_stream_type_service(): stream_type_repo = get_stream_type_repo() from service.stream_type_service import StreamTypeService + return StreamTypeService(stream_type_repo) def get_proposal_service(): proposal_repo = get_proposal_repo() from service.proposal_service import ProposalService + return ProposalService(proposal_repo) -@proposal_bp.route('/', methods=['GET']) +@proposal_bp.route("/", methods=["GET"]) def index(): """Display the proposals page with all proposals""" proposal_repo: ProposalRepository = get_proposal_repo() # Get all proposals for display (pass entity objects so templates can access id) proposals_from_db: List[Proposal] = proposal_repo.find_all() - return render_template('proposals.html', proposals=proposals_from_db) + return render_template("proposals.html", proposals=proposals_from_db) # only a registered use can propose a new radio source @login_required -@proposal_bp.route('/propose', methods=['GET', 'POST']) +@proposal_bp.route("/propose", methods=["GET", "POST"]) def propose(): """Handle proposal submission form.""" proposal_repo = get_proposal_repo() validation_service = get_validation_service() - if request.method == 'POST': + if request.method == "POST": # Extract form data - name = request.form.get('name') - url = request.form.get('url') - description = request.form.get('description', '') - user_name = request.form.get('user_name', 'Anonymous') + name = request.form.get("name") + url = request.form.get("url") + description = request.form.get("description", "") + user_name = request.form.get("user_name", "Anonymous") # Create proposal proposal = Proposal( - name=name, - url=url, - description=description, - user_name=user_name + name=name, url=url, description=description, user_name=user_name ) # Validate and save @@ -91,71 +98,73 @@ def propose(): if result.is_valid: # Save proposal proposal_repo.save(proposal) - flash('Proposal submitted successfully!', 'success') - return redirect(url_for('proposal.proposal_detail', proposal_id=proposal.id)) + flash("Proposal submitted successfully!", "success") + return redirect( + url_for("proposal.proposal_detail", proposal_id=proposal.id) + ) else: - flash(f'Validation failed: {result.message}', 'error') + flash(f"Validation failed: {result.message}", "error") except Exception as e: - flash(f'Error submitting proposal: {str(e)}', 'error') + flash(f"Error submitting proposal: {str(e)}", "error") # After proposing or when visiting this endpoint, show the proposals listing - return redirect(url_for('proposal.index')) + return redirect(url_for("proposal.index")) @login_required -@proposal_bp.route('/update/', methods=['GET', 'POST']) +@proposal_bp.route("/update/", methods=["GET", "POST"]) def proposal_detail(proposal_id): - if request.method == 'POST': + if request.method == "POST": # Read form values and delegate update to ProposalService - name = request.form.get('name') - website_url = request.form.get('website_url') - country = request.form.get('country') - description = request.form.get('description') + name = request.form.get("name") + website_url = request.form.get("website_url") + country = request.form.get("country") + description = request.form.get("description") # Accept either 'image' (form) or 'image_url' (tests/clients) - image = request.form.get('image_url') or request.form.get('image') or None + image = request.form.get("image_url") or request.form.get("image") or None update_dto = ProposalUpdateRequest( name=name, website_url=website_url, country=country, description=description, - image=image + image=image, ) proposal_service: ProposalService = get_proposal_service() try: proposal_service.update_proposal(proposal_id, update_dto) - flash('Proposal updated successfully', 'success') + flash("Proposal updated successfully", "success") except Exception as e: - flash(f'Failed to update proposal: {str(e)}', 'error') + flash(f"Failed to update proposal: {str(e)}", "error") - return redirect(url_for('proposal.index')) + return redirect(url_for("proposal.index")) """Display proposal details and validation status.""" proposal_repo = get_proposal_repo() - + proposal = proposal_repo.find_by_id(proposal_id) if not proposal: - flash('Proposal not found', 'error') - return redirect(url_for('proposal.index')) + flash("Proposal not found", "error") + return redirect(url_for("proposal.index")) - return render_template('proposal_detail.html',proposal=proposal) + return render_template("proposal_detail.html", proposal=proposal) # only admin users can approve proposals @admin_required -@proposal_bp.route('/approve/', methods=['POST']) +@proposal_bp.route("/approve/", methods=["POST"]) def approve_proposal(proposal_id): """Approve and convert proposal to radio source.""" radio_source_service = get_radio_source_service() - + try: success: RadioSource = radio_source_service.save_from_proposal(proposal_id) if success: - flash('Proposal approved and added as radio source!', 'success') + flash("Proposal approved and added as radio source!", "success") else: - flash('Failed to approve proposal', 'error') + flash("Failed to approve proposal", "error") except Exception as e: - flash(f'Error approving proposal: {str(e)}', 'error') + flash(f"Error approving proposal: {str(e)}", "error") - return redirect(url_for('proposal.index')) + return redirect(url_for("proposal.index")) diff --git a/route/radio_source_route.py b/route/radio_source_route.py index a337c2b..d5ca2a3 100644 --- a/route/radio_source_route.py +++ b/route/radio_source_route.py @@ -4,7 +4,7 @@ """ from typing import List -from flask import Blueprint, request,render_template, redirect, url_for, flash +from flask import Blueprint, request, render_template, redirect, url_for, flash from model.entity.stream_type import StreamType from model.repository.stream_type_repository import StreamTypeRepository from service.authorization import admin_required @@ -12,77 +12,85 @@ from model.repository.radio_source_repository import RadioSourceRepository from database import db -radio_source_bp = Blueprint('radio_source', __name__, url_prefix='/source') +radio_source_bp = Blueprint("radio_source", __name__, url_prefix="/source") + # Repository and service initialization functions def get_radio_source_repo() -> RadioSourceRepository: from database import db + return RadioSourceRepository(db.session) + def get_radio_source_service() -> RadioSourceService: radio_source_repo = get_radio_source_repo() from service.radio_source_service import RadioSourceService - return RadioSourceService(None, radio_source_repo, None) # Validation service will be injected if needed + + return RadioSourceService( + None, radio_source_repo, None + ) # Validation service will be injected if needed + def get_stream_type_repo() -> StreamTypeRepository: from database import db + return StreamTypeRepository(db.session) -@radio_source_bp.route('/') +@radio_source_bp.route("/") def source_detail(source_id): """Display radio source details.""" radio_source_repo = get_radio_source_repo() source = radio_source_repo.find_by_id(source_id) if not source: - flash('Radio source not found', 'error') - return redirect(url_for('main.index')) + flash("Radio source not found", "error") + return redirect(url_for("main.index")) - return render_template('source_detail.html', source=source) + return render_template("source_detail.html", source=source) # only admin users can edit or delete radio sources @admin_required -@radio_source_bp.route('/edit/', methods=['GET', 'POST']) +@radio_source_bp.route("/edit/", methods=["GET", "POST"]) def edit_source(source_id): """Edit radio source.""" radio_source_repo: RadioSourceRepository = get_radio_source_repo() stream_type_repo: StreamTypeRepository = get_stream_type_repo() source = radio_source_repo.find_by_id(source_id) - + if not source: - flash('Radio source not found', 'error') - return redirect(url_for('main.index')) - - if request.method == 'POST': - source.name = request.form.get('name') - source.stream_url = request.form.get('url') - source.description = request.form.get('description', '') - source.stream_type_id = int(request.form.get('stream_type_id')) + flash("Radio source not found", "error") + return redirect(url_for("main.index")) + + if request.method == "POST": + source.name = request.form.get("name") + source.stream_url = request.form.get("url") + source.description = request.form.get("description", "") + source.stream_type_id = int(request.form.get("stream_type_id")) try: radio_source_repo.save(source) - flash('Radio source updated successfully!', 'success') - return redirect(url_for('radio_source.source_detail', source_id=source.id)) - + flash("Radio source updated successfully!", "success") + return redirect(url_for("radio_source.source_detail", source_id=source.id)) + except Exception as e: - flash(f'Error updating source: {str(e)}', 'error') + flash(f"Error updating source: {str(e)}", "error") # For GET request, show edit form stream_types: List[StreamType] = stream_type_repo.find_all() - return render_template('edit_source.html', source=source, stream_types=stream_types) + return render_template("edit_source.html", source=source, stream_types=stream_types) @admin_required -@radio_source_bp.route('/delete/', methods=['POST']) +@radio_source_bp.route("/delete/", methods=["POST"]) def delete_source(source_id): """Delete radio source.""" radio_source_service: RadioSourceService = get_radio_source_service() - + if radio_source_service.delete_radio_source(source_id): - flash('Radio source deleted successfully!', 'success') + flash("Radio source deleted successfully!", "success") else: - flash('Error deleting source', 'error') + flash("Error deleting source", "error") - return redirect(url_for('main.index')) + return redirect(url_for("main.index")) diff --git a/scripts/smoke_auth_check.py b/scripts/smoke_auth_check.py index 84c292e..e34a84c 100644 --- a/scripts/smoke_auth_check.py +++ b/scripts/smoke_auth_check.py @@ -7,15 +7,17 @@ from app import app + def fetch(path): with app.test_client() as c: r = c.get(path) print(f"GET {path} -> {r.status_code}, {len(r.data)} bytes") - snippet = r.data.decode('utf-8', errors='replace')[:400] - print('--- snippet ---') + snippet = r.data.decode("utf-8", errors="replace")[:400] + print("--- snippet ---") print(snippet) - print('--- end ---\n') + print("--- end ---\n") + -if __name__ == '__main__': - fetch('/auth/login') - fetch('/auth/register') +if __name__ == "__main__": + fetch("/auth/login") + fetch("/auth/register") diff --git a/service/__init__.py b/service/__init__.py index ce08e1c..50f12c1 100644 --- a/service/__init__.py +++ b/service/__init__.py @@ -5,4 +5,4 @@ from .stream_analysis_service import StreamAnalysisService from .stream_type_service import StreamTypeService -__all__ = ["StreamAnalysisService", "StreamTypeService"] \ No newline at end of file +__all__ = ["StreamAnalysisService", "StreamTypeService"] diff --git a/service/auth_service.py b/service/auth_service.py index b5b95a9..de55ef5 100644 --- a/service/auth_service.py +++ b/service/auth_service.py @@ -6,7 +6,9 @@ # Prefer a pure-Python secure scheme for portability in dev/test; keep bcrypt available # Use pbkdf2_sha256 as default to avoid system bcrypt C-extension issues in some environments -pwd_context = CryptContext(schemes=["pbkdf2_sha256", "bcrypt"], default="pbkdf2_sha256", deprecated="auto") +pwd_context = CryptContext( + schemes=["pbkdf2_sha256", "bcrypt"], default="pbkdf2_sha256", deprecated="auto" +) class AuthService: @@ -18,7 +20,7 @@ def __init__(self, app=None): def init_app(self, app): lm = LoginManager() - lm.login_view = 'auth.login' + lm.login_view = "auth.login" lm.init_app(app) @lm.user_loader @@ -35,7 +37,7 @@ def verify_password(self, plain: str, hashed: str) -> (bool, str | None): Verify password; return (verified, new_hash_or_none) new_hash_or_none is non-None when the hash should be updated (lazy upgrade) """ - + new_hash: str | None = None try: # verify_and_update returns (bool, new_hash) @@ -47,12 +49,12 @@ def verify_password(self, plain: str, hashed: str) -> (bool, str | None): return verified, new_hash - def register_user(self, email: str, password: str, role: str = 'user') -> User: + def register_user(self, email: str, password: str, role: str = "user") -> User: existing: User | None = self.user_repo.find_by_email(email) - + if existing: - raise ValueError('Email already registered') - + raise ValueError("Email already registered") + hashed: str = self.hash_password(password) return self.user_repo.create(email=email, hash_password=hashed, role=role) diff --git a/service/authorization.py b/service/authorization.py index e3ab35f..783d431 100644 --- a/service/authorization.py +++ b/service/authorization.py @@ -2,6 +2,7 @@ from flask import abort from flask_login import current_user, login_required + # decorator for role-based access control def admin_required(func): @wraps(func) @@ -10,4 +11,5 @@ def wrapper(*args, **kwargs): if not getattr(current_user, "is_admin", False): abort(403) return func(*args, **kwargs) - return wrapper \ No newline at end of file + + return wrapper diff --git a/service/proposal_service.py b/service/proposal_service.py index c36f04f..6538554 100644 --- a/service/proposal_service.py +++ b/service/proposal_service.py @@ -15,7 +15,9 @@ class ProposalService: def __init__(self, proposal_repo: ProposalRepository): self.proposal_repo = proposal_repo - def update_proposal(self, proposal_id: int, updates: ProposalUpdateRequest) -> Proposal: + def update_proposal( + self, proposal_id: int, updates: ProposalUpdateRequest + ) -> Proposal: """Update editable fields of a proposal and persist changes. Editable fields: name, website_url, country, description, image (mapped to image_url) diff --git a/service/proposal_validation_service.py b/service/proposal_validation_service.py index 68487e7..9dc477c 100644 --- a/service/proposal_validation_service.py +++ b/service/proposal_validation_service.py @@ -15,53 +15,53 @@ class ProposalValidationService: """ Service for validating proposals before saving to RadioSourceNode. - + Validates: - Required fields presence - URL format validity - Duplicate stream URLs - Security status """ - + def __init__( - self, + self, proposal_repo: ProposalRepository, - radio_source_repo: RadioSourceRepository + radio_source_repo: RadioSourceRepository, ): self.proposal_repo = proposal_repo self.radio_source_repo = radio_source_repo - + def validate_proposal(self, proposal_id: int) -> ValidationResult: """ Validate a proposal before saving to RadioSourceNode. - + Checks: - Proposal exists - Required fields are present and non-empty - URLs are valid format - Stream URL is not duplicate - + Args: proposal_id: ID of the proposal to validate - + Returns: ValidationResult with is_valid flag and error/warning messages """ result = ValidationResult(is_valid=True) - + # Check proposal exists proposal = self.proposal_repo.find_by_id(proposal_id) if not proposal: result.add_error(f"Proposal with ID {proposal_id} not found") return result - + # Validate required fields (FR-006) if not str(proposal.stream_url) or not str(proposal.stream_url).strip(): result.add_error("Stream URL is required and cannot be empty") - + if not str(proposal.name) or not str(proposal.name).strip(): result.add_error("Name is required and cannot be empty") - + if not str(proposal.website_url) or not str(proposal.website_url).strip(): result.add_error("Website URL is required and cannot be empty") @@ -69,46 +69,50 @@ def validate_proposal(self, proposal_id: int) -> ValidationResult: # Proposals created from discovery (spec 001) may not have this yet; they # must be classified (spec 003) before being saved to the radio sources table. if proposal.stream_type_id is None: - result.add_error("Stream type is required. Please classify the stream before saving.") - + result.add_error( + "Stream type is required. Please classify the stream before saving." + ) + # Validate URL formats if str(proposal.stream_url): if not self._is_valid_url(str(proposal.stream_url)): result.add_error(f"Invalid stream URL format: {proposal.stream_url}") - + if str(proposal.website_url): if not self._is_valid_url(str(proposal.website_url)): result.add_error(f"Invalid website URL format: {proposal.website_url}") - + # Check for duplicate stream URL (FR-005) - if str(proposal.stream_url) and self.check_duplicate_stream_url(str(proposal.stream_url)): + if str(proposal.stream_url) and self.check_duplicate_stream_url( + str(proposal.stream_url) + ): result.add_error("This stream URL already exists in the database") - + # Add security warning if HTTP stream if str(proposal.stream_url) and not bool(proposal.is_secure): result.add_warning("This stream uses HTTP (not secure)") - + return result - + def check_duplicate_stream_url(self, stream_url: str) -> bool: """ Check if a stream URL already exists in RadioSourceNode table. - + Args: stream_url: The stream URL to check - + Returns: True if duplicate found, False otherwise """ return self.radio_source_repo.find_by_url(stream_url) is not None - + def get_security_status(self, proposal_id: int) -> Optional[SecurityStatus]: """ Get the security status of a proposal. - + Args: proposal_id: ID of the proposal - + Returns: SecurityStatus object with is_secure flag and warning message, or None if proposal not found @@ -116,22 +120,22 @@ def get_security_status(self, proposal_id: int) -> Optional[SecurityStatus]: proposal = self.proposal_repo.find_by_id(proposal_id) if not proposal: return None - + return SecurityStatus.from_is_secure(proposal.is_secure) - + def _is_valid_url(self, url: str) -> bool: """ Validate URL format. - + Args: url: URL string to validate - + Returns: True if URL has valid format, False otherwise """ try: parsed = urlparse(url) # Must have scheme (http/https only) and netloc (domain) - return bool(parsed.scheme in ['http', 'https'] and parsed.netloc) + return bool(parsed.scheme in ["http", "https"] and parsed.netloc) except Exception: return False diff --git a/service/radio_source_service.py b/service/radio_source_service.py index 673ffd7..e25027b 100644 --- a/service/radio_source_service.py +++ b/service/radio_source_service.py @@ -19,39 +19,39 @@ class RadioSourceService: """ Service for managing RadioSourceNodes and their lifecycle. - + Handles: - Saving proposals as RadioSourceNodes - Rejecting proposals - Updating proposal data """ - + def __init__( self, proposal_repo: ProposalRepository, radio_source_repo: RadioSourceRepository, - validation_service: ProposalValidationService + validation_service: ProposalValidationService, ): self.proposal_repo = proposal_repo self.radio_source_repo = radio_source_repo self.validation_service = validation_service - + def save_from_proposal(self, proposal_id: int) -> RadioSource: """ Save a proposal as a RadioSourceNode in the database. - + This method: 1. Validates the proposal 2. Creates a RadioSourceNode from proposal data 3. Saves to database 4. Deletes the proposal (single transaction) - + Args: proposal_id: ID of the proposal to save - + Returns: The saved RadioSourceNode - + Raises: ValueError: If validation fails or proposal not found RuntimeError: If database operation fails @@ -61,12 +61,12 @@ def save_from_proposal(self, proposal_id: int) -> RadioSource: if not validation_result.is_valid: error_msg = "; ".join(validation_result.errors) raise ValueError(f"Proposal validation failed: {error_msg}") - + # Get proposal proposal = self.proposal_repo.find_by_id(proposal_id) if not proposal: raise ValueError(f"Proposal with ID {proposal_id} not found") - + # Create RadioSourceNode from proposal radio_source = RadioSource( stream_url=proposal.stream_url, @@ -77,36 +77,37 @@ def save_from_proposal(self, proposal_id: int) -> RadioSource: country=proposal.country, description=proposal.description, image_url=proposal.image_url, - created_at=datetime.now() + created_at=datetime.now(), ) - + try: # Save RadioSourceNode (this will commit the transaction) saved_source = self.radio_source_repo.save(radio_source) - + # Delete proposal after successful save self.proposal_repo.delete(proposal_id) - + return saved_source - + except Exception as e: raise RuntimeError(f"Failed to save radio source: {str(e)}") - - - def update_proposal(self, proposal_id: int, updates: ProposalUpdateRequest) -> Proposal: + + def update_proposal( + self, proposal_id: int, updates: ProposalUpdateRequest + ) -> Proposal: """ Update user-editable fields of a proposal. - + Only allows updating: name, website_url, country, description, image Read-only fields (stream_type_id, is_secure) cannot be modified. - + Args: proposal_id: ID of the proposal to update updates: ProposalUpdateRequest with fields to update - + Returns: Updated Proposal - + Raises: ValueError: If proposal not found or no updates provided """ @@ -114,69 +115,65 @@ def update_proposal(self, proposal_id: int, updates: ProposalUpdateRequest) -> P proposal = self.proposal_repo.find_by_id(proposal_id) if not proposal: raise ValueError(f"Proposal with ID {proposal_id} not found") - + # Check if any updates provided if not updates.has_updates(): raise ValueError("No updates provided") - + # Update only user-editable fields if updates.name is not None: proposal.name = updates.name - + if updates.website_url is not None: proposal.website_url = updates.website_url - + if updates.country is not None: proposal.country = updates.country - + if updates.description is not None: proposal.description = updates.description - + if updates.image is not None: proposal.image = updates.image - + # Save and return updated proposal return self.proposal_repo.save(proposal) - def get_proposal(self, proposal_id: int) -> Optional[Proposal]: """ Get a proposal by ID. - + Args: proposal_id: ID of the proposal - + Returns: Proposal if found, None otherwise """ return self.proposal_repo.find_by_id(proposal_id) - def get_all_proposals(self) -> list[Proposal]: """ Get all proposals. - + Returns: List of all proposals """ return self.proposal_repo.get_all_proposals() - def get_all_radio_sources(self) -> list[RadioSource]: """ Get all radio sources. - + Returns: List of all radio sources """ return self.radio_source_repo.find_all() def delete_radio_source(self, id) -> bool: - """ delete a radio source""" + """delete a radio source""" if id: return self.radio_source_repo.delete(id) return False - def reject_proposal(self, proposal_id: int) -> bool: """ @@ -212,4 +209,3 @@ def reject_proposal(self, proposal_id: int) -> bool: return False except Exception: return False - diff --git a/service/stream_analysis_service.py b/service/stream_analysis_service.py index 79e76e4..d59e47b 100644 --- a/service/stream_analysis_service.py +++ b/service/stream_analysis_service.py @@ -1,7 +1,7 @@ """ StreamAnalysisService - Core implementation of spec 003 analyze-and-classify-stream. -This service implements the dual validation strategy (curl + ffmpeg) and +This service implements the dual validation strategy (curl + ffmpeg) and classification logic as defined in the specification. """ @@ -18,17 +18,23 @@ from model.repository.proposal_repository import ProposalRepository from model.entity.stream_analysis import StreamAnalysis + class StreamAnalysisService: """ Core service implementing spec 003: analyze-and-classify-stream - + Performs dual validation: 1. curl -I for headers/content-type analysis 2. ffmpeg -i for deep format analysis 3. FFmpeg is authoritative when results differ """ - - def __init__(self, stream_type_service: StreamTypeService, proposal_repository: ProposalRepository, analysis_repository: StreamAnalysisRepository): + + def __init__( + self, + stream_type_service: StreamTypeService, + proposal_repository: ProposalRepository, + analysis_repository: StreamAnalysisRepository, + ): self.stream_type_service: StreamTypeService = stream_type_service self.proposal_repository: ProposalRepository = proposal_repository self.analysis_repository: StreamAnalysisRepository = analysis_repository @@ -40,19 +46,25 @@ def _check_prerequisites(self) -> None: Raises RuntimeError if prerequisites are not met. """ if not shutil.which("ffmpeg"): - raise RuntimeError("ffmpeg is not installed or not accessible in PATH. Required for stream analysis.") - + raise RuntimeError( + "ffmpeg is not installed or not accessible in PATH. Required for stream analysis." + ) + if not shutil.which("curl"): - raise RuntimeError("curl is not installed or not accessible in PATH. Required for header analysis.") - - def analyze_stream(self, url: str, timeout_seconds: int = 30) -> StreamAnalysisResult: + raise RuntimeError( + "curl is not installed or not accessible in PATH. Required for header analysis." + ) + + def analyze_stream( + self, url: str, timeout_seconds: int = 30 + ) -> StreamAnalysisResult: """ Main entry point for stream analysis (spec 003). - + Args: url: The stream URL to analyze timeout_seconds: Maximum time to spend on analysis (default: 30s as per SC-001) - + Returns: StreamAnalysisResult with validation and classification data """ @@ -63,50 +75,52 @@ def analyze_stream(self, url: str, timeout_seconds: int = 30) -> StreamAnalysisR stream_url=url, is_valid=False, is_secure=False, - error_code=ErrorCode.UNSUPPORTED_PROTOCOL + error_code=ErrorCode.UNSUPPORTED_PROTOCOL, ) - + # Determine if URL is secure (HTTPS vs HTTP) is_secure = self._is_secure_url(url) - + try: # FR-002: Perform dual validation curl_result = self._analyze_with_curl(url, timeout_seconds) ffmpeg_result = self._analyze_with_ffmpeg(url, timeout_seconds) - + # FR-003: Compare results, ffmpeg is authoritative - final_result: StreamAnalysisResult = self._resolve_analysis_results(curl_result, ffmpeg_result, is_secure) + final_result: StreamAnalysisResult = self._resolve_analysis_results( + curl_result, ffmpeg_result, is_secure + ) # print("Analysis result for URL {}: {}".format(url, final_result)) return final_result - + except subprocess.TimeoutExpired: return StreamAnalysisResult( stream_url=url, is_valid=False, is_secure=is_secure, - error_code=ErrorCode.TIMEOUT + error_code=ErrorCode.TIMEOUT, ) except Exception: return StreamAnalysisResult( stream_url=url, is_valid=False, is_secure=is_secure, - error_code=ErrorCode.NETWORK_ERROR + error_code=ErrorCode.NETWORK_ERROR, ) - + def _is_supported_protocol(self, url: str) -> bool: """Check if the URL uses a supported protocol (HTTP/HTTPS only).""" parsed = urlparse(url) - return parsed.scheme.lower() in ['http', 'https'] - + return parsed.scheme.lower() in ["http", "https"] + def _is_secure_url(self, url: str) -> bool: """Determine if URL is secure (HTTPS = true, HTTP = false).""" - return urlparse(url).scheme.lower() == 'https' - + return urlparse(url).scheme.lower() == "https" + def _analyze_with_curl(self, url: str, timeout_seconds: int) -> Dict[str, Any]: """ Analyze stream using curl -I to get headers and content-type. - + Returns: Dict with 'success', 'content_type', 'raw_output' keys """ @@ -116,30 +130,34 @@ def _analyze_with_curl(self, url: str, timeout_seconds: int) -> Dict[str, Any]: capture_output=True, text=True, timeout=timeout_seconds, - check=False + check=False, ) - + if result.returncode != 0: - return {"success": False, "content_type": None, "raw_output": result.stderr} - + return { + "success": False, + "content_type": None, + "raw_output": result.stderr, + } + # Extract content-type from headers content_type = self._extract_content_type(result.stdout) - + return { "success": True, "content_type": content_type, - "raw_output": result.stdout + "raw_output": result.stdout, } - + except subprocess.TimeoutExpired: raise except Exception as e: return {"success": False, "content_type": None, "raw_output": str(e)} - + def _analyze_with_ffmpeg(self, url: str, timeout_seconds: int) -> Dict[str, Any]: """ Analyze stream using ffmpeg -i for deep format analysis. - + Returns: Dict with 'success', 'format', 'codec', 'raw_output' keys """ @@ -149,9 +167,9 @@ def _analyze_with_ffmpeg(self, url: str, timeout_seconds: int) -> Dict[str, Any] capture_output=True, text=True, timeout=timeout_seconds, - check=False + check=False, ) - + # ffmpeg writes info to stderr, not stdout output = result.stderr @@ -166,51 +184,48 @@ def _analyze_with_ffmpeg(self, url: str, timeout_seconds: int) -> Dict[str, Any] "format": format_info.get("format") if format_info else None, "codec": format_info.get("codec") if format_info else None, "raw_output": output, - "extracted_metadata": extracted_metadata + "extracted_metadata": extracted_metadata, } - + except subprocess.TimeoutExpired: raise except Exception as e: - return {"success": False, "format": None, "codec": None, "raw_output": str(e)} - + return { + "success": False, + "format": None, + "codec": None, + "raw_output": str(e), + } + def _extract_content_type(self, headers: str) -> Optional[str]: """Extract content-type from HTTP headers.""" # Handle both actual newlines and literal \n in headers - lines = headers.replace('\\n', '\n').split('\n') + lines = headers.replace("\\n", "\n").split("\n") for line in lines: - if line.lower().startswith('content-type:'): - return line.split(':', 1)[1].strip() + if line.lower().startswith("content-type:"): + return line.split(":", 1)[1].strip() return None - + def _parse_ffmpeg_output(self, output: str) -> Optional[Dict[str, str]]: """ Parse ffmpeg output to extract format and codec information. - + Example ffmpeg output: "Stream #0:0: Audio: mp3 (mp3float), 22050 Hz, mono, fltp, 24 kb/s" """ # Look for audio stream information - audio_match = re.search(r'Stream #\d+:\d+: Audio: (\w+)', output) + audio_match = re.search(r"Stream #\d+:\d+: Audio: (\w+)", output) if not audio_match: return None - + codec = audio_match.group(1).lower() - + # Map ffmpeg codec names to our format names - format_mapping = { - 'mp3': 'MP3', - 'aac': 'AAC', - 'ogg': 'OGG', - 'vorbis': 'OGG' - } - + format_mapping = {"mp3": "MP3", "aac": "AAC", "ogg": "OGG", "vorbis": "OGG"} + format_name = format_mapping.get(codec, codec.upper()) - - return { - "format": format_name, - "codec": codec - } + + return {"format": format_name, "codec": codec} def _extract_metadata_from_ffmpeg_output(self, output: str) -> Optional[str]: """ @@ -227,17 +242,20 @@ def _extract_metadata_from_ffmpeg_output(self, output: str) -> Optional[str]: if not output or not isinstance(output, str): return None - norm = output.replace('\r\n', '\n').replace('\r', '\n') + norm = output.replace("\r\n", "\n").replace("\r", "\n") # Find all 'Metadata:' markers; choose the last one - meta_matches = [m.start() for m in re.finditer(r"^\s*Metadata:\s*$", norm, flags=re.MULTILINE)] + meta_matches = [ + m.start() + for m in re.finditer(r"^\s*Metadata:\s*$", norm, flags=re.MULTILINE) + ] if not meta_matches: return None # Start scanning from the last metadata marker last_pos = meta_matches[-1] tail = norm[last_pos:] - lines = tail.split('\n') + lines = tail.split("\n") # Skip the 'Metadata:' line itself captured = [] @@ -252,8 +270,8 @@ def _extract_metadata_from_ffmpeg_output(self, output: str) -> Optional[str]: # normalize: remove leading whitespace, normalize spacing around ':' stripped = line.strip() # replace multiple spaces around colon - if ':' in stripped: - parts = stripped.split(':', 1) + if ":" in stripped: + parts = stripped.split(":", 1) key = parts[0].strip() val = parts[1].strip() captured.append(f"{key}: {val}") @@ -268,13 +286,15 @@ def _extract_metadata_from_ffmpeg_output(self, output: str) -> Optional[str]: joined = "\n".join(captured) # Remove control chars except newline and tab - cleaned = ''.join(ch for ch in joined if (ch >= ' ' or ch in '\n\t')) + cleaned = "".join(ch for ch in joined if (ch >= " " or ch in "\n\t")) cleaned = cleaned.strip() if len(cleaned) > 4096: cleaned = cleaned[:4096] return cleaned - - def _resolve_analysis_results(self, curl_result: dict, ffmpeg_result: dict, is_secure: bool) -> StreamAnalysisResult: + + def _resolve_analysis_results( + self, curl_result: dict, ffmpeg_result: dict, is_secure: bool + ) -> StreamAnalysisResult: """ Resolve analysis results from curl and ffmpeg. FR-003: FFmpeg is authoritative when results differ. @@ -285,55 +305,75 @@ def _resolve_analysis_results(self, curl_result: dict, ffmpeg_result: dict, is_s return self._classify_from_curl(curl_result, is_secure) else: return StreamAnalysisResult( - stream_url=curl_result["stream_url"] if "stream_url" in curl_result else None, + stream_url=( + curl_result["stream_url"] + if "stream_url" in curl_result + else None + ), is_valid=False, is_secure=is_secure, error_code=ErrorCode.UNREACHABLE, raw_content_type=curl_result.get("raw_output"), raw_ffmpeg_output=ffmpeg_result.get("raw_output"), - extracted_metadata=ffmpeg_result.get("extracted_metadata") + extracted_metadata=ffmpeg_result.get("extracted_metadata"), ) - + # FFmpeg succeeded - use it as authoritative source format_name = ffmpeg_result["format"] if not format_name: return StreamAnalysisResult( is_valid=False, - stream_url=ffmpeg_result["stream_url"] if "stream_url" in ffmpeg_result else None, + stream_url=( + ffmpeg_result["stream_url"] + if "stream_url" in ffmpeg_result + else None + ), is_secure=is_secure, error_code=ErrorCode.INVALID_FORMAT, raw_content_type=curl_result.get("raw_output"), raw_ffmpeg_output=ffmpeg_result.get("raw_output"), - extracted_metadata=ffmpeg_result.get("extracted_metadata") + extracted_metadata=ffmpeg_result.get("extracted_metadata"), ) - + # Determine protocol (HTTP/HTTPS based on URL, or HLS if m3u8 detected) protocol = "HTTPS" if is_secure else "HTTP" if ".m3u8" in ffmpeg_result.get("raw_output", "").lower(): protocol = "HLS" - + # Detect metadata support (basic heuristic - could be enhanced) metadata = self._detect_metadata_support(curl_result.get("raw_output", "")) - + # Find matching StreamType - stream_type_id = self.stream_type_service.find_stream_type_id(protocol, format_name, metadata) - - detection_method = DetectionMethod.BOTH if curl_result["success"] else DetectionMethod.FFMPEG - + stream_type_id = self.stream_type_service.find_stream_type_id( + protocol, format_name, metadata + ) + + detection_method = ( + DetectionMethod.BOTH if curl_result["success"] else DetectionMethod.FFMPEG + ) + return StreamAnalysisResult( is_valid=stream_type_id is not None, stream_type_id=stream_type_id, - stream_type_display_name=self.stream_type_service.get_display_name(stream_type_id) if stream_type_id else None, - stream_url=ffmpeg_result["stream_url"] if "stream_url" in ffmpeg_result else None, + stream_type_display_name=( + self.stream_type_service.get_display_name(stream_type_id) + if stream_type_id + else None + ), + stream_url=( + ffmpeg_result["stream_url"] if "stream_url" in ffmpeg_result else None + ), is_secure=is_secure, error_code=None, detection_method=detection_method, raw_content_type=curl_result.get("raw_output"), raw_ffmpeg_output=ffmpeg_result.get("raw_output"), - extracted_metadata=ffmpeg_result.get("extracted_metadata") + extracted_metadata=ffmpeg_result.get("extracted_metadata"), ) - - def _classify_from_curl(self, curl_result: dict, is_secure: bool) -> StreamAnalysisResult: + + def _classify_from_curl( + self, curl_result: dict, is_secure: bool + ) -> StreamAnalysisResult: """Classify stream based only on curl content-type headers.""" content_type = curl_result["content_type"] if not content_type: @@ -341,9 +381,9 @@ def _classify_from_curl(self, curl_result: dict, is_secure: bool) -> StreamAnaly is_valid=False, is_secure=is_secure, error_code=ErrorCode.INVALID_FORMAT, - raw_content_type=curl_result.get("raw_output") + raw_content_type=curl_result.get("raw_output"), ) - + # Map content-type to format format_name = None if "audio/mpeg" in content_type.lower() or "audio/mp3" in content_type.lower(): @@ -356,39 +396,51 @@ def _classify_from_curl(self, curl_result: dict, is_secure: bool) -> StreamAnaly # HLS playlist protocol = "HLS" format_name = "AAC" # Most common for HLS - stream_type_id = self.stream_type_service.find_stream_type_id(protocol, format_name, "None") + stream_type_id = self.stream_type_service.find_stream_type_id( + protocol, format_name, "None" + ) return StreamAnalysisResult( is_valid=stream_type_id is not None, stream_type_id=stream_type_id, - stream_type_display_name=self.stream_type_service.get_display_name(stream_type_id) if stream_type_id else None, + stream_type_display_name=( + self.stream_type_service.get_display_name(stream_type_id) + if stream_type_id + else None + ), stream_url=curl_result.get("stream_url", None), is_secure=is_secure, detection_method=DetectionMethod.HEADER, - raw_content_type=curl_result.get("raw_output") + raw_content_type=curl_result.get("raw_output"), ) - + if not format_name: return StreamAnalysisResult( is_valid=False, is_secure=is_secure, error_code=ErrorCode.INVALID_FORMAT, - raw_content_type=curl_result.get("raw_output") + raw_content_type=curl_result.get("raw_output"), ) - + protocol = "HTTPS" if is_secure else "HTTP" metadata = self._detect_metadata_support(curl_result.get("raw_output", "")) - stream_type_id = self.stream_type_service.find_stream_type_id(protocol, format_name, metadata) - + stream_type_id = self.stream_type_service.find_stream_type_id( + protocol, format_name, metadata + ) + return StreamAnalysisResult( is_valid=stream_type_id is not None, stream_type_id=stream_type_id, - stream_type_display_name=self.stream_type_service.get_display_name(stream_type_id) if stream_type_id else None, + stream_type_display_name=( + self.stream_type_service.get_display_name(stream_type_id) + if stream_type_id + else None + ), stream_url=curl_result.get("stream_url", None), is_secure=is_secure, detection_method=DetectionMethod.HEADER, - raw_content_type=curl_result.get("raw_output") + raw_content_type=curl_result.get("raw_output"), ) - + def _detect_metadata_support(self, headers: str) -> str: """ Detect metadata support (Icecast/Shoutcast) from headers. @@ -396,11 +448,11 @@ def _detect_metadata_support(self, headers: str) -> str: """ if not headers: return "None" - + # Handle both actual newlines and literal \n in headers - headers_normalized = headers.replace('\\n', '\n') + headers_normalized = headers.replace("\\n", "\n") headers_lower = headers_normalized.lower() - + # Check for Shoutcast first (more specific) if "shoutcast" in headers_lower: return "Shoutcast" @@ -408,7 +460,7 @@ def _detect_metadata_support(self, headers: str) -> str: return "Icecast" else: return "None" - + def save_analysis_as_proposal(self, stream_or_id) -> bool: """ Approve an analysis and create a proposal for radio source. @@ -429,10 +481,10 @@ def save_analysis_as_proposal(self, stream_or_id) -> bool: return False # Determine required fields - stream_url = getattr(stream_entity, 'stream_url', None) - stream_type_id = getattr(stream_entity, 'stream_type_id', None) - is_secure = getattr(stream_entity, 'is_secure', False) - is_valid = getattr(stream_entity, 'is_valid', True) + stream_url = getattr(stream_entity, "stream_url", None) + stream_type_id = getattr(stream_entity, "stream_type_id", None) + is_secure = getattr(stream_entity, "is_secure", False) + is_valid = getattr(stream_entity, "is_valid", True) if not is_valid or not stream_url or not stream_type_id: print("Cannot create proposal for invalid analysis or missing data.") @@ -460,7 +512,7 @@ def save_analysis_as_proposal(self, stream_or_id) -> bool: if isinstance(stream_or_id, int): self.analysis_repository.delete(stream_or_id) else: - stream_id = getattr(stream_entity, 'id', None) + stream_id = getattr(stream_entity, "id", None) if stream_id: self.analysis_repository.delete(stream_id) except Exception: @@ -482,8 +534,8 @@ def delete_analysis(self, stream_or_id) -> bool: return self.analysis_repository.delete(stream_or_id) # If an object provided, try to get id - stream_id = getattr(stream_or_id, 'id', None) + stream_id = getattr(stream_or_id, "id", None) if stream_id: return self.analysis_repository.delete(stream_id) - return False \ No newline at end of file + return False diff --git a/service/stream_type_service.py b/service/stream_type_service.py index 61b621c..7bf397b 100644 --- a/service/stream_type_service.py +++ b/service/stream_type_service.py @@ -10,24 +10,26 @@ class StreamTypeService: """Service for managing StreamType entities and lookup operations.""" - + def __init__(self, stream_type_repository: StreamTypeRepository): self.repository = stream_type_repository - - def find_stream_type_id(self, protocol: str, format: str, metadata: str) -> Optional[int]: + + def find_stream_type_id( + self, protocol: str, format: str, metadata: str + ) -> Optional[int]: """ Find StreamType ID for given protocol, format, and metadata combination. - + Args: protocol: HTTP, HTTPS, or HLS format: MP3, AAC, OGG metadata: Icecast, Shoutcast, or None - + Returns: StreamType ID if found, None otherwise """ return self.repository.find_by_combination(protocol, format, metadata) - + def get_stream_type(self, stream_type_id: int) -> Optional[StreamTypeDTO]: """Get StreamType by ID.""" stream_type = self.repository.get_by_id(stream_type_id) @@ -38,28 +40,31 @@ def get_stream_type(self, stream_type_id: int) -> Optional[StreamTypeDTO]: protocol=stream_type.protocol, format=stream_type.format, metadata=stream_type.metadata_type, - display_name=stream_type.display_name + display_name=stream_type.display_name, ) return None - + def get_all_stream_types(self) -> List[StreamTypeDTO]: """Get all available StreamTypes.""" stream_types = self.repository.get_all() - return [StreamTypeDTO( - id=st.id, - protocol=st.protocol, - format=st.format, - metadata=st.metadata_type, - display_name=st.display_name - ) for st in stream_types] - + return [ + StreamTypeDTO( + id=st.id, + protocol=st.protocol, + format=st.format, + metadata=st.metadata_type, + display_name=st.display_name, + ) + for st in stream_types + ] + def get_predefined_types_map(self) -> Dict[str, int]: """ Get a map of type keys (PROTOCOL-FORMAT-METADATA) to IDs. Useful for quick lookups during analysis. """ return self.repository.get_type_key_to_id_map() - + def initialize_predefined_types(self) -> None: """ Initialize the database with the 14 predefined StreamTypes from spec 003. @@ -79,15 +84,22 @@ def initialize_predefined_types(self) -> None: ("HTTPS", "AAC", "Shoutcast", "HTTPS AAC with Shoutcast metadata"), ("HTTPS", "AAC", "None", "HTTPS AAC direct stream"), ("HLS", "AAC", "None", "HTTP Live Streaming (HLS) with AAC"), - ("PLAYLIST", "PLAYLIST", "None", "Playlist file (.m3u, .pls, .m3u8) - parsing not implemented") + ( + "PLAYLIST", + "PLAYLIST", + "None", + "Playlist file (.m3u, .pls, .m3u8) - parsing not implemented", + ), ] - + for protocol, format_type, metadata, display_name in predefined_types: - self.repository.create_if_not_exists(protocol, format_type, metadata, display_name) + self.repository.create_if_not_exists( + protocol, format_type, metadata, display_name + ) def get_display_name(self, stream_type_id: int) -> Optional[str]: """Get the display name of a StreamType by its ID.""" stream_type = self.repository.find_by_id(stream_type_id) if stream_type: return stream_type.display_name - return None \ No newline at end of file + return None diff --git a/tests/conftest.py b/tests/conftest.py index 4b2f4ac..8014cc0 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,17 +14,18 @@ from database import db from model.entity.stream_type import StreamType + @pytest.fixture(scope="session") def test_app(): """Create a test Flask application.""" app = Flask(__name__) - app.config['TESTING'] = True - app.config['SQLALCHEMY_DATABASE_URI'] = 'sqlite:///:memory:' - app.config['SQLALCHEMY_TRACK_MODIFICATIONS'] = False + app.config["TESTING"] = True + app.config["SQLALCHEMY_DATABASE_URI"] = "sqlite:///:memory:" + app.config["SQLALCHEMY_TRACK_MODIFICATIONS"] = False # Required for session/flash in tests - app.secret_key = 'test-secret' + app.secret_key = "test-secret" # Disable CSRF in tests for simplicity (we assert behavior, not CSRF infra) - app.config['WTF_CSRF_ENABLED'] = False + app.config["WTF_CSRF_ENABLED"] = False db.init_app(app) @@ -34,6 +35,7 @@ def test_app(): _initialize_stream_types() # ensure LoginManager is configured early from service.auth_service import AuthService + AuthService(app) yield app @@ -65,7 +67,7 @@ def sample_urls(): "valid_http_aac": "http://stream.example.com:8000/stream.aac", "invalid_html": "https://example.com/index.html", "rtmp_unsupported": "rtmp://stream.example.com/live", - "hls_playlist": "https://stream.example.com/playlist.m3u8" + "hls_playlist": "https://stream.example.com/playlist.m3u8", } @@ -74,24 +76,94 @@ def _initialize_stream_types(): # Ensure related entity classes are imported so SQLAlchemy can configure relationships stream_types_data = [ - {"protocol": "HTTP", "format": "MP3", "metadata_type": "Icecast", "display_name": "HTTP MP3 Icecast"}, - {"protocol": "HTTP", "format": "MP3", "metadata_type": "Shoutcast", "display_name": "HTTP MP3 Shoutcast"}, - {"protocol": "HTTP", "format": "AAC", "metadata_type": "Icecast", "display_name": "HTTP AAC Icecast"}, - {"protocol": "HTTP", "format": "AAC", "metadata_type": "Shoutcast", "display_name": "HTTP AAC Shoutcast"}, - {"protocol": "HTTPS", "format": "MP3", "metadata_type": "Icecast", "display_name": "HTTPS MP3 Icecast"}, - {"protocol": "HTTPS", "format": "MP3", "metadata_type": "Shoutcast", "display_name": "HTTPS MP3 Shoutcast"}, - {"protocol": "HTTPS", "format": "AAC", "metadata_type": "Icecast", "display_name": "HTTPS AAC Icecast"}, - {"protocol": "HTTPS", "format": "AAC", "metadata_type": "Shoutcast", "display_name": "HTTPS AAC Shoutcast"}, - {"protocol": "HLS", "format": "AAC", "metadata_type": "None", "display_name": "HLS AAC"}, - {"protocol": "HLS", "format": "MP3", "metadata_type": "None", "display_name": "HLS MP3"}, - {"protocol": "HTTP", "format": "OGG", "metadata_type": "Icecast", "display_name": "HTTP OGG Icecast"}, - {"protocol": "HTTPS", "format": "OGG", "metadata_type": "Icecast", "display_name": "HTTPS OGG Icecast"}, - {"protocol": "HTTP", "format": "FLAC", "metadata_type": "None", "display_name": "HTTP FLAC"}, - {"protocol": "HTTPS", "format": "FLAC", "metadata_type": "None", "display_name": "HTTPS FLAC"}, + { + "protocol": "HTTP", + "format": "MP3", + "metadata_type": "Icecast", + "display_name": "HTTP MP3 Icecast", + }, + { + "protocol": "HTTP", + "format": "MP3", + "metadata_type": "Shoutcast", + "display_name": "HTTP MP3 Shoutcast", + }, + { + "protocol": "HTTP", + "format": "AAC", + "metadata_type": "Icecast", + "display_name": "HTTP AAC Icecast", + }, + { + "protocol": "HTTP", + "format": "AAC", + "metadata_type": "Shoutcast", + "display_name": "HTTP AAC Shoutcast", + }, + { + "protocol": "HTTPS", + "format": "MP3", + "metadata_type": "Icecast", + "display_name": "HTTPS MP3 Icecast", + }, + { + "protocol": "HTTPS", + "format": "MP3", + "metadata_type": "Shoutcast", + "display_name": "HTTPS MP3 Shoutcast", + }, + { + "protocol": "HTTPS", + "format": "AAC", + "metadata_type": "Icecast", + "display_name": "HTTPS AAC Icecast", + }, + { + "protocol": "HTTPS", + "format": "AAC", + "metadata_type": "Shoutcast", + "display_name": "HTTPS AAC Shoutcast", + }, + { + "protocol": "HLS", + "format": "AAC", + "metadata_type": "None", + "display_name": "HLS AAC", + }, + { + "protocol": "HLS", + "format": "MP3", + "metadata_type": "None", + "display_name": "HLS MP3", + }, + { + "protocol": "HTTP", + "format": "OGG", + "metadata_type": "Icecast", + "display_name": "HTTP OGG Icecast", + }, + { + "protocol": "HTTPS", + "format": "OGG", + "metadata_type": "Icecast", + "display_name": "HTTPS OGG Icecast", + }, + { + "protocol": "HTTP", + "format": "FLAC", + "metadata_type": "None", + "display_name": "HTTP FLAC", + }, + { + "protocol": "HTTPS", + "format": "FLAC", + "metadata_type": "None", + "display_name": "HTTPS FLAC", + }, ] for st_data in stream_types_data: stream_type = StreamType(**st_data) db.session.add(stream_type) - db.session.commit() \ No newline at end of file + db.session.commit() diff --git a/tests/integration/test_auth_flow.py b/tests/integration/test_auth_flow.py index fb3dbf5..2ccc24f 100644 --- a/tests/integration/test_auth_flow.py +++ b/tests/integration/test_auth_flow.py @@ -11,37 +11,49 @@ def register_blueprints_and_auth(app): # Ensure Flask finds the repository `templates/` folder when tests create app instances - app.template_folder = str(Path(__file__).parents[2] / 'templates') + app.template_folder = str(Path(__file__).parents[2] / "templates") # Register blueprints only if they are not already registered (idempotent) - for bp in (main_bp, database_bp, analysis_bp, proposal_bp, radio_source_bp, listen_bp, auth_bp): + for bp in ( + main_bp, + database_bp, + analysis_bp, + proposal_bp, + radio_source_bp, + listen_bp, + auth_bp, + ): if bp.name not in app.blueprints: app.register_blueprint(bp) if not hasattr(app, "login_manager"): AuthService(app) - def test_register_login_logout_flow(test_app): register_blueprints_and_auth(test_app) client = test_app.test_client() # Register - resp = client.post('/auth/register', data={ - 'email': 'tester@example.com', - 'password': 'Password123', - 'confirm': 'Password123' - }, follow_redirects=True) - assert b'Registration successful' in resp.data + resp = client.post( + "/auth/register", + data={ + "email": "tester@example.com", + "password": "Password123", + "confirm": "Password123", + }, + follow_redirects=True, + ) + assert b"Registration successful" in resp.data # Login - resp = client.post('/auth/login', data={ - 'email': 'tester@example.com', - 'password': 'Password123' - }, follow_redirects=True) + resp = client.post( + "/auth/login", + data={"email": "tester@example.com", "password": "Password123"}, + follow_redirects=True, + ) # After login index should show Logout link and email - assert b'Logout' in resp.data - assert b'tester@example.com' in resp.data + assert b"Logout" in resp.data + assert b"tester@example.com" in resp.data # Logout - resp = client.get('/auth/logout', follow_redirects=True) - assert b'Login' in resp.data + resp = client.get("/auth/logout", follow_redirects=True) + assert b"Login" in resp.data diff --git a/tests/integration/test_smoke_auth_pages.py b/tests/integration/test_smoke_auth_pages.py index 7c2e37a..e512e98 100644 --- a/tests/integration/test_smoke_auth_pages.py +++ b/tests/integration/test_smoke_auth_pages.py @@ -12,9 +12,17 @@ def register_blueprints(app): from route.auth_route import auth_bp from service.auth_service import AuthService - app.template_folder = str(Path(__file__).parents[2] / 'templates') + app.template_folder = str(Path(__file__).parents[2] / "templates") # Register blueprints only if they are not already registered (idempotent) - for bp in (main_bp, database_bp, analysis_bp, proposal_bp, radio_source_bp, listen_bp, auth_bp): + for bp in ( + main_bp, + database_bp, + analysis_bp, + proposal_bp, + radio_source_bp, + listen_bp, + auth_bp, + ): if bp.name not in app.blueprints: app.register_blueprint(bp) @@ -23,10 +31,10 @@ def test_smoke_auth_pages_render(test_app): register_blueprints(test_app) client = test_app.test_client() - r = client.get('/auth/login') + r = client.get("/auth/login") assert r.status_code == 200 - assert b'Login' in r.data + assert b"Login" in r.data - r = client.get('/auth/register') + r = client.get("/auth/register") assert r.status_code == 200 - assert b'Register' in r.data + assert b"Register" in r.data diff --git a/tests/integration/test_validate_and_add_workflow.py b/tests/integration/test_validate_and_add_workflow.py index 075fa82..28a01c7 100644 --- a/tests/integration/test_validate_and_add_workflow.py +++ b/tests/integration/test_validate_and_add_workflow.py @@ -40,7 +40,9 @@ def services(self, repositories): """Create services with repositories.""" proposal_repo, radio_source_repo = repositories validation_service = ProposalValidationService(proposal_repo, radio_source_repo) - radio_source_service = RadioSourceService(proposal_repo, radio_source_repo, validation_service) + radio_source_service = RadioSourceService( + proposal_repo, radio_source_repo, validation_service + ) return validation_service, radio_source_service def test_complete_save_workflow(self, db_session, services): @@ -56,7 +58,7 @@ def test_complete_save_workflow(self, db_session, services): is_secure=True, country="Italy", description="A test radio station", - image_url="test.jpg" + image_url="test.jpg", ) db_session.add(proposal) db_session.commit() @@ -74,9 +76,11 @@ def test_complete_save_workflow(self, db_session, services): saved_proposal = db_session.query(Proposal).filter_by(id=proposal.id).first() assert saved_proposal is None - saved_radio_source = db_session.query(RadioSource).filter_by( - stream_url="https://stream.example.com/radio.mp3" - ).first() + saved_radio_source = ( + db_session.query(RadioSource) + .filter_by(stream_url="https://stream.example.com/radio.mp3") + .first() + ) assert saved_radio_source is not None assert saved_radio_source.name == "Test Radio Station" @@ -90,7 +94,7 @@ def test_duplicate_stream_url_prevention(self, db_session, services): name="First Radio", website_url="https://first.com", stream_type_id=1, - is_secure=True + is_secure=True, ) db_session.add(proposal1) db_session.commit() @@ -105,7 +109,7 @@ def test_duplicate_stream_url_prevention(self, db_session, services): name="Second Radio", website_url="https://second.com", stream_type_id=1, - is_secure=True + is_secure=True, ) db_session.add(proposal2) db_session.commit() @@ -113,7 +117,9 @@ def test_duplicate_stream_url_prevention(self, db_session, services): # Validation should fail due to duplicate validation_result = validation_service.validate_proposal(proposal2.id) assert not validation_result.is_valid - assert "This stream URL already exists in the database" in validation_result.errors + assert ( + "This stream URL already exists in the database" in validation_result.errors + ) def test_proposal_rejection_workflow(self, db_session, services): """Test rejecting a proposal.""" @@ -125,7 +131,7 @@ def test_proposal_rejection_workflow(self, db_session, services): name="Rejected Radio", website_url="https://reject.com", stream_type_id=1, - is_secure=True + is_secure=True, ) db_session.add(proposal) db_session.commit() @@ -150,7 +156,7 @@ def test_proposal_update_workflow(self, db_session, services): stream_type_id=1, is_secure=True, country="Original Country", - description="Original description" + description="Original description", ) db_session.add(proposal) db_session.commit() @@ -160,10 +166,12 @@ def test_proposal_update_workflow(self, db_session, services): name="Updated Name", website_url="https://updated.com", country="Updated Country", - description="Updated description" + description="Updated description", ) - update_result = radio_source_service.update_proposal(proposal.id, update_request) + update_result = radio_source_service.update_proposal( + proposal.id, update_request + ) assert update_result.name == "Updated Name" # Verify the update @@ -183,7 +191,7 @@ def test_validation_with_missing_required_fields(self, db_session, services): name="Test Radio", website_url="https://test.com", stream_type_id=1, - is_secure=True + is_secure=True, ) db_session.add(proposal) db_session.commit() @@ -203,7 +211,7 @@ def test_insecure_stream_warning(self, db_session, services): name="Insecure Radio", website_url="https://insecure.com", stream_type_id=1, - is_secure=False + is_secure=False, ) db_session.add(proposal) db_session.commit() diff --git a/tests/unit/test_analysis_routes.py b/tests/unit/test_analysis_routes.py index 4e8dfd6..ffb314c 100644 --- a/tests/unit/test_analysis_routes.py +++ b/tests/unit/test_analysis_routes.py @@ -9,26 +9,35 @@ def _register_blueprints(app): - app.register_blueprint(analysis_bp, url_prefix='/analysis') - app.register_blueprint(proposal_bp, url_prefix='/proposal') + app.register_blueprint(analysis_bp, url_prefix="/analysis") + app.register_blueprint(proposal_bp, url_prefix="/proposal") def test_delete_analysis_route_removes_row(test_app, test_db): # Ensure session works for flashing - test_app.secret_key = 'test-secret' + test_app.secret_key = "test-secret" # Insert a StreamAnalysis row - sa = StreamAnalysis(stream_url='http://test/delete', is_valid=True, is_secure=False, stream_type_id=1) + sa = StreamAnalysis( + stream_url="http://test/delete", + is_valid=True, + is_secure=False, + stream_type_id=1, + ) test_db.add(sa) test_db.commit() assert sa.id is not None # Patch shutil.which so constructing StreamAnalysisService inside route doesn't raise - with patch('service.stream_analysis_service.shutil.which', return_value='/usr/bin/ffmpeg'): + with patch( + "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" + ): # Patch url_for used inside the view to avoid needing registered endpoints - with patch('route.analysis_route.url_for', return_value='/'): - with test_app.test_request_context(f'/analysis/delete/{sa.id}', method='POST'): + with patch("route.analysis_route.url_for", return_value="/"): + with test_app.test_request_context( + f"/analysis/delete/{sa.id}", method="POST" + ): resp = delete_analysis(sa.id) # view returns a redirect response object (or response) assert resp is not None @@ -40,25 +49,40 @@ def test_delete_analysis_route_removes_row(test_app, test_db): def test_approve_analysis_route_creates_proposal(test_app, test_db): # Ensure session works for flashing - test_app.secret_key = 'test-secret' + test_app.secret_key = "test-secret" # Insert a StreamAnalysis row with required fields - sa = StreamAnalysis(stream_url='http://test/propose', is_valid=True, is_secure=True, stream_type_id=1) + sa = StreamAnalysis( + stream_url="http://test/propose", + is_valid=True, + is_secure=True, + stream_type_id=1, + ) test_db.add(sa) test_db.commit() assert sa.id is not None - with patch('service.stream_analysis_service.shutil.which', return_value='/usr/bin/ffmpeg'): - with patch('route.analysis_route.url_for', return_value='/'): - with test_app.test_request_context(f'/analysis/approve/{sa.id}', method='POST'): + with patch( + "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" + ): + with patch("route.analysis_route.url_for", return_value="/"): + with test_app.test_request_context( + f"/analysis/approve/{sa.id}", method="POST" + ): resp = approve_analysis(sa.id) assert resp is not None # Verify the StreamAnalysis row was removed after proposing - found = test_db.query(StreamAnalysis).filter(StreamAnalysis.stream_url == sa.stream_url).first() + found = ( + test_db.query(StreamAnalysis) + .filter(StreamAnalysis.stream_url == sa.stream_url) + .first() + ) assert found is None # Verify a Proposal was created for that stream_url - proposal = test_db.query(Proposal).filter(Proposal.stream_url == sa.stream_url).first() + proposal = ( + test_db.query(Proposal).filter(Proposal.stream_url == sa.stream_url).first() + ) assert proposal is not None assert proposal.stream_url == sa.stream_url diff --git a/tests/unit/test_auth_service.py b/tests/unit/test_auth_service.py index 81429bb..5cccbfd 100644 --- a/tests/unit/test_auth_service.py +++ b/tests/unit/test_auth_service.py @@ -3,7 +3,7 @@ def test_hash_and_verify_roundtrip(): svc = AuthService() - plain = 'Secur3P@ss!' + plain = "Secur3P@ss!" hashed = svc.hash_password(plain) assert isinstance(hashed, str) and len(hashed) > 0 verified, new_hash = svc.verify_password(plain, hashed) diff --git a/tests/unit/test_proposal_update.py b/tests/unit/test_proposal_update.py index aa57a33..3efe0e1 100644 --- a/tests/unit/test_proposal_update.py +++ b/tests/unit/test_proposal_update.py @@ -4,14 +4,14 @@ def test_update_proposal_post(test_app, test_db): # Create a proposal in the test DB proposal = Proposal( - stream_url='https://stream.example.com/test', - name='Old Name', - website_url='https://old.example.com', + stream_url="https://stream.example.com/test", + name="Old Name", + website_url="https://old.example.com", stream_type_id=1, is_secure=False, - country='OldCountry', - description='Old description', - image_url='https://old.example.com/img.png' + country="OldCountry", + description="Old description", + image_url="https://old.example.com/img.png", ) test_db.add(proposal) test_db.commit() @@ -19,24 +19,26 @@ def test_update_proposal_post(test_app, test_db): # Prepare updated data data = { - 'name': 'New Name', - 'website_url': 'https://new.example.com', - 'country': 'Italy', - 'description': 'New description', - 'image_url': 'https://new.example.com/img.png' + "name": "New Name", + "website_url": "https://new.example.com", + "country": "Italy", + "description": "New description", + "image_url": "https://new.example.com/img.png", } # Register blueprint so url_for('proposal.index') resolves during the view from route.proposal_route import proposal_bp - # register only if not present to avoid "register_blueprint after first request" errors if proposal_bp.name not in test_app.blueprints: test_app.register_blueprint(proposal_bp) # Call the view function within a request context - with test_app.test_request_context(f'/proposal/{proposal.id}', method='POST', data=data): + with test_app.test_request_context( + f"/proposal/{proposal.id}", method="POST", data=data + ): from route.proposal_route import proposal_detail + resp = proposal_detail(proposal.id) # Expect a redirect response to proposals index @@ -45,8 +47,8 @@ def test_update_proposal_post(test_app, test_db): # Reload from DB and assert changes updated = test_db.query(Proposal).filter(Proposal.id == proposal.id).first() assert updated is not None - assert updated.name == 'New Name' - assert updated.website_url == 'https://new.example.com' - assert updated.country == 'Italy' - assert updated.description == 'New description' - assert updated.image_url == 'https://new.example.com/img.png' + assert updated.name == "New Name" + assert updated.website_url == "https://new.example.com" + assert updated.country == "Italy" + assert updated.description == "New description" + assert updated.image_url == "https://new.example.com/img.png" diff --git a/tests/unit/test_proposal_validation_service.py b/tests/unit/test_proposal_validation_service.py index a36874e..34c07d5 100644 --- a/tests/unit/test_proposal_validation_service.py +++ b/tests/unit/test_proposal_validation_service.py @@ -21,23 +21,30 @@ from model.entity.stream_analysis import StreamAnalysis from service.stream_analysis_service import StreamAnalysisService + @pytest.fixture def mock_proposal_repo() -> ProposalRepository: """Create mock ProposalRepository.""" return Mock(spec=ProposalRepository) + @pytest.fixture def mock_radio_source_repo() -> RadioSourceRepository: """Create mock RadioSourceRepository.""" return Mock(spec=RadioSourceRepository) + @pytest.fixture def mock_stream_analysis_service() -> StreamAnalysisService: """Create mock StreamAnalysisService.""" return Mock(spec=StreamAnalysisService) + @pytest.fixture -def validation_service(mock_proposal_repo: ProposalRepository, mock_radio_source_repo: RadioSourceRepository) -> ProposalValidationService: +def validation_service( + mock_proposal_repo: ProposalRepository, + mock_radio_source_repo: RadioSourceRepository, +) -> ProposalValidationService: """Create ProposalValidationService with mocked repositories.""" return ProposalValidationService(mock_proposal_repo, mock_radio_source_repo) @@ -45,12 +52,19 @@ def validation_service(mock_proposal_repo: ProposalRepository, mock_radio_source class TestProposalValidationService: """Test suite for ProposalValidationService.""" - def test_validate_proposal_success(self, validation_service: ProposalValidationService, mock_proposal_repo: ProposalRepository, - mock_stream_analysis_service: StreamAnalysisService, mock_radio_source_repo: RadioSourceRepository) -> None: + def test_validate_proposal_success( + self, + validation_service: ProposalValidationService, + mock_proposal_repo: ProposalRepository, + mock_stream_analysis_service: StreamAnalysisService, + mock_radio_source_repo: RadioSourceRepository, + ) -> None: """Test validation succeeds for valid proposal.""" - - url="https://stream.example.com/radio.mp3" - mock_analysis_result: StreamAnalysisResult = mock_stream_analysis_service.analyze_stream(url).return_value + + url = "https://stream.example.com/radio.mp3" + mock_analysis_result: StreamAnalysisResult = ( + mock_stream_analysis_service.analyze_stream(url).return_value + ) mock_analysis_result.success = True mock_analysis_result.content_type = "audio/mpeg" mock_analysis_result.raw_output = "HTTP/1.1 200 OK\nContent-Type: audio/mpeg" @@ -67,8 +81,9 @@ def test_validate_proposal_success(self, validation_service: ProposalValidationS name="Test Radio", website_url="http://example.com", stream_type_id=mock_analysis_result.stream_type_id, - is_secure=True) - + is_secure=True, + ) + mock_proposal_repo.find_by_id.return_value = proposal mock_radio_source_repo.find_by_url.return_value = None # No duplicate mock_stream_analysis_service.analyze_stream.return_value = mock_analysis_result @@ -81,7 +96,11 @@ def test_validate_proposal_success(self, validation_service: ProposalValidationS assert len(result.errors) == 0 assert len(result.warnings) == 0 - def test_validate_proposal_missing_required_fields(self, validation_service: ProposalValidationService, mock_proposal_repo: ProposalRepository): + def test_validate_proposal_missing_required_fields( + self, + validation_service: ProposalValidationService, + mock_proposal_repo: ProposalRepository, + ): """Test validation fails for missing required fields.""" # Arrange proposal = Proposal( @@ -90,7 +109,7 @@ def test_validate_proposal_missing_required_fields(self, validation_service: Pro name="Test Radio", website_url="https://example.com", stream_type_id=1, - is_secure=True + is_secure=True, ) mock_proposal_repo.find_by_id.return_value = proposal @@ -101,7 +120,11 @@ def test_validate_proposal_missing_required_fields(self, validation_service: Pro assert not result.is_valid assert "Stream URL is required and cannot be empty" in result.errors - def test_validate_proposal_invalid_url_format(self, validation_service: ProposalValidationService, mock_proposal_repo: ProposalRepository): + def test_validate_proposal_invalid_url_format( + self, + validation_service: ProposalValidationService, + mock_proposal_repo: ProposalRepository, + ): """Test validation fails for invalid URL format.""" # Arrange proposal = Proposal( @@ -110,7 +133,7 @@ def test_validate_proposal_invalid_url_format(self, validation_service: Proposal name="Test Radio", website_url="https://example.com", stream_type_id=1, - is_secure=True + is_secure=True, ) mock_proposal_repo.find_by_id.return_value = proposal @@ -121,7 +144,12 @@ def test_validate_proposal_invalid_url_format(self, validation_service: Proposal assert not result.is_valid assert "Invalid stream URL format: not-a-valid-url" in result.errors - def test_validate_proposal_duplicate_stream_url(self, validation_service: ProposalValidationService, mock_proposal_repo: ProposalRepository, mock_radio_source_repo: RadioSourceRepository): + def test_validate_proposal_duplicate_stream_url( + self, + validation_service: ProposalValidationService, + mock_proposal_repo: ProposalRepository, + mock_radio_source_repo: RadioSourceRepository, + ): """Test validation fails for duplicate stream URL.""" # Arrange proposal = Proposal( @@ -130,10 +158,17 @@ def test_validate_proposal_duplicate_stream_url(self, validation_service: Propos name="Test Radio", website_url="https://example.com", stream_type_id=1, - is_secure=True + is_secure=True, ) mock_proposal_repo.find_by_id.return_value = proposal - mock_radio_source_repo.find_by_url.return_value = RadioSource(id=1, stream_url="https://stream.example.com/radio.mp3", name="Existing Radio", website_url="https://existing.com", stream_type_id=1, is_secure=True) + mock_radio_source_repo.find_by_url.return_value = RadioSource( + id=1, + stream_url="https://stream.example.com/radio.mp3", + name="Existing Radio", + website_url="https://existing.com", + stream_type_id=1, + is_secure=True, + ) # Act result = validation_service.validate_proposal(1) @@ -142,7 +177,12 @@ def test_validate_proposal_duplicate_stream_url(self, validation_service: Propos assert not result.is_valid assert "This stream URL already exists in the database" in result.errors - def test_validate_proposal_insecure_stream_warning(self, validation_service: ProposalValidationService, mock_proposal_repo: ProposalRepository, mock_radio_source_repo: RadioSourceRepository): + def test_validate_proposal_insecure_stream_warning( + self, + validation_service: ProposalValidationService, + mock_proposal_repo: ProposalRepository, + mock_radio_source_repo: RadioSourceRepository, + ): """Test validation warns for insecure HTTP streams.""" # Arrange proposal = Proposal( @@ -151,7 +191,7 @@ def test_validate_proposal_insecure_stream_warning(self, validation_service: Pro name="Insecure Radio", website_url="https://insecure.com", stream_type_id=1, - is_secure=False + is_secure=False, ) mock_proposal_repo.find_by_id.return_value = proposal mock_radio_source_repo.find_by_url.return_value = None @@ -163,7 +203,11 @@ def test_validate_proposal_insecure_stream_warning(self, validation_service: Pro assert result.is_valid # Still valid, just a warning assert "This stream uses HTTP (not secure)" in result.warnings - def test_validate_proposal_nonexistent_proposal(self, validation_service: ProposalValidationService, mock_proposal_repo: ProposalRepository): + def test_validate_proposal_nonexistent_proposal( + self, + validation_service: ProposalValidationService, + mock_proposal_repo: ProposalRepository, + ): """Test validation fails for nonexistent proposal.""" # Arrange mock_proposal_repo.find_by_id.return_value = None @@ -175,46 +219,67 @@ def test_validate_proposal_nonexistent_proposal(self, validation_service: Propos assert not result.is_valid assert "Proposal with ID 999 not found" in result.errors - def test_validate_url_format_valid_urls(self, validation_service: ProposalValidationService): + def test_validate_url_format_valid_urls( + self, validation_service: ProposalValidationService + ): """Test URL format validation for valid URLs.""" valid_urls = [ "https://stream.example.com/radio.mp3", "http://stream.example.com:8000/stream.aac", - "https://example.com/playlist.m3u8" + "https://example.com/playlist.m3u8", ] for url in valid_urls: assert validation_service._is_valid_url(url), f"URL should be valid: {url}" - def test_validate_url_format_invalid_urls(self, validation_service: ProposalValidationService): + def test_validate_url_format_invalid_urls( + self, validation_service: ProposalValidationService + ): """Test URL format validation for invalid URLs.""" - invalid_urls = [ - "not-a-url", - "rtmp://stream.example.com/live", - "" - ] + invalid_urls = ["not-a-url", "rtmp://stream.example.com/live", ""] for url in invalid_urls: - assert not validation_service._is_valid_url(url), f"URL should be invalid: {url}" - - def test_check_duplicate_stream_url_no_duplicate(self, validation_service: ProposalValidationService, mock_radio_source_repo: RadioSourceRepository): + assert not validation_service._is_valid_url( + url + ), f"URL should be invalid: {url}" + + def test_check_duplicate_stream_url_no_duplicate( + self, + validation_service: ProposalValidationService, + mock_radio_source_repo: RadioSourceRepository, + ): """Test duplicate check when no duplicate exists.""" # Arrange mock_radio_source_repo.find_by_url.return_value = None # Act - is_duplicate = validation_service.check_duplicate_stream_url("https://stream.example.com/radio.mp3") + is_duplicate = validation_service.check_duplicate_stream_url( + "https://stream.example.com/radio.mp3" + ) # Assert assert not is_duplicate - def test_check_duplicate_stream_url_duplicate_exists(self, validation_service: ProposalValidationService, mock_radio_source_repo: RadioSourceRepository): + def test_check_duplicate_stream_url_duplicate_exists( + self, + validation_service: ProposalValidationService, + mock_radio_source_repo: RadioSourceRepository, + ): """Test duplicate check when duplicate exists.""" # Arrange - mock_radio_source_repo.find_by_url.return_value = RadioSource(id=1, stream_url="https://stream.example.com/radio.mp3", name="Existing Radio", website_url="https://existing.com", stream_type_id=1, is_secure=True) + mock_radio_source_repo.find_by_url.return_value = RadioSource( + id=1, + stream_url="https://stream.example.com/radio.mp3", + name="Existing Radio", + website_url="https://existing.com", + stream_type_id=1, + is_secure=True, + ) # Act - is_duplicate = validation_service.check_duplicate_stream_url("https://stream.example.com/radio.mp3") + is_duplicate = validation_service.check_duplicate_stream_url( + "https://stream.example.com/radio.mp3" + ) # Assert - assert is_duplicate \ No newline at end of file + assert is_duplicate diff --git a/tests/unit/test_radio_source_service.py b/tests/unit/test_radio_source_service.py index 4f307f9..14f4d3c 100644 --- a/tests/unit/test_radio_source_service.py +++ b/tests/unit/test_radio_source_service.py @@ -28,24 +28,29 @@ def mock_proposal_repo() -> ProposalRepository: """Create mock ProposalRepository.""" return Mock(spec=ProposalRepository) + @pytest.fixture def mock_radio_source_repo() -> RadioSourceRepository: """Create mock RadioSourceRepository.""" return Mock(spec=RadioSourceRepository) + @pytest.fixture def mock_validation_service() -> ProposalValidationService: """Create mock ProposalValidationService.""" return Mock(spec=ProposalValidationService) + @pytest.fixture -def radio_source_service(mock_proposal_repo: ProposalRepository, - mock_radio_source_repo: RadioSourceRepository, mock_validation_service: ProposalValidationService) -> RadioSourceService: +def radio_source_service( + mock_proposal_repo: ProposalRepository, + mock_radio_source_repo: RadioSourceRepository, + mock_validation_service: ProposalValidationService, +) -> RadioSourceService: """Create RadioSourceService with mocked dependencies.""" return RadioSourceService( - mock_proposal_repo, - mock_radio_source_repo, - mock_validation_service) + mock_proposal_repo, mock_radio_source_repo, mock_validation_service + ) class TestRadioSourceService: @@ -56,7 +61,7 @@ def test_save_from_proposal_success( radio_source_service, mock_proposal_repo, mock_radio_source_repo, - mock_validation_service + mock_validation_service, ): """Test successfully saving a valid proposal as RadioSource.""" @@ -70,7 +75,7 @@ def test_save_from_proposal_success( is_secure=True, country="Italy", description="Test description", - image_url="test.jpg" + image_url="test.jpg", ) validation_result = ValidationResult(is_valid=True) @@ -87,7 +92,7 @@ def test_save_from_proposal_success( country="Italy", description="Test description", image_url="test.jpg", - created_at=datetime.now() + created_at=datetime.now(), ) # Act @@ -102,27 +107,23 @@ def test_save_from_proposal_success( mock_proposal_repo.delete.assert_called_once_with(1) def test_save_from_proposal_validation_failure( - self, - radio_source_service, - mock_validation_service + self, radio_source_service, mock_validation_service ): """Test saving fails when validation fails.""" # Arrange validation_result = ValidationResult( - is_valid=False, - errors=["Stream URL is required"] + is_valid=False, errors=["Stream URL is required"] ) mock_validation_service.validate_proposal.return_value = validation_result # Act & Assert - with pytest.raises(ValueError, match="Proposal validation failed: Stream URL is required"): + with pytest.raises( + ValueError, match="Proposal validation failed: Stream URL is required" + ): radio_source_service.save_from_proposal(1) def test_save_from_proposal_not_found( - self, - radio_source_service, - mock_proposal_repo, - mock_validation_service + self, radio_source_service, mock_proposal_repo, mock_validation_service ): """Test saving fails when proposal not found.""" # Arrange @@ -134,11 +135,7 @@ def test_save_from_proposal_not_found( with pytest.raises(ValueError, match="Proposal with ID 1 not found"): radio_source_service.save_from_proposal(1) - def test_reject_proposal_success( - self, - radio_source_service, - mock_proposal_repo - ): + def test_reject_proposal_success(self, radio_source_service, mock_proposal_repo): """Test successfully rejecting a proposal.""" # Arrange proposal = Proposal( @@ -147,7 +144,7 @@ def test_reject_proposal_success( name="Test Radio", website_url="https://example.com", stream_type_id=1, - is_secure=True + is_secure=True, ) mock_proposal_repo.find_by_id.return_value = proposal @@ -158,11 +155,7 @@ def test_reject_proposal_success( assert result mock_proposal_repo.delete.assert_called_once_with(1) - def test_reject_proposal_not_found( - self, - radio_source_service, - mock_proposal_repo - ): + def test_reject_proposal_not_found(self, radio_source_service, mock_proposal_repo): """Test rejecting fails when proposal not found.""" # Arrange mock_proposal_repo.find_by_id.return_value = None @@ -173,18 +166,14 @@ def test_reject_proposal_not_found( # Assert assert not result - def test_update_proposal_success( - self, - radio_source_service, - mock_proposal_repo - ): + def test_update_proposal_success(self, radio_source_service, mock_proposal_repo): """Test successfully updating proposal data.""" # Arrange update_request = ProposalUpdateRequest( name="Updated Radio Name", website_url="https://updated.example.com", country="Updated Country", - description="Updated description" + description="Updated description", ) proposal = Proposal( @@ -195,7 +184,7 @@ def test_update_proposal_success( stream_type_id=1, is_secure=True, country="Original Country", - description="Original description" + description="Original description", ) mock_proposal_repo.find_by_id.return_value = proposal @@ -207,7 +196,7 @@ def test_update_proposal_success( stream_type_id=1, is_secure=True, country="Updated Country", - description="Updated description" + description="Updated description", ) mock_proposal_repo.save.return_value = updated_proposal @@ -220,11 +209,7 @@ def test_update_proposal_success( assert result.country == "Updated Country" assert result.description == "Updated description" - def test_update_proposal_not_found( - self, - radio_source_service, - mock_proposal_repo - ): + def test_update_proposal_not_found(self, radio_source_service, mock_proposal_repo): """Test updating fails when proposal not found.""" # Arrange update_request = ProposalUpdateRequest(name="New Name") @@ -234,11 +219,7 @@ def test_update_proposal_not_found( with pytest.raises(ValueError, match="Proposal with ID 1 not found"): radio_source_service.update_proposal(1, update_request) - def test_get_proposal( - self, - radio_source_service, - mock_proposal_repo - ): + def test_get_proposal(self, radio_source_service, mock_proposal_repo): """Test getting a proposal by ID.""" # Arrange proposal = Proposal( @@ -247,7 +228,7 @@ def test_get_proposal( name="Test Radio", website_url="https://example.com", stream_type_id=1, - is_secure=True + is_secure=True, ) mock_proposal_repo.find_by_id.return_value = proposal @@ -257,16 +238,26 @@ def test_get_proposal( # Assert assert result == proposal - def test_get_all_proposals( - self, - radio_source_service, - mock_proposal_repo - ): + def test_get_all_proposals(self, radio_source_service, mock_proposal_repo): """Test getting all proposals.""" # Arrange proposals = [ - Proposal(id=1, stream_url="url1", name="Radio 1", website_url="web1", stream_type_id=1, is_secure=True), - Proposal(id=2, stream_url="url2", name="Radio 2", website_url="web2", stream_type_id=2, is_secure=False) + Proposal( + id=1, + stream_url="url1", + name="Radio 1", + website_url="web1", + stream_type_id=1, + is_secure=True, + ), + Proposal( + id=2, + stream_url="url2", + name="Radio 2", + website_url="web2", + stream_type_id=2, + is_secure=False, + ), ] mock_proposal_repo.get_all_proposals.return_value = proposals @@ -276,11 +267,7 @@ def test_get_all_proposals( # Assert assert result == proposals - def test_reject_proposal( - self, - radio_source_service, - mock_proposal_repo - ): + def test_reject_proposal(self, radio_source_service, mock_proposal_repo): """Test successfully rejecting a proposal.""" # Arrange mock_proposal_repo.delete.return_value = True @@ -292,11 +279,7 @@ def test_reject_proposal( assert result is True mock_proposal_repo.delete.assert_called_once_with(1) - def test_reject_proposal_not_found( - self, - radio_source_service, - mock_proposal_repo - ): + def test_reject_proposal_not_found(self, radio_source_service, mock_proposal_repo): """Test rejecting a proposal that doesn't exist.""" # Arrange mock_proposal_repo.delete.return_value = False @@ -308,16 +291,28 @@ def test_reject_proposal_not_found( assert result is False mock_proposal_repo.delete.assert_called_once_with(1) - def test_get_all_radio_sources( - self, - radio_source_service, - mock_radio_source_repo - ): + def test_get_all_radio_sources(self, radio_source_service, mock_radio_source_repo): """Test getting all radio sources.""" # Arrange radio_sources = [ - RadioSource(id=1, stream_url="url1", name="Radio 1", website_url="web1", stream_type_id=1, is_secure=True, created_at=datetime.now()), - RadioSource(id=2, stream_url="url2", name="Radio 2", website_url="web2", stream_type_id=2, is_secure=False, created_at=datetime.now()) + RadioSource( + id=1, + stream_url="url1", + name="Radio 1", + website_url="web1", + stream_type_id=1, + is_secure=True, + created_at=datetime.now(), + ), + RadioSource( + id=2, + stream_url="url2", + name="Radio 2", + website_url="web2", + stream_type_id=2, + is_secure=False, + created_at=datetime.now(), + ), ] mock_radio_source_repo.find_all.return_value = radio_sources @@ -325,4 +320,4 @@ def test_get_all_radio_sources( result = radio_source_service.get_all_radio_sources() # Assert - assert result == radio_sources \ No newline at end of file + assert result == radio_sources diff --git a/tests/unit/test_stream_analysis_service.py b/tests/unit/test_stream_analysis_service.py index aece540..25c2d6f 100644 --- a/tests/unit/test_stream_analysis_service.py +++ b/tests/unit/test_stream_analysis_service.py @@ -31,6 +31,7 @@ def mock_stream_type_service() -> StreamTypeService: # cast to the interface/class so the fixture's return type is StreamTypeService (Pylance-friendly) return cast(StreamTypeService, mock_service) + @pytest.fixture def mock_stream_analysis_repo() -> StreamAnalysisRepository: """Mock StreamAnalysisRepository for testing.""" @@ -46,19 +47,30 @@ def mock_proposal_repo() -> ProposalRepository: @pytest.fixture -def analysis_service(mock_stream_type_service: StreamTypeService, mock_proposal_repo: ProposalRepository, mock_stream_analysis_repo: StreamAnalysisRepository) -> StreamAnalysisService: +def analysis_service( + mock_stream_type_service: StreamTypeService, + mock_proposal_repo: ProposalRepository, + mock_stream_analysis_repo: StreamAnalysisRepository, +) -> StreamAnalysisService: """Create StreamAnalysisService with mocked dependencies.""" # patch shutil.which only during construction so the constructor doesn't raise - with patch('service.stream_analysis_service.shutil.which', return_value='/usr/bin/ffmpeg'): - service = StreamAnalysisService(stream_type_service=mock_stream_type_service, - proposal_repository=mock_proposal_repo, analysis_repository=mock_stream_analysis_repo) + with patch( + "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" + ): + service = StreamAnalysisService( + stream_type_service=mock_stream_type_service, + proposal_repository=mock_proposal_repo, + analysis_repository=mock_stream_analysis_repo, + ) return service class TestStreamAnalysisService: """Test cases for StreamAnalysisService.""" - def test_unsupported_protocol_rejection(self, analysis_service: StreamAnalysisService) -> None: + def test_unsupported_protocol_rejection( + self, analysis_service: StreamAnalysisService + ) -> None: """Test FR-004: RTMP/RTSP URLs are rejected.""" rtmp_url = "rtmp://stream.example.com/live" @@ -68,79 +80,103 @@ def test_unsupported_protocol_rejection(self, analysis_service: StreamAnalysisSe assert result.error_code == ErrorCode.UNSUPPORTED_PROTOCOL assert not result.is_secure - def test_https_security_detection(self, analysis_service: StreamAnalysisService) -> None: + def test_https_security_detection( + self, analysis_service: StreamAnalysisService + ) -> None: """Test security detection for HTTPS URLs.""" https_url = "https://stream.example.com/radio.mp3" - with patch.object(analysis_service, '_analyze_with_curl') as mock_curl, \ - patch.object(analysis_service, '_analyze_with_ffmpeg') as mock_ffmpeg: + with patch.object( + analysis_service, "_analyze_with_curl" + ) as mock_curl, patch.object( + analysis_service, "_analyze_with_ffmpeg" + ) as mock_ffmpeg: # Mock successful analysis mock_curl.return_value = { "success": True, "content_type": "audio/mpeg", "raw_output": "HTTP/1.1 200 OK\\nContent-Type: audio/mpeg", - "security_status": "SAFE" + "security_status": "SAFE", } mock_ffmpeg.return_value = { "success": True, "format": "MP3", "codec": "mp3", "raw_output": "Stream #0:0: Audio: mp3 (mp3float), 22050 Hz, mono, fltp, 24 kb/s", - "security_status": "SAFE" + "security_status": "SAFE", } result = analysis_service.analyze_stream(https_url) assert result.is_secure # HTTPS should be secure - def test_http_security_warning(self, analysis_service: StreamAnalysisService) -> None: + def test_http_security_warning( + self, analysis_service: StreamAnalysisService + ) -> None: """Test FR-005: HTTP streams flagged as insecure but valid.""" http_url = "http://stream.example.com:8000/" - with patch.object(analysis_service, '_analyze_with_curl') as mock_curl, \ - patch.object(analysis_service, '_analyze_with_ffmpeg') as mock_ffmpeg: + with patch.object( + analysis_service, "_analyze_with_curl" + ) as mock_curl, patch.object( + analysis_service, "_analyze_with_ffmpeg" + ) as mock_ffmpeg: mock_curl.return_value = { "success": True, "content_type": "audio/mpeg", "raw_output": "HTTP/1.1 200 OK\\nContent-Type: audio/mpeg", - "security_status": "UNSAFE" + "security_status": "UNSAFE", } mock_ffmpeg.return_value = { "success": True, "format": "MP3", "codec": "mp3", "raw_output": "Stream #0:0: Audio: mp3", - "security_status": "UNSAFE" + "security_status": "UNSAFE", } result: StreamAnalysisResult = analysis_service.analyze_stream(http_url) assert not result.is_secure # HTTP should be insecure - assert result.is_valid # But still valid + assert result.is_valid # But still valid - @patch('subprocess.run') - def test_ffmpeg_authoritative_over_curl(self, mock_run: Mock, analysis_service: StreamAnalysisService) -> None: + @patch("subprocess.run") + def test_ffmpeg_authoritative_over_curl( + self, mock_run: Mock, analysis_service: StreamAnalysisService + ) -> None: """Test FR-003: FFmpeg is authoritative when results differ.""" # Mock curl detecting MP3, but ffmpeg detecting AAC curl_responses: list[Mock] = [ # First call: curl -I - Mock(returncode=0, stdout="HTTP/1.1 200 OK\\nContent-Type: audio/mpeg\\n", stderr=""), + Mock( + returncode=0, + stdout="HTTP/1.1 200 OK\\nContent-Type: audio/mpeg\\n", + stderr="", + ), # Second call: ffmpeg -i - Mock(returncode=0, stdout="", stderr="Stream #0:0: Audio: aac, 44100 Hz, stereo") + Mock( + returncode=0, + stdout="", + stderr="Stream #0:0: Audio: aac, 44100 Hz, stereo", + ), ] mock_run.side_effect = curl_responses - result: StreamAnalysisResult = analysis_service.analyze_stream("https://stream.example.com/test") + result: StreamAnalysisResult = analysis_service.analyze_stream( + "https://stream.example.com/test" + ) # Should use FFmpeg's AAC detection, not curl's MP3 assert result.detection_method == DetectionMethod.BOTH # The mock should have called find_stream_type_id with AAC format - def test_curl_header_extraction(self, analysis_service: StreamAnalysisService) -> None: + def test_curl_header_extraction( + self, analysis_service: StreamAnalysisService + ) -> None: """Test _extract_content_type method.""" headers = "HTTP/1.1 200 OK\\nContent-Type: audio/mpeg\\nServer: Icecast\\n" @@ -148,7 +184,9 @@ def test_curl_header_extraction(self, analysis_service: StreamAnalysisService) - print("Extracted Content-Type:", content_type) assert content_type == "audio/mpeg" - def test_ffmpeg_output_parsing(self, analysis_service: StreamAnalysisService) -> None: + def test_ffmpeg_output_parsing( + self, analysis_service: StreamAnalysisService + ) -> None: """Test _parse_ffmpeg_output method.""" ffmpeg_output = "Input #0, mp3, from 'stream':\\nStream #0:0: Audio: mp3 (mp3float), 22050 Hz, mono, fltp, 24 kb/s" @@ -157,7 +195,9 @@ def test_ffmpeg_output_parsing(self, analysis_service: StreamAnalysisService) -> assert result["format"] == "MP3" assert result["codec"] == "mp3" - def test_metadata_detection_icecast(self, analysis_service: StreamAnalysisService) -> None: + def test_metadata_detection_icecast( + self, analysis_service: StreamAnalysisService + ) -> None: """Test metadata detection for Icecast streams.""" headers = "HTTP/1.1 200 OK\\nicy-name: Test Radio\\nServer: Icecast" @@ -165,7 +205,9 @@ def test_metadata_detection_icecast(self, analysis_service: StreamAnalysisServic assert metadata == "Icecast" - def test_metadata_detection_shoutcast(self, analysis_service: StreamAnalysisService) -> None: + def test_metadata_detection_shoutcast( + self, analysis_service: StreamAnalysisService + ) -> None: """Test metadata detection for Shoutcast streams.""" headers = "HTTP/1.1 200 OK\\nServer: Shoutcast\\nicy-genre: Rock" @@ -177,22 +219,31 @@ def test_prerequisites_check_missing_ffmpeg(self): """Test NFR-001: Prerequisites check for missing ffmpeg.""" mock_service = Mock() - with patch('service.stream_analysis_service.shutil.which', return_value=None): + with patch("service.stream_analysis_service.shutil.which", return_value=None): with pytest.raises(RuntimeError, match="ffmpeg is not installed"): - StreamAnalysisService(mock_service, proposal_repository=Mock(), analysis_repository=Mock()) - - @patch('subprocess.run') - def test_timeout_handling(self, mock_run, analysis_service: StreamAnalysisService) -> None: + StreamAnalysisService( + mock_service, proposal_repository=Mock(), analysis_repository=Mock() + ) + + @patch("subprocess.run") + def test_timeout_handling( + self, mock_run, analysis_service: StreamAnalysisService + ) -> None: """Test NFR-002: Timeout handling.""" from subprocess import TimeoutExpired - mock_run.side_effect = TimeoutExpired('curl', 30) - result = analysis_service.analyze_stream("https://slow.example.com/stream", timeout_seconds=1) + mock_run.side_effect = TimeoutExpired("curl", 30) + + result = analysis_service.analyze_stream( + "https://slow.example.com/stream", timeout_seconds=1 + ) assert not result.is_valid assert result.error_code == ErrorCode.TIMEOUT - def test_extract_metadata_from_ffmpeg_output_basic(self, analysis_service: StreamAnalysisService) -> None: + def test_extract_metadata_from_ffmpeg_output_basic( + self, analysis_service: StreamAnalysisService + ) -> None: """Unit test for _extract_metadata_from_ffmpeg_output helper.""" ffmpeg_stderr = ( "Input #0, mp3, from 'stream':\n" @@ -205,7 +256,9 @@ def test_extract_metadata_from_ffmpeg_output_basic(self, analysis_service: Strea extracted = analysis_service._extract_metadata_from_ffmpeg_output(ffmpeg_stderr) assert extracted == "title: Test Title\nartist: Example Artist" - def test_analyze_stream_populates_extracted_metadata_from_ffmpeg(self, analysis_service: StreamAnalysisService) -> None: + def test_analyze_stream_populates_extracted_metadata_from_ffmpeg( + self, analysis_service: StreamAnalysisService + ) -> None: """Integration-style test: ensure analyze_stream result contains extracted_metadata.""" url = "https://stream.example.com/test" ffmpeg_stderr = ( @@ -216,13 +269,16 @@ def test_analyze_stream_populates_extracted_metadata_from_ffmpeg(self, analysis_ " Stream #0:0: Audio: mp3 (mp3float), 22050 Hz, mono" ) - with patch.object(analysis_service, '_analyze_with_curl') as mock_curl, \ - patch.object(analysis_service, '_analyze_with_ffmpeg') as mock_ffmpeg: + with patch.object( + analysis_service, "_analyze_with_curl" + ) as mock_curl, patch.object( + analysis_service, "_analyze_with_ffmpeg" + ) as mock_ffmpeg: mock_curl.return_value = { "success": True, "content_type": "audio/mpeg", - "raw_output": "HTTP/1.1 200 OK\nContent-Type: audio/mpeg" + "raw_output": "HTTP/1.1 200 OK\nContent-Type: audio/mpeg", } # Return ffmpeg result including extracted_metadata (what our helper would produce) @@ -231,17 +287,21 @@ def test_analyze_stream_populates_extracted_metadata_from_ffmpeg(self, analysis_ "format": "MP3", "codec": "mp3", "raw_output": ffmpeg_stderr, - "extracted_metadata": "title: Test Title\nartist: Example Artist" + "extracted_metadata": "title: Test Title\nartist: Example Artist", } result = analysis_service.analyze_stream(url) assert result.raw_ffmpeg_output == ffmpeg_stderr - assert result.extracted_metadata == "title: Test Title\nartist: Example Artist" + assert ( + result.extracted_metadata == "title: Test Title\nartist: Example Artist" + ) - def test_save_analysis_as_proposal_basic(self, analysis_service: StreamAnalysisService) -> None: + def test_save_analysis_as_proposal_basic( + self, analysis_service: StreamAnalysisService + ) -> None: """Unit test promoting a analysis into a proposal.""" - with patch.object(analysis_service, 'analyze_stream') as mock_stream_analysis: + with patch.object(analysis_service, "analyze_stream") as mock_stream_analysis: mock_stream_analysis.return_value = StreamAnalysisResult( stream_url="https://stream.example.com/test", stream_type_display_name="Test Stream", @@ -250,8 +310,10 @@ def test_save_analysis_as_proposal_basic(self, analysis_service: StreamAnalysisS stream_type_id=1, is_secure=True, raw_ffmpeg_output="Stream #0:0: Audio: mp3", - extracted_metadata="title: Test Title\nartist: Example Artist" + extracted_metadata="title: Test Title\nartist: Example Artist", + ) + + result: bool = analysis_service.save_analysis_as_proposal( + mock_stream_analysis.return_value ) - - result: bool = analysis_service.save_analysis_as_proposal(mock_stream_analysis.return_value) - assert result is True \ No newline at end of file + assert result is True diff --git a/tests/unit/test_stream_type_service.py b/tests/unit/test_stream_type_service.py index 2b23e72..57a4187 100644 --- a/tests/unit/test_stream_type_service.py +++ b/tests/unit/test_stream_type_service.py @@ -8,11 +8,13 @@ from service.stream_type_service import StreamTypeService from model.dto.stream_type import StreamTypeDTO + @pytest.fixture def mock_repository() -> StreamTypeRepository: """Mock StreamTypeRepository for testing.""" return Mock() + @pytest.fixture def stream_type_service(mock_repository: StreamTypeRepository) -> StreamTypeService: """Create StreamTypeService with mocked repository.""" @@ -22,16 +24,26 @@ def stream_type_service(mock_repository: StreamTypeRepository) -> StreamTypeServ class TestStreamTypeService: """Test cases for StreamTypeService.""" - def test_find_stream_type_id(self, stream_type_service: StreamTypeService, mock_repository: StreamTypeRepository): + def test_find_stream_type_id( + self, + stream_type_service: StreamTypeService, + mock_repository: StreamTypeRepository, + ): """Test finding stream type ID by combination.""" mock_repository.find_by_combination.return_value = 5 result = stream_type_service.find_stream_type_id("HTTPS", "MP3", "Icecast") assert result == 5 - mock_repository.find_by_combination.assert_called_once_with("HTTPS", "MP3", "Icecast") - - def test_find_stream_type_id_not_found(self, stream_type_service: StreamTypeService, mock_repository: StreamTypeRepository): + mock_repository.find_by_combination.assert_called_once_with( + "HTTPS", "MP3", "Icecast" + ) + + def test_find_stream_type_id_not_found( + self, + stream_type_service: StreamTypeService, + mock_repository: StreamTypeRepository, + ): """Test finding stream type ID when not found.""" mock_repository.find_by_combination.return_value = None @@ -39,7 +51,11 @@ def test_find_stream_type_id_not_found(self, stream_type_service: StreamTypeServ assert result is None - def test_get_stream_type(self, stream_type_service: StreamTypeService, mock_repository: StreamTypeRepository): + def test_get_stream_type( + self, + stream_type_service: StreamTypeService, + mock_repository: StreamTypeRepository, + ): """Test getting stream type by ID.""" # Mock the entity returned by repository mock_entity = Mock() @@ -59,7 +75,11 @@ def test_get_stream_type(self, stream_type_service: StreamTypeService, mock_repo assert result.metadata == "Icecast" assert result.display_name == "HTTPS MP3 Icecast" - def test_get_stream_type_not_found(self, stream_type_service: StreamTypeService, mock_repository: StreamTypeRepository): + def test_get_stream_type_not_found( + self, + stream_type_service: StreamTypeService, + mock_repository: StreamTypeRepository, + ): """Test getting stream type when not found.""" mock_repository.get_by_id.return_value = None @@ -67,12 +87,28 @@ def test_get_stream_type_not_found(self, stream_type_service: StreamTypeService, assert result is None - def test_get_all_stream_types(self, stream_type_service: StreamTypeService, mock_repository: StreamTypeRepository): + def test_get_all_stream_types( + self, + stream_type_service: StreamTypeService, + mock_repository: StreamTypeRepository, + ): """Test getting all stream types.""" # Mock entities mock_entities = [ - Mock(id=1, protocol="HTTP", format="MP3", metadata_type="Icecast", display_name="HTTP MP3 Icecast"), - Mock(id=2, protocol="HTTPS", format="AAC", metadata_type="Shoutcast", display_name="HTTPS AAC Shoutcast") + Mock( + id=1, + protocol="HTTP", + format="MP3", + metadata_type="Icecast", + display_name="HTTP MP3 Icecast", + ), + Mock( + id=2, + protocol="HTTPS", + format="AAC", + metadata_type="Shoutcast", + display_name="HTTPS AAC Shoutcast", + ), ] mock_repository.get_all.return_value = mock_entities @@ -83,11 +119,15 @@ def test_get_all_stream_types(self, stream_type_service: StreamTypeService, mock assert result[0].protocol == "HTTP" assert result[1].protocol == "HTTPS" - def test_get_predefined_types_map(self, stream_type_service: StreamTypeService, mock_repository: StreamTypeRepository): + def test_get_predefined_types_map( + self, + stream_type_service: StreamTypeService, + mock_repository: StreamTypeRepository, + ): """Test getting predefined types map.""" mock_repository.get_type_key_to_id_map.return_value = { "HTTP-MP3-Icecast": 1, - "HTTPS-AAC-Shoutcast": 2 + "HTTPS-AAC-Shoutcast": 2, } result = stream_type_service.get_predefined_types_map() @@ -95,7 +135,11 @@ def test_get_predefined_types_map(self, stream_type_service: StreamTypeService, assert result == {"HTTP-MP3-Icecast": 1, "HTTPS-AAC-Shoutcast": 2} mock_repository.get_type_key_to_id_map.assert_called_once() - def test_initialize_predefined_types(self, stream_type_service: StreamTypeService, mock_repository: StreamTypeRepository): + def test_initialize_predefined_types( + self, + stream_type_service: StreamTypeService, + mock_repository: StreamTypeRepository, + ): """Test initializing predefined types.""" stream_type_service.initialize_predefined_types() @@ -104,4 +148,7 @@ def test_initialize_predefined_types(self, stream_type_service: StreamTypeServic # Check one of the calls calls = mock_repository.create_if_not_exists.call_args_list - assert any(call[0] == ("HTTP", "MP3", "Icecast", "HTTP MP3 with Icecast metadata") for call in calls) \ No newline at end of file + assert any( + call[0] == ("HTTP", "MP3", "Icecast", "HTTP MP3 with Icecast metadata") + for call in calls + ) From b01317467d7a64299b9c4ea187bfc3c39bc21218 Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Fri, 5 Dec 2025 12:23:04 +0100 Subject: [PATCH 04/14] adjusted check at login in user_route --- route/auth_route.py | 14 +------------- specs/feature/feat-auth-layer-apply-roles.md | 7 +++++++ templates/index.html | 2 +- templates/proposals.html | 6 ++++++ templates/source_detail.html | 2 ++ 5 files changed, 17 insertions(+), 14 deletions(-) diff --git a/route/auth_route.py b/route/auth_route.py index e840bd4..65bf090 100644 --- a/route/auth_route.py +++ b/route/auth_route.py @@ -38,9 +38,6 @@ class LoginForm(FlaskForm): submit = SubmitField("Login") -user_repo = UserRepository() - - @auth_bp.route("/change_password", methods=["GET", "POST"]) @login_required def change_password(): @@ -63,9 +60,6 @@ def change_password(): return render_template("user/change_password.html", form=form) -user_repo = UserRepository() - - @auth_bp.route("/login", methods=["GET", "POST"]) def login(): form = LoginForm() @@ -74,13 +68,7 @@ def login(): form.email.data ) # adapt to your repo/service call - if found_user and auth_service.verify_password( - form.password.data, found_user.hash_password - ): - login_user(found_user) - flash("Signed in successfully", "success") - next_page = request.args.get("next") or url_for("main.index") - return redirect(next_page) + verified = False if found_user: res = auth_service.verify_password( diff --git a/specs/feature/feat-auth-layer-apply-roles.md b/specs/feature/feat-auth-layer-apply-roles.md index fb33136..f53a8ea 100644 --- a/specs/feature/feat-auth-layer-apply-roles.md +++ b/specs/feature/feat-auth-layer-apply-roles.md @@ -120,7 +120,14 @@ def delete_analysis(id: int): ... ``` +## Rationale of roles +- User not authenticated: listen, browse sources, view proposals, but no user or admin actions. +- Normal user (`role=user`): register/login, analyze streams, submit proposals, edit its own proposals, but no approve/reject, not edit7delete of radio sources. +- Admin user (`role=admin`): all normal user actions + approve/reject proposals, delete analyses, manage radio sources. + + ## Testing notes and expected behavior + - Admin success: approve endpoint returns redirect and DB effect (proposal created / radio source saved). - Normal user: call returns HTTP 403 (Abort), or custom error page — tests should expect 403. - Test fixtures: use `tests/conftest.py` test DB and call `AuthService.register_user(..., role='admin')` or create user directly via repository + hashed password. diff --git a/templates/index.html b/templates/index.html index 7da9f87..eda7134 100644 --- a/templates/index.html +++ b/templates/index.html @@ -13,10 +13,10 @@ RadioChWeb
Browse Sources + {% if current_user.is_authenticated %} Analyze Stream View Proposals Database - {% if current_user.is_authenticated %} Signed in as {{ current_user.email }} Change password Logout diff --git a/templates/proposals.html b/templates/proposals.html index 0a66ff6..ddf6835 100644 --- a/templates/proposals.html +++ b/templates/proposals.html @@ -48,13 +48,19 @@

Pending Proposals

Submitted: {{ proposal.created_at.strftime('%Y-%m-%d %H:%M') if proposal.created_at else 'Unknown' }}

+ {% if current_user.is_authenticated %} + {% if current_user.id == proposal.user_id or current_user.is_admin %}
Review Proposal + {% endif %} + {% if current_user.is_admin %}
+ {% endif %}
+ {% endif %} {% endfor %} diff --git a/templates/source_detail.html b/templates/source_detail.html index 21bc75c..7e9345d 100644 --- a/templates/source_detail.html +++ b/templates/source_detail.html @@ -38,11 +38,13 @@

{{ source.name }}

Listen + {% if current_user.is_authenticated and current_user.is_admin %} Edit
+ {% endif %} From 5e878c3797ec04ebb93cb3432995a2250e198ebd Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Fri, 5 Dec 2025 13:22:15 +0100 Subject: [PATCH 05/14] added relations user/proposal and user/stream_analysys --- .../V6_0__add_created_by_fields.sql | 70 ++++++++ model/entity/proposal.py | 11 +- model/entity/stream_analysis.py | 5 + model/entity/user.py | 5 + model/repository/proposal_repository.py | 10 ++ .../repository/stream_analysis_repository.py | 10 ++ route/analysis_route.py | 5 +- service/stream_analysis_service.py | 6 +- specs/feature/feat-auth-layer-apply-roles.md | 20 ++- templates/database.html | 5 +- test-output.txt | 157 ------------------ 11 files changed, 131 insertions(+), 173 deletions(-) create mode 100644 migrate_db/migrations/V6_0__add_created_by_fields.sql delete mode 100644 test-output.txt diff --git a/migrate_db/migrations/V6_0__add_created_by_fields.sql b/migrate_db/migrations/V6_0__add_created_by_fields.sql new file mode 100644 index 0000000..468b603 --- /dev/null +++ b/migrate_db/migrations/V6_0__add_created_by_fields.sql @@ -0,0 +1,70 @@ +-- V6_0__add_created_by_fields.sql +-- Rebuild `proposals` and `stream_analysis` to add `created_by` columns +-- with foreign key constraints referencing `users(id)`. + +PRAGMA foreign_keys = OFF; +BEGIN TRANSACTION; + +-- Rebuild proposals table with created_by FK +CREATE TABLE proposals_new ( + id INTEGER NOT NULL, + stream_url VARCHAR(200) NOT NULL, + name VARCHAR(200) NOT NULL, + website_url VARCHAR(200), + image_url VARCHAR(200), + stream_type_id INTEGER NOT NULL, + is_secure BOOLEAN NOT NULL DEFAULT 1, + country VARCHAR(50), + description VARCHAR(200), + created_at DATETIME, + created_by INTEGER, + PRIMARY KEY (id), + FOREIGN KEY (stream_type_id) REFERENCES stream_types(id), + FOREIGN KEY (created_by) REFERENCES users(id) +); + +-- copy existing data (created_by will be NULL for existing rows) +INSERT INTO proposals_new (id, stream_url, name, website_url, image_url, stream_type_id, is_secure, country, description, created_at) + SELECT id, stream_url, name, website_url, image_url, stream_type_id, is_secure, country, description, created_at + FROM proposals; + +DROP TABLE proposals; +ALTER TABLE proposals_new RENAME TO proposals; + +CREATE INDEX IF NOT EXISTS idx_proposals_url ON proposals(stream_url); +CREATE INDEX IF NOT EXISTS idx_proposals_stream_type_id ON proposals(stream_type_id); +CREATE INDEX IF NOT EXISTS idx_proposals_is_secure ON proposals(is_secure); +CREATE INDEX IF NOT EXISTS idx_proposals_created_by ON proposals(created_by); + +-- Rebuild stream_analysis table with created_by FK +CREATE TABLE stream_analysis_new ( + id INTEGER NOT NULL, + stream_url VARCHAR(200) NOT NULL, + stream_type_id INTEGER, + is_valid BOOLEAN NOT NULL, + is_secure BOOLEAN NOT NULL, + error_code VARCHAR(200), + detection_method VARCHAR(200), + raw_content_type TEXT NULL, + raw_ffmpeg_output TEXT NULL, + extracted_metadata TEXT NULL, + created_by INTEGER, + PRIMARY KEY (id), + FOREIGN KEY (stream_type_id) REFERENCES stream_types(id), + FOREIGN KEY (created_by) REFERENCES users(id) +); + +INSERT INTO stream_analysis_new (id, stream_url, stream_type_id, is_valid, is_secure, error_code, detection_method, raw_content_type, raw_ffmpeg_output, extracted_metadata) + SELECT id, stream_url, stream_type_id, is_valid, is_secure, error_code, detection_method, raw_content_type, raw_ffmpeg_output, extracted_metadata + FROM stream_analysis; + +DROP TABLE stream_analysis; +ALTER TABLE stream_analysis_new RENAME TO stream_analysis; + +CREATE INDEX IF NOT EXISTS idx_stream_analysis_stream_url ON stream_analysis(stream_url); +CREATE INDEX IF NOT EXISTS idx_stream_analysis_created_by ON stream_analysis(created_by); + +COMMIT; +PRAGMA foreign_keys = ON; + +-- End of migration V6_0 diff --git a/model/entity/proposal.py b/model/entity/proposal.py index 67d9af8..34756ca 100644 --- a/model/entity/proposal.py +++ b/model/entity/proposal.py @@ -11,21 +11,22 @@ class Proposal(db.Model): website_url = db.Column(db.String(500)) # Classification data from analysis - stream_type_id = db.Column( - db.Integer, db.ForeignKey("stream_types.id"), nullable=False - ) + stream_type_id = db.Column(db.Integer, db.ForeignKey("stream_types.id"), nullable=False) is_secure = db.Column(db.Boolean, nullable=False, default=False) # User-editable fields country = db.Column(db.String(50)) description = db.Column(db.Text) image_url = db.Column(db.String(500)) - + created_by = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=False) # Timestamps created_at = db.Column(db.DateTime(timezone=True), server_default=func.now()) - # Relationship + # Relationship with StreamType stream_type = db.relationship("StreamType", back_populates="proposals") + # Relationship with User + proposal_user = db.relationship("User", back_populates="proposals") + def __repr__(self): return f"" diff --git a/model/entity/stream_analysis.py b/model/entity/stream_analysis.py index 8401c51..f4836ec 100644 --- a/model/entity/stream_analysis.py +++ b/model/entity/stream_analysis.py @@ -22,9 +22,14 @@ class StreamAnalysis(db.Model): db.Text, nullable=True ) # String from ffmpeg detection extracted_metadata = db.Column(db.Text, nullable=True) + # Foreign Key to User who created the analysis + created_by = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=False) # Relationship with StreamTypes stream_type = db.relationship("StreamType", back_populates="stream_analysis") + + # Relationship with User + stream_user = db.relationship("User", back_populates="stream_analysis") def __repr__(self): return f"" diff --git a/model/entity/user.py b/model/entity/user.py index 5e17f44..0036e56 100644 --- a/model/entity/user.py +++ b/model/entity/user.py @@ -18,6 +18,11 @@ class User(db.Model): db.DateTime(timezone=True), server_default=func.now(), onupdate=func.now() ) + # Relationship with StreamAnalysis + stream_analysis = db.relationship("StreamAnalysis", back_populates="stream_user") + # Relationship with Proposal + proposals = db.relationship("Proposal", back_populates="proposal_user") + def get_id(self): return str(self.id) diff --git a/model/repository/proposal_repository.py b/model/repository/proposal_repository.py index 7f31bc2..6d8e2a9 100644 --- a/model/repository/proposal_repository.py +++ b/model/repository/proposal_repository.py @@ -56,3 +56,13 @@ def exists_by_stream_url(self, stream_url: str) -> bool: def get_all_proposals(self) -> List[Proposal]: """Retrieve all proposals from the database.""" return self.db.query(Proposal).all() + + def find_by_creator(self, user_id: int) -> List[Proposal]: + """Get all Proposals created by a specific user.""" + return ( + self.db.query(Proposal).filter(Proposal.created_by == user_id).all() + ) + + # backward-compatible alias + def find_by_created_by(self, user_id: int) -> List[Proposal]: + return self.find_by_creator(user_id) diff --git a/model/repository/stream_analysis_repository.py b/model/repository/stream_analysis_repository.py index 633ad84..85ee304 100644 --- a/model/repository/stream_analysis_repository.py +++ b/model/repository/stream_analysis_repository.py @@ -68,3 +68,13 @@ def delete(self, id: int) -> bool: self.db.commit() return True return False + + def find_by_creator(self, user_id: int) -> List[StreamAnalysis]: + """Get all StreamAnalysis rows created by a specific user.""" + return ( + self.db.query(StreamAnalysis).filter(StreamAnalysis.created_by == user_id).all() + ) + + # backward-compatible alias + def find_by_created_by(self, user_id: int) -> List[StreamAnalysis]: + return self.find_by_creator(user_id) diff --git a/route/analysis_route.py b/route/analysis_route.py index 8fde3ac..007cd02 100644 --- a/route/analysis_route.py +++ b/route/analysis_route.py @@ -5,7 +5,7 @@ from typing import List from flask import Blueprint, request, jsonify, render_template, redirect, url_for, flash -from flask_login import login_required +from flask_login import login_required, current_user from model.entity.stream_analysis import StreamAnalysis from model.dto.stream_analysis import StreamAnalysisResult from model.repository.stream_analysis_repository import StreamAnalysisRepository @@ -124,6 +124,7 @@ def analyze_url(): raw_content_type=result.raw_content_type, raw_ffmpeg_output=result.raw_ffmpeg_output, extracted_metadata=result.extracted_metadata, + created_by=current_user.id, ) analysis_repo.save(analysis_entity) @@ -140,7 +141,7 @@ def approve_analysis(id: int): try: analysis_service: StreamAnalysisService = get_stream_analysis_service() - success = analysis_service.save_analysis_as_proposal(id) + success = analysis_service.save_analysis_as_proposal(id, created_by=current_user.id) if success: flash( "ProposalAnalysis approved and added as proposal for radio source!", diff --git a/service/stream_analysis_service.py b/service/stream_analysis_service.py index d59e47b..60b500e 100644 --- a/service/stream_analysis_service.py +++ b/service/stream_analysis_service.py @@ -461,7 +461,7 @@ def _detect_metadata_support(self, headers: str) -> str: else: return "None" - def save_analysis_as_proposal(self, stream_or_id) -> bool: + def save_analysis_as_proposal(self, stream_or_id, created_by: int | None = None) -> bool: """ Approve an analysis and create a proposal for radio source. @@ -490,6 +490,9 @@ def save_analysis_as_proposal(self, stream_or_id) -> bool: print("Cannot create proposal for invalid analysis or missing data.") return False + # Determine created_by: prefer explicit arg, then analysis entity attribute if present + resolved_created_by = created_by if created_by is not None else getattr(stream_entity, "created_by", None) + proposal = Proposal( stream_url=stream_url, name="", @@ -499,6 +502,7 @@ def save_analysis_as_proposal(self, stream_or_id) -> bool: image_url=None, stream_type_id=stream_type_id, is_secure=is_secure, + created_by=resolved_created_by, created_at=date.today(), ) diff --git a/specs/feature/feat-auth-layer-apply-roles.md b/specs/feature/feat-auth-layer-apply-roles.md index f53a8ea..c2e0581 100644 --- a/specs/feature/feat-auth-layer-apply-roles.md +++ b/specs/feature/feat-auth-layer-apply-roles.md @@ -16,14 +16,22 @@ Add server-side role-based authorization (user vs admin). Use the existing `role ## Implementation Steps (concrete) 1. Branch: - `git checkout -b feat/auth-roles` -2. Domain: `model/entity/user.py` +1. Domain: `model/entity/user.py` - Add property: ```python @property def is_admin(self) -> bool: return (self.role or '').lower() == 'admin' ``` -3. Authorization helper: new `service/authorization.py` +1. Domain `model/entity/stream_analysis` and `model/entity/proposal`: + - Add relation stream_analysis -> user in database, create migration + - Add relation proposal -> user in database, create migration + This wil be made adding created_by field to both tables + +1. Domain: `model/repository/proposal_repository.py` and `model/repository/stream_analysis_repository.py` + add creator-aware query helpers and set new field `created_by` when creating analyses/proposals + +1. Authorization helper: new `service/authorization.py` - Add `admin_required` decorator: ```python from functools import wraps @@ -39,7 +47,7 @@ Add server-side role-based authorization (user vs admin). Use the existing `role return func(*args, **kwargs) return wrapper ``` -4. Protect server routes (examples) +1. Protect server routes (examples) - `route/proposal_route.py`: - Add `from service.authorization import admin_required` - Annotate: @@ -51,7 +59,7 @@ Add server-side role-based authorization (user vs admin). Use the existing `role ``` - `route/analysis_route.py`: - Protect `approve_analysis` and `delete_analysis`. -5. Templates: show admin UI (examples) +1. Templates: show admin UI (examples) - `templates/index.html` (navbar): ```jinja {% if current_user.is_authenticated and current_user.is_admin %} @@ -64,7 +72,7 @@ Add server-side role-based authorization (user vs admin). Use the existing `role
{% endif %} ``` -6. Seed CLI (new) `scripts/create_admin.py` — minimal safe script: +1. Seed CLI (new) `scripts/create_admin.py` — minimal safe script: ```python # scripts/create_admin.py import argparse @@ -92,7 +100,7 @@ Add server-side role-based authorization (user vs admin). Use the existing `role - Unit tests: - `test_user_is_admin` (verify `is_admin` behavior). - `test_admin_required_decorator` (use Flask test_request_context and a mocked `current_user` to assert abort). -8. Audit (optional) +1. Audit (optional) - Consider a simple `admin_actions` table for future auditing. Optional for MVP. ## File Summary (exact edits) diff --git a/templates/database.html b/templates/database.html index 31f5487..f4070c4 100644 --- a/templates/database.html +++ b/templates/database.html @@ -12,8 +12,7 @@
RadioChWeb
- Manage Sources - Database + Browse Sources
@@ -69,12 +68,14 @@

Database Management

{{ rs.id }} {{ rs.name }} {{ rs.stream_url }} + {% if current_user.is_authenticated and current_user.is_admin %}
+ {% endif %} {% endfor %} diff --git a/test-output.txt b/test-output.txt deleted file mode 100644 index 63e903e..0000000 --- a/test-output.txt +++ /dev/null @@ -1,157 +0,0 @@ -============================= test session starts ============================== -platform linux -- Python 3.14.0, pytest-8.0.0, pluggy-1.6.0 -- /home/riccardo/Documenti/Programming/Projects/RadioChWeb/.venv/bin/python -cachedir: .pytest_cache -rootdir: /home/riccardo/Documenti/Programming/Projects/RadioChWeb -plugins: cov-4.1.0 -collecting ... collected 53 items - -tests/integration/test_auth_flow.py::test_register_login_logout_flow FAILED [ 1%] -tests/integration/test_smoke_auth_pages.py::test_smoke_auth_pages_render FAILED [ 3%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_complete_save_workflow PASSED [ 5%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_duplicate_stream_url_prevention PASSED [ 7%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_proposal_rejection_workflow PASSED [ 9%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_proposal_update_workflow PASSED [ 11%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_validation_with_missing_required_fields PASSED [ 13%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_insecure_stream_warning PASSED [ 15%] -tests/unit/test_analysis_routes.py::test_delete_analysis_route_removes_row PASSED [ 16%] -tests/unit/test_analysis_routes.py::test_approve_analysis_route_creates_proposal PASSED [ 18%] -tests/unit/test_auth_service.py::test_hash_and_verify_roundtrip PASSED [ 20%] -tests/unit/test_proposal_update.py::test_update_proposal_post PASSED [ 22%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_success PASSED [ 24%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_missing_required_fields PASSED [ 26%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_invalid_url_format PASSED [ 28%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_duplicate_stream_url PASSED [ 30%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_insecure_stream_warning PASSED [ 32%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_nonexistent_proposal PASSED [ 33%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_url_format_valid_urls PASSED [ 35%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_url_format_invalid_urls PASSED [ 37%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_check_duplicate_stream_url_no_duplicate PASSED [ 39%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_check_duplicate_stream_url_duplicate_exists PASSED [ 41%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_save_from_proposal_success PASSED [ 43%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_save_from_proposal_validation_failure PASSED [ 45%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_save_from_proposal_not_found PASSED [ 47%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_reject_proposal_success PASSED [ 49%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_reject_proposal_not_found PASSED [ 50%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_update_proposal_success PASSED [ 52%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_update_proposal_not_found PASSED [ 54%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_get_proposal PASSED [ 56%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_get_all_proposals PASSED [ 58%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_reject_proposal PASSED [ 60%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_get_all_radio_sources PASSED [ 62%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_unsupported_protocol_rejection PASSED [ 64%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_https_security_detection PASSED [ 66%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_http_security_warning PASSED [ 67%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_ffmpeg_authoritative_over_curl PASSED [ 69%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_curl_header_extraction PASSED [ 71%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_ffmpeg_output_parsing PASSED [ 73%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_metadata_detection_icecast PASSED [ 75%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_metadata_detection_shoutcast PASSED [ 77%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_prerequisites_check_missing_ffmpeg PASSED [ 79%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_timeout_handling PASSED [ 81%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_extract_metadata_from_ffmpeg_output_basic PASSED [ 83%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_analyze_stream_populates_extracted_metadata_from_ffmpeg PASSED [ 84%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_save_analysis_as_proposal_basic PASSED [ 86%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_find_stream_type_id PASSED [ 88%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_find_stream_type_id_not_found PASSED [ 90%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_get_stream_type PASSED [ 92%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_get_stream_type_not_found PASSED [ 94%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_get_all_stream_types PASSED [ 96%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_get_predefined_types_map PASSED [ 98%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_initialize_predefined_types PASSED [100%] - -=================================== FAILURES =================================== -_______________________ test_register_login_logout_flow ________________________ - -test_app = - - def test_register_login_logout_flow(test_app): - register_blueprints_and_auth(test_app) - client = test_app.test_client() - - # Register -> resp = client.post('/auth/register', data={ - 'email': 'tester@example.com', - 'password': 'Password123', - 'confirm': 'Password123' - }, follow_redirects=True) - -tests/integration/test_auth_flow.py:29: -_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ -.venv/lib64/python3.14/site-packages/werkzeug/test.py:1167: in post - return self.open(*args, **kw) -.venv/lib64/python3.14/site-packages/flask/testing.py:232: in open - response = super().open( -.venv/lib64/python3.14/site-packages/werkzeug/test.py:1150: in open - response = self.resolve_redirect(response, buffered=buffered) -.venv/lib64/python3.14/site-packages/werkzeug/test.py:1056: in resolve_redirect - return self.open(builder, buffered=buffered) -.venv/lib64/python3.14/site-packages/flask/testing.py:232: in open - response = super().open( -.venv/lib64/python3.14/site-packages/werkzeug/test.py:1116: in open - response_parts = self.run_wsgi_app(request.environ, buffered=buffered) -.venv/lib64/python3.14/site-packages/werkzeug/test.py:988: in run_wsgi_app - rv = run_wsgi_app(self.application, environ, buffered=buffered) -.venv/lib64/python3.14/site-packages/werkzeug/test.py:1264: in run_wsgi_app - app_rv = app(environ, start_response) -.venv/lib64/python3.14/site-packages/flask/app.py:1478: in __call__ - return self.wsgi_app(environ, start_response) -.venv/lib64/python3.14/site-packages/flask/app.py:1458: in wsgi_app - response = self.handle_exception(e) -.venv/lib64/python3.14/site-packages/flask/app.py:1455: in wsgi_app - response = self.full_dispatch_request() -.venv/lib64/python3.14/site-packages/flask/app.py:869: in full_dispatch_request - rv = self.handle_user_exception(e) -.venv/lib64/python3.14/site-packages/flask/app.py:867: in full_dispatch_request - rv = self.dispatch_request() -.venv/lib64/python3.14/site-packages/flask/app.py:852: in dispatch_request - return self.ensure_sync(self.view_functions[rule.endpoint])(**view_args) -_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ - - @auth_bp.route('/login', methods=['GET', 'POST']) - def login(): -> form = LoginForm() -E NameError: name 'LoginForm' is not defined - -route/auth_route.py:58: NameError -_________________________ test_smoke_auth_pages_render _________________________ - -test_app = - - def test_smoke_auth_pages_render(test_app): -> register_blueprints(test_app) - -tests/integration/test_smoke_auth_pages.py:25: -_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ -tests/integration/test_smoke_auth_pages.py:22: in register_blueprints - AuthService(app) -service/auth_service.py:17: in __init__ - self.init_app(app) -service/auth_service.py:22: in init_app - lm.init_app(app) -.venv/lib64/python3.14/site-packages/flask_login/login_manager.py:137: in init_app - app.after_request(self._update_remember_cookie) -.venv/lib64/python3.14/site-packages/flask/sansio/scaffold.py:43: in wrapper_func - self._check_setup_finished(f_name) -_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ - -self = , f_name = 'after_request' - - def _check_setup_finished(self, f_name: str) -> None: - if self._got_first_request: -> raise AssertionError( - f"The setup method '{f_name}' can no longer be called" - " on the application. It has already handled its first" - " request, any changes will not be applied" - " consistently.\n" - "Make sure all imports, decorators, functions, etc." - " needed to set up the application are done before" - " running it." - ) -E AssertionError: The setup method 'after_request' can no longer be called on the application. It has already handled its first request, any changes will not be applied consistently. -E Make sure all imports, decorators, functions, etc. needed to set up the application are done before running it. - -.venv/lib64/python3.14/site-packages/flask/sansio/app.py:417: AssertionError -=========================== short test summary info ============================ -FAILED tests/integration/test_auth_flow.py::test_register_login_logout_flow -FAILED tests/integration/test_smoke_auth_pages.py::test_smoke_auth_pages_render -========================= 2 failed, 51 passed in 0.59s ========================= From 3e35af124e86939cab186b72f7906b9c7ca7659f Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Fri, 5 Dec 2025 15:17:11 +0100 Subject: [PATCH 06/14] user created_by managed in routes --- .../repository/stream_analysis_repository.py | 2 +- route/analysis_route.py | 4 +- route/proposal_route.py | 8 ++-- templates/index.html | 44 ++++++++++++++----- templates/proposal_detail.html | 2 +- templates/proposals.html | 10 ++--- 6 files changed, 45 insertions(+), 25 deletions(-) diff --git a/model/repository/stream_analysis_repository.py b/model/repository/stream_analysis_repository.py index 85ee304..b3b3208 100644 --- a/model/repository/stream_analysis_repository.py +++ b/model/repository/stream_analysis_repository.py @@ -2,7 +2,7 @@ StreamAnalysysRepository - Data access layer for StreamAnalysys entity. """ -from typing import Optional, List, Dict +from typing import Optional, List from sqlalchemy.orm import Session from model.entity.stream_analysis import StreamAnalysis diff --git a/route/analysis_route.py b/route/analysis_route.py index 007cd02..3da95fd 100644 --- a/route/analysis_route.py +++ b/route/analysis_route.py @@ -4,17 +4,15 @@ """ from typing import List -from flask import Blueprint, request, jsonify, render_template, redirect, url_for, flash +from flask import Blueprint, request, render_template, redirect, url_for, flash from flask_login import login_required, current_user from model.entity.stream_analysis import StreamAnalysis from model.dto.stream_analysis import StreamAnalysisResult from model.repository.stream_analysis_repository import StreamAnalysisRepository from model.repository.proposal_repository import ProposalRepository from model.repository.radio_source_repository import RadioSourceRepository -from model.entity.proposal import Proposal from model.repository.stream_type_repository import StreamTypeRepository from service.stream_analysis_service import StreamAnalysisService -from service.stream_type_service import StreamTypeService analysis_bp = Blueprint("analysis", __name__, url_prefix="/analysis") diff --git a/route/proposal_route.py b/route/proposal_route.py index 6ac66d5..80e5594 100644 --- a/route/proposal_route.py +++ b/route/proposal_route.py @@ -5,7 +5,7 @@ from typing import List from flask import Blueprint, request, render_template, redirect, url_for, flash -from flask_login import login_required +from flask_login import current_user, login_required from model.entity.radio_source import RadioSource from model.repository.proposal_repository import ProposalRepository @@ -85,11 +85,10 @@ def propose(): name = request.form.get("name") url = request.form.get("url") description = request.form.get("description", "") - user_name = request.form.get("user_name", "Anonymous") # Create proposal proposal = Proposal( - name=name, url=url, description=description, user_name=user_name + name=name, url=url, description=description, proposal_user=current_user ) # Validate and save @@ -107,8 +106,9 @@ def propose(): except Exception as e: flash(f"Error submitting proposal: {str(e)}", "error") - # After proposing or when visiting this endpoint, show the proposals listing + # After proposing show the proposals listing return redirect(url_for("proposal.index")) + @login_required diff --git a/templates/index.html b/templates/index.html index eda7134..7de0ca1 100644 --- a/templates/index.html +++ b/templates/index.html @@ -5,6 +5,18 @@ RadioChWeb - Home +
@@ -46,17 +58,29 @@

Welcome to RadioChWeb

{% for source in sources %}
-
+
-
{{ source.name }}
-

{{ source.description or 'No description available' }}

- {{ source.website_url }} - View Details - - Listen -
+
{{ source.title }}
+

{{ source.description or "No description available" }}

+ + +
+ +
+ + +
+ View Details + + Listen +
+
{% endfor %} diff --git a/templates/proposal_detail.html b/templates/proposal_detail.html index 4862cd0..1836500 100644 --- a/templates/proposal_detail.html +++ b/templates/proposal_detail.html @@ -69,7 +69,7 @@
Edit Proposal
- +
diff --git a/templates/proposals.html b/templates/proposals.html index ddf6835..01ff349 100644 --- a/templates/proposals.html +++ b/templates/proposals.html @@ -48,19 +48,17 @@

Pending Proposals

Submitted: {{ proposal.created_at.strftime('%Y-%m-%d %H:%M') if proposal.created_at else 'Unknown' }}

- {% if current_user.is_authenticated %} - {% if current_user.id == proposal.user_id or current_user.is_admin %}
+ {% if current_user.is_authenticated and (current_user.id == proposal.proposal_user.id or current_user.is_admin) %} Review Proposal - {% endif %} - {% if current_user.is_admin %} + {% endif %} + {% if current_user.is_admin %}
- {% endif %} -
{% endif %} + {% endfor %} From 800ef51d493d106887959edba2d216163bbf9210 Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Fri, 5 Dec 2025 16:12:01 +0100 Subject: [PATCH 07/14] work in progress revising tests --- tests/conftest.py | 43 +++++++++++++++- .../test_validate_and_add_workflow.py | 35 ++++++++----- tests/unit/test_analysis_routes.py | 51 +++++++++++-------- tests/unit/test_proposal_update.py | 35 +++++++------ 4 files changed, 113 insertions(+), 51 deletions(-) diff --git a/tests/conftest.py b/tests/conftest.py index 8014cc0..08cd88b 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,18 +2,20 @@ Test configuration and fixtures for RadioChWeb tests. """ +from flask_login import login_user import pytest import sys from pathlib import Path from flask import Flask from unittest.mock import Mock + # Add current directory to Python path for tests sys.path.insert(0, str(Path(__file__).parent.parent)) from database import db from model.entity.stream_type import StreamType - +from route.auth_route import login @pytest.fixture(scope="session") def test_app(): @@ -70,6 +72,45 @@ def sample_urls(): "hls_playlist": "https://stream.example.com/playlist.m3u8", } +@pytest.fixture +def test_user(test_db): + """Create a test user with role 'user'.""" + from model.entity.user import User # Assuming User model is in model.entity.user + user = User(email="testuser@example.com", hash_password="hashed_password1", role="user") # Adjust fields as per User model + test_db.add(user) + test_db.commit() + return user + + +@pytest.fixture +def admin_user(test_db): + """Create an admin user with role 'admin'.""" + from model.entity.user import User # Assuming User model is in model.entity.user + user = User(email="adminuser@example.com", hash_password="hashed_password2", role="admin") # Adjust fields as per User model + test_db.add(user) + test_db.commit() + return user + +# role="user" login helper +@pytest.fixture +def login_helper(test_app, test_user): + """Return a helper that logs test_user into a test_client by setting session keys.""" + def login(client): + # client is a Flask test_client instance + with client.session_transaction() as sess: + sess['_user_id'] = str(test_user.id) + sess['_fresh'] = True + return login + +# role="admin" login helper +@pytest.fixture +def login_admin_helper(test_app, admin_user): + """Return a helper that logs admin_user into a test_client by setting session keys.""" + def login(client): + with client.session_transaction() as sess: + sess['_user_id'] = str(admin_user.id) + sess['_fresh'] = True + return login def _initialize_stream_types(): """Initialize stream types in test database.""" diff --git a/tests/integration/test_validate_and_add_workflow.py b/tests/integration/test_validate_and_add_workflow.py index 28a01c7..5b60ec4 100644 --- a/tests/integration/test_validate_and_add_workflow.py +++ b/tests/integration/test_validate_and_add_workflow.py @@ -18,6 +18,7 @@ from service.proposal_validation_service import ProposalValidationService from service.radio_source_service import RadioSourceService from model.dto.validation import ProposalUpdateRequest +from tests.conftest import login_helper, test_app, test_user class TestValidateAndAddWorkflow: @@ -59,6 +60,7 @@ def test_complete_save_workflow(self, db_session, services): country="Italy", description="A test radio station", image_url="test.jpg", + proposal_user=test_user ) db_session.add(proposal) db_session.commit() @@ -110,6 +112,7 @@ def test_duplicate_stream_url_prevention(self, db_session, services): website_url="https://second.com", stream_type_id=1, is_secure=True, + proposal_user=test_user ) db_session.add(proposal2) db_session.commit() @@ -132,6 +135,7 @@ def test_proposal_rejection_workflow(self, db_session, services): website_url="https://reject.com", stream_type_id=1, is_secure=True, + proposal_user=test_user ) db_session.add(proposal) db_session.commit() @@ -147,19 +151,22 @@ def test_proposal_rejection_workflow(self, db_session, services): def test_proposal_update_workflow(self, db_session, services): """Test updating proposal data.""" _, radio_source_service = services - - # Create a proposal - proposal = Proposal( - stream_url="https://update.example.com/stream.mp3", - name="Original Name", - website_url="https://original.com", - stream_type_id=1, - is_secure=True, - country="Original Country", - description="Original description", - ) - db_session.add(proposal) - db_session.commit() + with test_app.test_client() as client: + login_helper(client) + + # Create a proposal + proposal = Proposal( + stream_url="https://update.example.com/stream.mp3", + name="Original Name", + website_url="https://original.com", + stream_type_id=1, + is_secure=True, + country="Original Country", + description="Original description", + proposal_user=client + ) + db_session.add(proposal) + db_session.commit() # Update the proposal update_request = ProposalUpdateRequest( @@ -192,6 +199,7 @@ def test_validation_with_missing_required_fields(self, db_session, services): website_url="https://test.com", stream_type_id=1, is_secure=True, + proposal_user=test_user ) db_session.add(proposal) db_session.commit() @@ -212,6 +220,7 @@ def test_insecure_stream_warning(self, db_session, services): website_url="https://insecure.com", stream_type_id=1, is_secure=False, + proposal_user=test_user ) db_session.add(proposal) db_session.commit() diff --git a/tests/unit/test_analysis_routes.py b/tests/unit/test_analysis_routes.py index ffb314c..4ab36e0 100644 --- a/tests/unit/test_analysis_routes.py +++ b/tests/unit/test_analysis_routes.py @@ -8,26 +8,30 @@ from model.entity.proposal import Proposal + def _register_blueprints(app): app.register_blueprint(analysis_bp, url_prefix="/analysis") app.register_blueprint(proposal_bp, url_prefix="/proposal") -def test_delete_analysis_route_removes_row(test_app, test_db): +def test_delete_analysis_route_removes_row(test_app, test_db, test_user, login_helper): # Ensure session works for flashing test_app.secret_key = "test-secret" + with test_app.test_client() as client: + login_helper(client) # Log in as test_user + + # Insert a StreamAnalysis row + sa = StreamAnalysis( + stream_url="http://test/delete", + is_valid=True, + is_secure=False, + stream_type_id=1, + stream_user=test_user + ) + test_db.add(sa) + test_db.commit() - # Insert a StreamAnalysis row - sa = StreamAnalysis( - stream_url="http://test/delete", - is_valid=True, - is_secure=False, - stream_type_id=1, - ) - test_db.add(sa) - test_db.commit() - - assert sa.id is not None + assert sa.id is not None # Patch shutil.which so constructing StreamAnalysisService inside route doesn't raise with patch( @@ -47,20 +51,23 @@ def test_delete_analysis_route_removes_row(test_app, test_db): assert found is None -def test_approve_analysis_route_creates_proposal(test_app, test_db): +def test_approve_analysis_route_creates_proposal(test_app, test_db, login_helper, test_user): # Ensure session works for flashing test_app.secret_key = "test-secret" # Insert a StreamAnalysis row with required fields - sa = StreamAnalysis( - stream_url="http://test/propose", - is_valid=True, - is_secure=True, - stream_type_id=1, - ) - test_db.add(sa) - test_db.commit() - assert sa.id is not None + with test_app.test_client() as client: + login_helper(client) # Log in as test_user + sa = StreamAnalysis( + stream_url="http://test/propose", + is_valid=True, + is_secure=True, + stream_type_id=1, + stream_user=test_user + ) + test_db.add(sa) + test_db.commit() + assert sa.id is not None with patch( "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" diff --git a/tests/unit/test_proposal_update.py b/tests/unit/test_proposal_update.py index 3efe0e1..8fcb375 100644 --- a/tests/unit/test_proposal_update.py +++ b/tests/unit/test_proposal_update.py @@ -1,21 +1,26 @@ from model.entity.proposal import Proposal -def test_update_proposal_post(test_app, test_db): - # Create a proposal in the test DB - proposal = Proposal( - stream_url="https://stream.example.com/test", - name="Old Name", - website_url="https://old.example.com", - stream_type_id=1, - is_secure=False, - country="OldCountry", - description="Old description", - image_url="https://old.example.com/img.png", - ) - test_db.add(proposal) - test_db.commit() - test_db.refresh(proposal) +def test_update_proposal_post(test_app, test_db, login_helper): + + with test_app.test_client() as client: + login_helper(client) # logs test_user into this client + + # Create a proposal in the test DB + proposal = Proposal( + stream_url="https://stream.example.com/test", + name="Old Name", + website_url="https://old.example.com", + stream_type_id=1, + is_secure=False, + country="OldCountry", + description="Old description", + image_url="https://old.example.com/img.png", + proposal_user=client + ) + test_db.add(proposal) + test_db.commit() + test_db.refresh(proposal) # Prepare updated data data = { From a81d736021cbba6b7b4e51f96a032ebb31b2f2a3 Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Sat, 6 Dec 2025 09:56:02 +0100 Subject: [PATCH 08/14] confest.py creates all entities --- test-continue.md | 74 ++++++++++++++++++++++++++++++ tests/conftest.py | 11 +++-- tests/unit/test_analysis_routes.py | 18 ++++---- tests/unit/test_proposal_update.py | 4 +- 4 files changed, 93 insertions(+), 14 deletions(-) create mode 100644 test-continue.md diff --git a/test-continue.md b/test-continue.md new file mode 100644 index 0000000..05304a1 --- /dev/null +++ b/test-continue.md @@ -0,0 +1,74 @@ +Session summary — 2025-12-05 + +Workspace / branch +- Repo: RadioChWeb +- Branch: feat/auth-layer-apply-roles +- CWD: /home/riccardo/Documenti/Programming/Projects/RadioChWeb + +What I changed (already committed in this branch) +- Database migration V6 (was created then reverted/undone by user; verify file contents before running migrations). +- Added `created_by` column in model entities: + - `model/entity/proposal.py`: `created_by = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=False)` and relationship `proposal_user`. + - `model/entity/stream_analysis.py`: `created_by = db.Column(db.Integer, db.ForeignKey("users.id"), nullable=False)` and relationship `stream_user`. +- Repository updates: + - `model/repository/proposal_repository.py`: added `find_by_creator` & `find_by_created_by` helpers. + - `model/repository/stream_analysis_repository.py`: added `find_by_creator` & `find_by_created_by` helpers. +- Route/service changes: + - `route/analysis_route.py`: set `created_by=current_user.id` when saving analyses and pass `created_by` when approving. + - `service/stream_analysis_service.py`: `save_analysis_as_proposal` accepts `created_by` arg and sets it on created `Proposal`. +- Tests/fixtures: + - `tests/conftest.py`: provides `test_user`, `admin_user`, `login_helper`, and `login_admin_helper` fixtures (login helpers set session keys). + - Several tests updated/inspected; some test fixes suggested (see failures below). + +Test runs +- Command used: `python3 -m pytest -q tests/` +- Latest result snapshot (summary lines): + - Total collected: 53 tests + - Final: 51 passed, 2 failed + - Failing tests: + 1. tests/integration/test_smoke_auth_pages.py::test_smoke_auth_pages_render + - Error: Flask AssertionError: "The setup method 'after_request' can no longer be called..." + - Root cause: `AuthService.init_app(app)` / `LoginManager.init_app(app)` was called after the app handled a request; must initialize extensions before the first request (initialize in `test_app` fixture / app factory). + 2. tests/unit/test_proposal_update.py::test_update_proposal_post + - Symptom: `image_url` did not update (assertion failure). + - Root cause: test created a `Proposal` with `proposal_user=client` (FlaskClient) instead of a `User` instance (e.g., `test_user`). Also test used `test_request_context` rather than posting through a logged-in `test_client`. Fix: assign `proposal_user=test_user` and use `client.post(...)` after `login_helper(client)`. + +Earlier run (before some fixes) showed 9 failures caused by NOT NULL `created_by` errors — these were addressed by aligning repository/service/route code to pass `created_by` from `current_user`. Keep an eye for any remaining places where tests or code create objects directly without `created_by`. + +Pending TODOs (high-level) +- Fix model-related test gaps (ensure tests create entities with `created_by` and services set created_by). +- Initialize auth-related extensions in `test_app` fixture to avoid after-request setup errors. +- After applying fixes, re-run full test suite until green. +- Optionally: add `is_admin` property and `admin_required` decorator (spec in `specs/feature/feat-auth-layer-apply-roles.md`) and add integration tests for admin flows. + +Exact commands to save this summary and to re-run tests locally +- Save summary (run once): + mkdir -p docs + cat > docs/session-2025-12-05-summary.md <<'EOF' + [paste the same summary text into STDIN if not using the above heredoc] + EOF + +- Re-run the full test suite (recommended flow): + python3 -m venv .venv + source .venv/bin/activate + python -m pip install --upgrade pip setuptools wheel + python -m pip install -r requirements.txt + python3 -m pytest -q tests/ + +Notes & quick fixes to apply next session +- In `tests/unit/test_proposal_update.py`: + - Add `test_user` fixture to test signature. + - Create the Proposal with `proposal_user=test_user` (not `client`). + - Use `with test_app.test_client() as client: login_helper(client); client.post(...)` to perform the update POST. +- In `tests/conftest.py`: + - Ensure `test_user` and `admin_user` are committed (so `.id` exists) before using `login_helper`. + - Ensure `AuthService(app)` (LoginManager init) is performed in the `test_app` fixture before any test_client usage. +- Before running schema migrations on your dev DB, back up the `instance/radio_sources.db` file. + +If you want, next session I can: +- Apply the small test fixes (update `test_proposal_update.py`, ensure `test_app` initializes `AuthService`) and re-run tests. +- Create a small backfill script or a migration plan for existing `created_by` values. + +Fix test_proposal_update.py to use test_user and post via client.post(...). +Initialize AuthService(app) inside the test_app fixture before any requests. +Re-run tests: python3 -m pytest -q tests/ and address any remaining failures. \ No newline at end of file diff --git a/tests/conftest.py b/tests/conftest.py index 08cd88b..d7f335f 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -2,23 +2,28 @@ Test configuration and fixtures for RadioChWeb tests. """ -from flask_login import login_user import pytest import sys from pathlib import Path from flask import Flask from unittest.mock import Mock - # Add current directory to Python path for tests sys.path.insert(0, str(Path(__file__).parent.parent)) +# to make tables found at db.create_all() from database import db from model.entity.stream_type import StreamType -from route.auth_route import login +from model.entity.stream_analysis import StreamAnalysis +from model.entity.user import User +from model.entity.radio_source import RadioSource +from model.entity.proposal import Proposal + @pytest.fixture(scope="session") def test_app(): + + """Create a test Flask application.""" app = Flask(__name__) app.config["TESTING"] = True diff --git a/tests/unit/test_analysis_routes.py b/tests/unit/test_analysis_routes.py index 4ab36e0..f24bf38 100644 --- a/tests/unit/test_analysis_routes.py +++ b/tests/unit/test_analysis_routes.py @@ -69,15 +69,15 @@ def test_approve_analysis_route_creates_proposal(test_app, test_db, login_helper test_db.commit() assert sa.id is not None - with patch( - "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" - ): - with patch("route.analysis_route.url_for", return_value="/"): - with test_app.test_request_context( - f"/analysis/approve/{sa.id}", method="POST" - ): - resp = approve_analysis(sa.id) - assert resp is not None + with patch( + "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" + ): + with patch("route.analysis_route.url_for", return_value="/"): + with test_app.test_request_context( + f"/analysis/approve/{sa.id}", method="POST" + ): + resp = client.post(f"/analysis/approve/{sa.id}") + assert resp is not None # Verify the StreamAnalysis row was removed after proposing found = ( diff --git a/tests/unit/test_proposal_update.py b/tests/unit/test_proposal_update.py index 8fcb375..84fb757 100644 --- a/tests/unit/test_proposal_update.py +++ b/tests/unit/test_proposal_update.py @@ -1,7 +1,7 @@ from model.entity.proposal import Proposal -def test_update_proposal_post(test_app, test_db, login_helper): +def test_update_proposal_post(test_app, test_db, test_user, login_helper): with test_app.test_client() as client: login_helper(client) # logs test_user into this client @@ -16,7 +16,7 @@ def test_update_proposal_post(test_app, test_db, login_helper): country="OldCountry", description="Old description", image_url="https://old.example.com/img.png", - proposal_user=client + proposal_user=test_user ) test_db.add(proposal) test_db.commit() From 74561ae5f8b077ae5fe87892ba0616df97b5e10b Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Sat, 6 Dec 2025 10:33:53 +0100 Subject: [PATCH 09/14] test_proposal_update ok --- tests/unit/test_analysis_routes.py | 33 +++++++++------- tests/unit/test_proposal_update.py | 63 ++++++++++++------------------ 2 files changed, 43 insertions(+), 53 deletions(-) diff --git a/tests/unit/test_analysis_routes.py b/tests/unit/test_analysis_routes.py index f24bf38..0989fa6 100644 --- a/tests/unit/test_analysis_routes.py +++ b/tests/unit/test_analysis_routes.py @@ -56,26 +56,31 @@ def test_approve_analysis_route_creates_proposal(test_app, test_db, login_helper test_app.secret_key = "test-secret" # Insert a StreamAnalysis row with required fields + sa = StreamAnalysis( + stream_url="http://test/propose", + is_valid=True, + is_secure=True, + stream_type_id=1, + stream_user=test_user + ) + + test_db.add(sa) + test_db.commit() + assert sa.id is not None + + # Ensure blueprint/extension registration happens before opening the client + from route.analysis_route import analysis_bp + if analysis_bp.name not in test_app.blueprints: + test_app.register_blueprint(analysis_bp) + + # Patch shutil.which so constructing StreamAnalysisService inside route doesn't raise with test_app.test_client() as client: login_helper(client) # Log in as test_user - sa = StreamAnalysis( - stream_url="http://test/propose", - is_valid=True, - is_secure=True, - stream_type_id=1, - stream_user=test_user - ) - test_db.add(sa) - test_db.commit() - assert sa.id is not None - with patch( "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" ): with patch("route.analysis_route.url_for", return_value="/"): - with test_app.test_request_context( - f"/analysis/approve/{sa.id}", method="POST" - ): + with test_app.test_client() as client: resp = client.post(f"/analysis/approve/{sa.id}") assert resp is not None diff --git a/tests/unit/test_proposal_update.py b/tests/unit/test_proposal_update.py index 84fb757..48ded6b 100644 --- a/tests/unit/test_proposal_update.py +++ b/tests/unit/test_proposal_update.py @@ -2,28 +2,23 @@ def test_update_proposal_post(test_app, test_db, test_user, login_helper): - - with test_app.test_client() as client: - login_helper(client) # logs test_user into this client - - # Create a proposal in the test DB - proposal = Proposal( - stream_url="https://stream.example.com/test", - name="Old Name", - website_url="https://old.example.com", - stream_type_id=1, - is_secure=False, - country="OldCountry", - description="Old description", - image_url="https://old.example.com/img.png", - proposal_user=test_user - ) - test_db.add(proposal) - test_db.commit() - test_db.refresh(proposal) - - # Prepare updated data - data = { + # Create a proposal in DB + proposal = Proposal( + stream_url="https://stream.example.com/test", + name="Old Name", + website_url="https://old.example.com", + stream_type_id=1, + is_secure=False, + country="OldCountry", + description="Old description", + image_url="https://old.example.com/img.png", + proposal_user=test_user, + ) + test_db.add(proposal) + test_db.commit() + test_db.refresh(proposal) + + data: dict[str, str] = { "name": "New Name", "website_url": "https://new.example.com", "country": "Italy", @@ -31,29 +26,19 @@ def test_update_proposal_post(test_app, test_db, test_user, login_helper): "image_url": "https://new.example.com/img.png", } - # Register blueprint so url_for('proposal.index') resolves during the view + # Ensure blueprint/extension registration happens before opening the client from route.proposal_route import proposal_bp - - # register only if not present to avoid "register_blueprint after first request" errors if proposal_bp.name not in test_app.blueprints: test_app.register_blueprint(proposal_bp) - # Call the view function within a request context - with test_app.test_request_context( - f"/proposal/{proposal.id}", method="POST", data=data - ): - from route.proposal_route import proposal_detail - - resp = proposal_detail(proposal.id) + with test_app.test_client() as client: + # Use the login helper to set the session (_user_id etc.) for this client + login_helper(client) - # Expect a redirect response to proposals index + # POST via the client so Flask-Login loads current_user + resp = client.post(f"/proposal/update/{proposal.id}", data=data, follow_redirects=False) assert resp.status_code == 302 - # Reload from DB and assert changes + # Verify DB changes after the request updated = test_db.query(Proposal).filter(Proposal.id == proposal.id).first() - assert updated is not None - assert updated.name == "New Name" - assert updated.website_url == "https://new.example.com" - assert updated.country == "Italy" - assert updated.description == "New description" assert updated.image_url == "https://new.example.com/img.png" From 97fe44aca28580a0a43c583c4c540a73f17cbded Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Sat, 6 Dec 2025 10:57:49 +0100 Subject: [PATCH 10/14] test_analysis_route ok --- tests/unit/test_analysis_routes.py | 71 ++++++++++++++---------------- 1 file changed, 34 insertions(+), 37 deletions(-) diff --git a/tests/unit/test_analysis_routes.py b/tests/unit/test_analysis_routes.py index 0989fa6..f8aa96e 100644 --- a/tests/unit/test_analysis_routes.py +++ b/tests/unit/test_analysis_routes.py @@ -9,42 +9,40 @@ -def _register_blueprints(app): - app.register_blueprint(analysis_bp, url_prefix="/analysis") - app.register_blueprint(proposal_bp, url_prefix="/proposal") - - def test_delete_analysis_route_removes_row(test_app, test_db, test_user, login_helper): # Ensure session works for flashing test_app.secret_key = "test-secret" - with test_app.test_client() as client: - login_helper(client) # Log in as test_user + + # Insert a StreamAnalysis row with required fields + sa = StreamAnalysis( + stream_url="http://test/delete", + is_valid=True, + is_secure=False, + stream_type_id=1, + stream_user=test_user # a user can delete analyze that have created + ) + test_db.add(sa) + test_db.commit() + assert sa.id is not None - # Insert a StreamAnalysis row - sa = StreamAnalysis( - stream_url="http://test/delete", - is_valid=True, - is_secure=False, - stream_type_id=1, - stream_user=test_user - ) - test_db.add(sa) - test_db.commit() - - assert sa.id is not None + # Ensure blueprint/extension registration happens before opening the client + from route.analysis_route import analysis_bp + if analysis_bp.name not in test_app.blueprints: + test_app.register_blueprint(analysis_bp) - # Patch shutil.which so constructing StreamAnalysisService inside route doesn't raise + +# Patch shutil.which so constructing StreamAnalysisService inside route doesn't raise with patch( "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" ): - # Patch url_for used inside the view to avoid needing registered endpoints - with patch("route.analysis_route.url_for", return_value="/"): - with test_app.test_request_context( - f"/analysis/delete/{sa.id}", method="POST" - ): - resp = delete_analysis(sa.id) - # view returns a redirect response object (or response) - assert resp is not None + + with test_app.test_client() as client: + login_helper(client) # Log in as test_user + resp = client.post(f"/analysis/delete/{sa.id}") + print(resp.data) + + # view returns a redirect response object (or response) + assert resp is not None # Verify the row was deleted found = test_db.query(StreamAnalysis).filter(StreamAnalysis.id == sa.id).first() @@ -74,15 +72,14 @@ def test_approve_analysis_route_creates_proposal(test_app, test_db, login_helper test_app.register_blueprint(analysis_bp) # Patch shutil.which so constructing StreamAnalysisService inside route doesn't raise - with test_app.test_client() as client: - login_helper(client) # Log in as test_user - with patch( - "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" - ): - with patch("route.analysis_route.url_for", return_value="/"): - with test_app.test_client() as client: - resp = client.post(f"/analysis/approve/{sa.id}") - assert resp is not None + with patch( + "service.stream_analysis_service.shutil.which", return_value="/usr/bin/ffmpeg" + ): + with patch("route.analysis_route.url_for", return_value="/"): + with test_app.test_client() as client: + login_helper(client) # Log in as test_user + resp = client.post(f"/analysis/approve/{sa.id}") + assert resp is not None # Verify the StreamAnalysis row was removed after proposing found = ( From 7526a4f8d9910f5a70e8716ca9fb9bc61f593f62 Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Sat, 6 Dec 2025 11:25:38 +0100 Subject: [PATCH 11/14] unit test --- pytest-output.txt | 167 ----------------------------- tests/conftest.py | 35 ++++-- tests/unit/test_analysis_routes.py | 4 - 3 files changed, 27 insertions(+), 179 deletions(-) delete mode 100644 pytest-output.txt diff --git a/pytest-output.txt b/pytest-output.txt deleted file mode 100644 index 71cd260..0000000 --- a/pytest-output.txt +++ /dev/null @@ -1,167 +0,0 @@ -============================= test session starts ============================== -platform linux -- Python 3.14.0, pytest-8.0.0, pluggy-1.6.0 -- /home/riccardo/Documenti/Programming/Projects/RadioChWeb/.venv/bin/python -cachedir: .pytest_cache -rootdir: /home/riccardo/Documenti/Programming/Projects/RadioChWeb -plugins: cov-4.1.0 -collecting ... collected 53 items - -tests/integration/test_auth_flow.py::test_register_login_logout_flow PASSED [ 1%] -tests/integration/test_smoke_auth_pages.py::test_smoke_auth_pages_render FAILED [ 3%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_complete_save_workflow PASSED [ 5%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_duplicate_stream_url_prevention PASSED [ 7%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_proposal_rejection_workflow PASSED [ 9%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_proposal_update_workflow PASSED [ 11%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_validation_with_missing_required_fields PASSED [ 13%] -tests/integration/test_validate_and_add_workflow.py::TestValidateAndAddWorkflow::test_insecure_stream_warning PASSED [ 15%] -tests/unit/test_analysis_routes.py::test_delete_analysis_route_removes_row PASSED [ 16%] -tests/unit/test_analysis_routes.py::test_approve_analysis_route_creates_proposal PASSED [ 18%] -tests/unit/test_auth_service.py::test_hash_and_verify_roundtrip PASSED [ 20%] -tests/unit/test_proposal_update.py::test_update_proposal_post FAILED [ 22%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_success PASSED [ 24%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_missing_required_fields PASSED [ 26%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_invalid_url_format PASSED [ 28%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_duplicate_stream_url PASSED [ 30%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_insecure_stream_warning PASSED [ 32%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_proposal_nonexistent_proposal PASSED [ 33%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_url_format_valid_urls PASSED [ 35%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_validate_url_format_invalid_urls PASSED [ 37%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_check_duplicate_stream_url_no_duplicate PASSED [ 39%] -tests/unit/test_proposal_validation_service.py::TestProposalValidationService::test_check_duplicate_stream_url_duplicate_exists PASSED [ 41%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_save_from_proposal_success PASSED [ 43%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_save_from_proposal_validation_failure PASSED [ 45%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_save_from_proposal_not_found PASSED [ 47%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_reject_proposal_success PASSED [ 49%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_reject_proposal_not_found PASSED [ 50%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_update_proposal_success PASSED [ 52%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_update_proposal_not_found PASSED [ 54%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_get_proposal PASSED [ 56%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_get_all_proposals PASSED [ 58%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_reject_proposal PASSED [ 60%] -tests/unit/test_radio_source_service.py::TestRadioSourceService::test_get_all_radio_sources PASSED [ 62%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_unsupported_protocol_rejection PASSED [ 64%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_https_security_detection PASSED [ 66%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_http_security_warning PASSED [ 67%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_ffmpeg_authoritative_over_curl PASSED [ 69%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_curl_header_extraction PASSED [ 71%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_ffmpeg_output_parsing PASSED [ 73%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_metadata_detection_icecast PASSED [ 75%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_metadata_detection_shoutcast PASSED [ 77%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_prerequisites_check_missing_ffmpeg PASSED [ 79%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_timeout_handling PASSED [ 81%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_extract_metadata_from_ffmpeg_output_basic PASSED [ 83%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_analyze_stream_populates_extracted_metadata_from_ffmpeg PASSED [ 84%] -tests/unit/test_stream_analysis_service.py::TestStreamAnalysisService::test_save_analysis_as_proposal_basic PASSED [ 86%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_find_stream_type_id PASSED [ 88%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_find_stream_type_id_not_found PASSED [ 90%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_get_stream_type PASSED [ 92%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_get_stream_type_not_found PASSED [ 94%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_get_all_stream_types PASSED [ 96%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_get_predefined_types_map PASSED [ 98%] -tests/unit/test_stream_type_service.py::TestStreamTypeService::test_initialize_predefined_types PASSED [100%] - -=================================== FAILURES =================================== -_________________________ test_smoke_auth_pages_render _________________________ - -test_app = - - def test_smoke_auth_pages_render(test_app): -> register_blueprints(test_app) - -tests/integration/test_smoke_auth_pages.py:23: -_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ -tests/integration/test_smoke_auth_pages.py:20: in register_blueprints - AuthService(app) -service/auth_service.py:17: in __init__ - self.init_app(app) -service/auth_service.py:22: in init_app - lm.init_app(app) -.venv/lib64/python3.14/site-packages/flask_login/login_manager.py:137: in init_app - app.after_request(self._update_remember_cookie) -.venv/lib64/python3.14/site-packages/flask/sansio/scaffold.py:43: in wrapper_func - self._check_setup_finished(f_name) -_ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ _ - -self = , f_name = 'after_request' - - def _check_setup_finished(self, f_name: str) -> None: - if self._got_first_request: -> raise AssertionError( - f"The setup method '{f_name}' can no longer be called" - " on the application. It has already handled its first" - " request, any changes will not be applied" - " consistently.\n" - "Make sure all imports, decorators, functions, etc." - " needed to set up the application are done before" - " running it." - ) -E AssertionError: The setup method 'after_request' can no longer be called on the application. It has already handled its first request, any changes will not be applied consistently. -E Make sure all imports, decorators, functions, etc. needed to set up the application are done before running it. - -.venv/lib64/python3.14/site-packages/flask/sansio/app.py:417: AssertionError -__________________________ test_update_proposal_post ___________________________ - -test_app = -test_db = - - def test_update_proposal_post(test_app, test_db): - # Create a proposal in the test DB - proposal = Proposal( - stream_url='https://stream.example.com/test', - name='Old Name', - website_url='https://old.example.com', - stream_type_id=1, - is_secure=False, - country='OldCountry', - description='Old description', - image_url='https://old.example.com/img.png' - ) - test_db.add(proposal) - test_db.commit() - test_db.refresh(proposal) - - # Prepare updated data - data = { - 'name': 'New Name', - 'website_url': 'https://new.example.com', - 'country': 'Italy', - 'description': 'New description', - 'image_url': 'https://new.example.com/img.png' - } - - # Register blueprint so url_for('proposal.index') resolves during the view - from route.proposal_route import proposal_bp - - # Register blueprint so url_for('proposal.index') resolves during the view - from route.proposal_route import proposal_bp - # register only if not present to avoid "register_blueprint after first request" errors - if proposal_bp.name not in test_app.blueprints: - test_app.register_blueprint(proposal_bp) - - # Call the view function within a request context - with test_app.test_request_context(f'/proposal/{proposal.id}', method='POST', data=data): - from route.proposal_route import proposal_detail - resp = proposal_detail(proposal.id) - - # Expect a redirect response to proposals index - assert resp.status_code == 302 - - # Reload from DB and assert changes - updated = test_db.query(Proposal).filter(Proposal.id == proposal.id).first() - assert updated is not None - assert updated.name == 'New Name' - assert updated.website_url == 'https://new.example.com' - assert updated.country == 'Italy' - assert updated.description == 'New description' -> assert updated.image_url == 'https://new.example.com/img.png' -E AssertionError: assert 'https://old.example.com/img.png' == 'https://new.example.com/img.png' -E -E - https://new.example.com/img.png -E ? ^^^ -E + https://old.example.com/img.png -E ? ^^^ - -tests/unit/test_proposal_update.py:53: AssertionError -=========================== short test summary info ============================ -FAILED tests/integration/test_smoke_auth_pages.py::test_smoke_auth_pages_render -FAILED tests/unit/test_proposal_update.py::test_update_proposal_post - Assert... -========================= 2 failed, 51 passed in 0.63s ========================= diff --git a/tests/conftest.py b/tests/conftest.py index d7f335f..86f24f2 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -79,23 +79,42 @@ def sample_urls(): @pytest.fixture def test_user(test_db): - """Create a test user with role 'user'.""" - from model.entity.user import User # Assuming User model is in model.entity.user - user = User(email="testuser@example.com", hash_password="hashed_password1", role="user") # Adjust fields as per User model + """Create (or return existing) a test user with role 'user'.""" + from model.entity.user import User + + existing = test_db.query(User).filter_by(email="testuser@example.com").first() + if existing: + return existing + + user = User( + email="testuser@example.com", + hash_password="hashed_password1", + role="user", + ) test_db.add(user) - test_db.commit() + test_db.flush() # assign id without committing return user @pytest.fixture def admin_user(test_db): - """Create an admin user with role 'admin'.""" - from model.entity.user import User # Assuming User model is in model.entity.user - user = User(email="adminuser@example.com", hash_password="hashed_password2", role="admin") # Adjust fields as per User model + """Create (or return existing) an admin user with role 'admin'.""" + from model.entity.user import User + + existing = test_db.query(User).filter_by(email="adminuser@example.com").first() + if existing: + return existing + + user = User( + email="adminuser@example.com", + hash_password="hashed_password2", + role="admin", + ) test_db.add(user) - test_db.commit() + test_db.flush() return user + # role="user" login helper @pytest.fixture def login_helper(test_app, test_user): diff --git a/tests/unit/test_analysis_routes.py b/tests/unit/test_analysis_routes.py index f8aa96e..1d10f7b 100644 --- a/tests/unit/test_analysis_routes.py +++ b/tests/unit/test_analysis_routes.py @@ -1,9 +1,5 @@ -import pytest from unittest.mock import patch -from route.analysis_route import analysis_bp, delete_analysis, approve_analysis -from route.proposal_route import proposal_bp -from database import db from model.entity.stream_analysis import StreamAnalysis from model.entity.proposal import Proposal From b75c4c15e904b1d34a2da322cae91661eb7f5ed7 Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Sat, 6 Dec 2025 12:23:50 +0100 Subject: [PATCH 12/14] =?UTF-8?q?preserve=20nested=20transaction=20?= =?UTF-8?q?=E2=80=94=20use=20flush()=20in=20tests=20and=20correct=20passin?= =?UTF-8?q?g=20parameters=20of=20fixtures=20in=20unit=20and=20integration?= =?UTF-8?q?=20tests?= MIME-Version: 1.0 Content-Type: text/plain; charset=UTF-8 Content-Transfer-Encoding: 8bit --- tests/integration/test_smoke_auth_pages.py | 1 - .../test_validate_and_add_workflow.py | 94 +++++++++---------- tests/unit/test_proposal_update.py | 2 +- 3 files changed, 47 insertions(+), 50 deletions(-) diff --git a/tests/integration/test_smoke_auth_pages.py b/tests/integration/test_smoke_auth_pages.py index e512e98..a4fc48b 100644 --- a/tests/integration/test_smoke_auth_pages.py +++ b/tests/integration/test_smoke_auth_pages.py @@ -10,7 +10,6 @@ def register_blueprints(app): from route.radio_source_route import radio_source_bp from route.listen_route import listen_bp from route.auth_route import auth_bp - from service.auth_service import AuthService app.template_folder = str(Path(__file__).parents[2] / "templates") # Register blueprints only if they are not already registered (idempotent) diff --git a/tests/integration/test_validate_and_add_workflow.py b/tests/integration/test_validate_and_add_workflow.py index 5b60ec4..694fdc8 100644 --- a/tests/integration/test_validate_and_add_workflow.py +++ b/tests/integration/test_validate_and_add_workflow.py @@ -12,28 +12,22 @@ import pytest from model.entity.proposal import Proposal from model.entity.radio_source import RadioSource -from model.entity.stream_type import StreamType from model.repository.proposal_repository import ProposalRepository from model.repository.radio_source_repository import RadioSourceRepository from service.proposal_validation_service import ProposalValidationService from service.radio_source_service import RadioSourceService from model.dto.validation import ProposalUpdateRequest -from tests.conftest import login_helper, test_app, test_user class TestValidateAndAddWorkflow: """Integration tests for the complete validate and add workflow.""" - @pytest.fixture(scope="function") - def db_session(self, test_db): - """Use the test database session from conftest.""" - return test_db @pytest.fixture - def repositories(self, db_session): + def repositories(self, test_db): """Create repositories with test session.""" - proposal_repo = ProposalRepository(db_session) - radio_source_repo = RadioSourceRepository(db_session) + proposal_repo = ProposalRepository(test_db) + radio_source_repo = RadioSourceRepository(test_db) return proposal_repo, radio_source_repo @pytest.fixture @@ -46,7 +40,8 @@ def services(self, repositories): ) return validation_service, radio_source_service - def test_complete_save_workflow(self, db_session, services): + + def test_complete_save_workflow(self, test_db, test_user, services): """Test the complete workflow: create proposal -> validate -> save as radio source.""" validation_service, radio_source_service = services @@ -62,8 +57,8 @@ def test_complete_save_workflow(self, db_session, services): image_url="test.jpg", proposal_user=test_user ) - db_session.add(proposal) - db_session.commit() + test_db.add(proposal) + test_db.flush() # Validate the proposal validation_result = validation_service.validate_proposal(proposal.id) @@ -75,18 +70,18 @@ def test_complete_save_workflow(self, db_session, services): assert save_result.stream_url == "https://stream.example.com/radio.mp3" # Verify proposal was deleted and radio source was created - saved_proposal = db_session.query(Proposal).filter_by(id=proposal.id).first() + saved_proposal = test_db.query(Proposal).filter_by(id=proposal.id).first() assert saved_proposal is None saved_radio_source = ( - db_session.query(RadioSource) + test_db.query(RadioSource) .filter_by(stream_url="https://stream.example.com/radio.mp3") .first() ) assert saved_radio_source is not None assert saved_radio_source.name == "Test Radio Station" - def test_duplicate_stream_url_prevention(self, db_session, services): + def test_duplicate_stream_url_prevention(self, test_db, test_user, services): """Test that duplicate stream URLs are prevented.""" validation_service, radio_source_service = services @@ -97,9 +92,10 @@ def test_duplicate_stream_url_prevention(self, db_session, services): website_url="https://first.com", stream_type_id=1, is_secure=True, + proposal_user=test_user ) - db_session.add(proposal1) - db_session.commit() + test_db.add(proposal1) + test_db.flush() # Save first proposal save_result1 = radio_source_service.save_from_proposal(proposal1.id) @@ -114,8 +110,9 @@ def test_duplicate_stream_url_prevention(self, db_session, services): is_secure=True, proposal_user=test_user ) - db_session.add(proposal2) - db_session.commit() + test_db.add(proposal2) + + test_db.flush() # Validation should fail due to duplicate validation_result = validation_service.validate_proposal(proposal2.id) @@ -124,10 +121,10 @@ def test_duplicate_stream_url_prevention(self, db_session, services): "This stream URL already exists in the database" in validation_result.errors ) - def test_proposal_rejection_workflow(self, db_session, services): + def test_proposal_rejection_workflow(self, test_db, test_user, services): """Test rejecting a proposal.""" _, radio_source_service = services - + # Create a proposal proposal = Proposal( stream_url="https://reject.example.com/stream.mp3", @@ -137,36 +134,35 @@ def test_proposal_rejection_workflow(self, db_session, services): is_secure=True, proposal_user=test_user ) - db_session.add(proposal) - db_session.commit() + test_db.add(proposal) + test_db.flush() # Reject the proposal reject_result = radio_source_service.reject_proposal(proposal.id) assert reject_result is True # Verify proposal was deleted - deleted_proposal = db_session.query(Proposal).filter_by(id=proposal.id).first() + deleted_proposal = test_db.query(Proposal).filter_by(id=proposal.id).first() assert deleted_proposal is None - def test_proposal_update_workflow(self, db_session, services): + + def test_proposal_update_workflow(self, test_db, test_user, services): """Test updating proposal data.""" _, radio_source_service = services - with test_app.test_client() as client: - login_helper(client) - - # Create a proposal - proposal = Proposal( - stream_url="https://update.example.com/stream.mp3", - name="Original Name", - website_url="https://original.com", - stream_type_id=1, - is_secure=True, - country="Original Country", - description="Original description", - proposal_user=client - ) - db_session.add(proposal) - db_session.commit() + + # Create a proposal + proposal = Proposal( + stream_url="https://update.example.com/stream.mp3", + name="Original Name", + website_url="https://original.com", + stream_type_id=1, + is_secure=True, + country="Original Country", + description="Original description", + proposal_user=test_user + ) + test_db.add(proposal) + test_db.flush() # Update the proposal update_request = ProposalUpdateRequest( @@ -182,13 +178,14 @@ def test_proposal_update_workflow(self, db_session, services): assert update_result.name == "Updated Name" # Verify the update - updated_proposal = db_session.query(Proposal).filter_by(id=proposal.id).first() + updated_proposal = test_db.query(Proposal).filter_by(id=proposal.id).first() assert updated_proposal.name == "Updated Name" assert updated_proposal.website_url == "https://updated.com" assert updated_proposal.country == "Updated Country" assert updated_proposal.description == "Updated description" - def test_validation_with_missing_required_fields(self, db_session, services): + + def test_validation_with_missing_required_fields(self, test_db, test_user, services): """Test validation fails with missing required fields.""" validation_service, _ = services @@ -201,15 +198,16 @@ def test_validation_with_missing_required_fields(self, db_session, services): is_secure=True, proposal_user=test_user ) - db_session.add(proposal) - db_session.commit() + test_db.add(proposal) + test_db.flush() # Validation should fail validation_result = validation_service.validate_proposal(proposal.id) assert not validation_result.is_valid assert "Stream URL is required and cannot be empty" in validation_result.errors - def test_insecure_stream_warning(self, db_session, services): + + def test_insecure_stream_warning(self, test_db, test_user, services): """Test warning for insecure HTTP streams.""" validation_service, _ = services @@ -222,8 +220,8 @@ def test_insecure_stream_warning(self, db_session, services): is_secure=False, proposal_user=test_user ) - db_session.add(proposal) - db_session.commit() + test_db.add(proposal) + test_db.flush() # Validation should succeed but with warning validation_result = validation_service.validate_proposal(proposal.id) diff --git a/tests/unit/test_proposal_update.py b/tests/unit/test_proposal_update.py index 48ded6b..8a05335 100644 --- a/tests/unit/test_proposal_update.py +++ b/tests/unit/test_proposal_update.py @@ -15,7 +15,7 @@ def test_update_proposal_post(test_app, test_db, test_user, login_helper): proposal_user=test_user, ) test_db.add(proposal) - test_db.commit() + test_db.flush() test_db.refresh(proposal) data: dict[str, str] = { From 8f0b916618a47bbf2966b381618e7efc2105e8f3 Mon Sep 17 00:00:00 2001 From: Riccardo Corsi Date: Sat, 6 Dec 2025 16:51:59 +0100 Subject: [PATCH 13/14] code_review_feat_auth_layer_apply_roles --- ...code_review_feat-auth_layer_apply_roles.md | 89 +++++++++++++++++++ specs/feature/feat-auth-layer-apply-roles.md | 20 ++--- tests/conftest.py | 5 +- tests/unit/test_radio_source_service.py | 18 ++-- 4 files changed, 104 insertions(+), 28 deletions(-) create mode 100644 specs/feature/code_review_feat-auth_layer_apply_roles.md diff --git a/specs/feature/code_review_feat-auth_layer_apply_roles.md b/specs/feature/code_review_feat-auth_layer_apply_roles.md new file mode 100644 index 0000000..1e83a50 --- /dev/null +++ b/specs/feature/code_review_feat-auth_layer_apply_roles.md @@ -0,0 +1,89 @@ +# Code review – feat/auth layer apply roles + +## Overview + +- PR Phase 2 autenticazione: introdotti ruoli (user vs admin) e tracciamento dei creator per proposte e analisi. [file:1] +- Applicato Black a molti file (service, route, test, app.py). [file:1] +- Aggiornati i test per usare nuovi fixture di autenticazione e pattern con transazioni annidate. [file:1] + +--- +## Issues Identified + +* In tests/unit/test_radio_source_service.py: + +> @@ -158,11 +155,7 @@ def test_reject_proposal_success( + assert result + mock_proposal_repo.delete.assert_called_once_with(1) + +- def test_reject_proposal_not_found( +- self, +- radio_source_service, +- mock_proposal_repo +- ): ++ def test_reject_proposal_not_found(self, radio_source_service, mock_proposal_repo): +This assignment to 'test_reject_proposal_not_found' is unnecessary as it is [1]redefined before this value is used. +`OK SOLVED by removing the previous definition`. + +* In tests/conftest.py: + +> from database import db + from model.entity.stream_type import StreamType ++from model.entity.stream_analysis import StreamAnalysis +Import of 'StreamAnalysis' is not used. +`OK removed` + +* In tests/conftest.py: + +> from database import db + from model.entity.stream_type import StreamType ++from model.entity.stream_analysis import StreamAnalysis ++from model.entity.user import User +Import of 'User' is not used. + +* In tests/conftest.py: + +> from database import db + from model.entity.stream_type import StreamType ++from model.entity.stream_analysis import StreamAnalysis ++from model.entity.user import User ++from model.entity.radio_source import RadioSource +Import of 'RadioSource' is not used. + +* In tests/conftest.py: + +> from database import db + from model.entity.stream_type import StreamType ++from model.entity.stream_analysis import StreamAnalysis ++from model.entity.user import User ++from model.entity.radio_source import RadioSource ++from model.entity.proposal import Proposal +Import of 'Proposal' is not used. +`OK removed 3 entities non used` + +* In specs/feature/feat-auth-layer-apply-roles.md: + +> +1. Seed CLI (new) `scripts/create_admin.py` — minimal safe script: ++ ```python ++ # scripts/create_admin.py ++ import argparse ++ from app import app ++ from service.auth_service import AuthService ++ ++ def main(): ++ p = argparse.ArgumentParser() ++ p.add_argument('--email', required=True) ++ p.add_argument('--password', required=True) ++ args = p.parse_args() ++ ++ AuthService(app).register_user(email=args.email, password=args.password, role='admin') ++ print(f"Admin user '{args.email}' created.") ++ ++ if __name__ == '__main__': ++ main() ++ ``` ++ + +* Run locally (do not commit secrets): `python scripts/create_admin.py --email admin@example.com --password 'Strong!'` +The example scripts/create_admin.py in this spec passes the admin password via a --password command-line argument (lines 75–93 and the example invocation on line 94). Command-line arguments are visible in process listings and can be stored in shell history or other monitoring logs, which can expose the admin password to other local users or tooling. + +Instead of accepting the password via CLI args, use a secure input method such as getpass.getpass() or read it from a protected environment variable or prompt inside the script, and update the usage example to avoid putting the password directly on the command line. \ No newline at end of file diff --git a/specs/feature/feat-auth-layer-apply-roles.md b/specs/feature/feat-auth-layer-apply-roles.md index c2e0581..aea945e 100644 --- a/specs/feature/feat-auth-layer-apply-roles.md +++ b/specs/feature/feat-auth-layer-apply-roles.md @@ -13,7 +13,7 @@ Add server-side role-based authorization (user vs admin). Use the existing `role - Complex RBAC (permissions table) — defer to later if needed. - UI redesign — add minimal UI changes only (navbar badges/links). -## Implementation Steps (concrete) +## Implementation Steps (main points) 1. Branch: - `git checkout -b feat/auth-roles` 1. Domain: `model/entity/user.py` @@ -91,19 +91,15 @@ Add server-side role-based authorization (user vs admin). Use the existing `role if __name__ == '__main__': main() ``` - - Run locally (do not commit secrets): `python scripts/create_admin.py --email admin@example.com --password 'Strong!'` -7. Tests (new) - - `tests/integration/test_auth_admin_flow.py` (skeleton): - - Create admin via `AuthService.register_user(..., role='admin')` or create via the repo directly in test DB. - - Log in with test client, POST to admin-only endpoints and assert success and DB side-effect. - - Log in as normal user and assert 403 on admin endpoints. - - Unit tests: - - `test_user_is_admin` (verify `is_admin` behavior). - - `test_admin_required_decorator` (use Flask test_request_context and a mocked `current_user` to assert abort). + - Run locally (do not commit secrets): `python scripts/create_admin.py --email $ADMIN_EMAIL --password $ADMIN_PASSWORD` + +1. Tests (new) + In general use the existing test DB fixture in `tests/conftest.py` to create users with different roles and test the behavior of protected endpoints. + 1. Audit (optional) - - Consider a simple `admin_actions` table for future auditing. Optional for MVP. + - Consider a simple `admin_actions` table for future auditing. Optional for MVP. This will be made adding a new table with fields: id, admin_user_id, action_type, target_id, timestamp, in a future PR. -## File Summary (exact edits) +## File Summary (main, for exact list see PR) - `model/entity/user.py` — add `is_admin` property. - `service/authorization.py` — new. - `route/proposal_route.py` — import decorator + protect `approve_proposal`. diff --git a/tests/conftest.py b/tests/conftest.py index 86f24f2..9fecd54 100644 --- a/tests/conftest.py +++ b/tests/conftest.py @@ -14,10 +14,7 @@ # to make tables found at db.create_all() from database import db from model.entity.stream_type import StreamType -from model.entity.stream_analysis import StreamAnalysis -from model.entity.user import User -from model.entity.radio_source import RadioSource -from model.entity.proposal import Proposal + @pytest.fixture(scope="session") diff --git a/tests/unit/test_radio_source_service.py b/tests/unit/test_radio_source_service.py index 14f4d3c..d0a235c 100644 --- a/tests/unit/test_radio_source_service.py +++ b/tests/unit/test_radio_source_service.py @@ -16,7 +16,7 @@ from service.proposal_validation_service import ProposalValidationService from model.entity.proposal import Proposal from model.entity.radio_source import RadioSource -from model.entity.stream_analysis import StreamAnalysis + from model.dto.validation import ProposalUpdateRequest from model.dto.validation import ValidationResult @@ -106,6 +106,7 @@ def test_save_from_proposal_success( mock_radio_source_repo.save.assert_called_once() mock_proposal_repo.delete.assert_called_once_with(1) + def test_save_from_proposal_validation_failure( self, radio_source_service, mock_validation_service ): @@ -122,6 +123,7 @@ def test_save_from_proposal_validation_failure( ): radio_source_service.save_from_proposal(1) + def test_save_from_proposal_not_found( self, radio_source_service, mock_proposal_repo, mock_validation_service ): @@ -135,6 +137,7 @@ def test_save_from_proposal_not_found( with pytest.raises(ValueError, match="Proposal with ID 1 not found"): radio_source_service.save_from_proposal(1) + def test_reject_proposal_success(self, radio_source_service, mock_proposal_repo): """Test successfully rejecting a proposal.""" # Arrange @@ -155,6 +158,7 @@ def test_reject_proposal_success(self, radio_source_service, mock_proposal_repo) assert result mock_proposal_repo.delete.assert_called_once_with(1) + def test_reject_proposal_not_found(self, radio_source_service, mock_proposal_repo): """Test rejecting fails when proposal not found.""" # Arrange @@ -166,6 +170,7 @@ def test_reject_proposal_not_found(self, radio_source_service, mock_proposal_rep # Assert assert not result + def test_update_proposal_success(self, radio_source_service, mock_proposal_repo): """Test successfully updating proposal data.""" # Arrange @@ -267,17 +272,6 @@ def test_get_all_proposals(self, radio_source_service, mock_proposal_repo): # Assert assert result == proposals - def test_reject_proposal(self, radio_source_service, mock_proposal_repo): - """Test successfully rejecting a proposal.""" - # Arrange - mock_proposal_repo.delete.return_value = True - - # Act - result = radio_source_service.reject_proposal(1) - - # Assert - assert result is True - mock_proposal_repo.delete.assert_called_once_with(1) def test_reject_proposal_not_found(self, radio_source_service, mock_proposal_repo): """Test rejecting a proposal that doesn't exist.""" From 7327928b26223f63cb854262c6a46a08685d7946 Mon Sep 17 00:00:00 2001 From: "copilot-swe-agent[bot]" <198982749+Copilot@users.noreply.github.com> Date: Sat, 6 Dec 2025 15:57:13 +0000 Subject: [PATCH 14/14] Initial plan