From db640033b0b5a67d7d3e2b965f657a951e79cdcc Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 16 May 2025 13:28:16 +0200 Subject: [PATCH 01/72] Improve type checking in oonirun --- .../oonirun/src/oonirun/dependencies.py | 4 +++- .../services/oonirun/src/oonirun/routers/v2.py | 18 +++++++++--------- 2 files changed, 12 insertions(+), 10 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/dependencies.py b/ooniapi/services/oonirun/src/oonirun/dependencies.py index 84a8c8a55..6a93b5e30 100644 --- a/ooniapi/services/oonirun/src/oonirun/dependencies.py +++ b/ooniapi/services/oonirun/src/oonirun/dependencies.py @@ -4,7 +4,7 @@ from fastapi import Depends from sqlalchemy import create_engine -from sqlalchemy.orm import sessionmaker +from sqlalchemy.orm import sessionmaker, Session from .common.config import Settings from .common.dependencies import get_settings @@ -19,3 +19,5 @@ def get_postgresql_session(settings: Annotated[Settings, Depends(get_settings)]) yield db finally: db.close() + +PostgresSession = Annotated[Session, Depends(get_postgresql_session)] \ No newline at end of file diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index d823d2880..76b2b3d02 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -22,7 +22,7 @@ from ..common.auth import ( get_account_id_or_none, ) -from ..dependencies import get_postgresql_session +from ..dependencies import PostgresSession, get_postgresql_session log = logging.getLogger(__name__) @@ -151,9 +151,9 @@ class OONIRunLinkCreateEdit(OONIRunLinkBase): ) def create_oonirun_link( create_request: OONIRunLinkCreateEdit, + db : PostgresSession, token=Depends(role_required(["admin", "user"])), - db=Depends(get_postgresql_session), -): +) -> OONIRunLink: """Create a new oonirun link or a new version for an existing one.""" log.debug("creating oonirun") account_id = token["account_id"] @@ -233,8 +233,8 @@ def create_oonirun_link( def edit_oonirun_link( oonirun_link_id: str, edit_request: OONIRunLinkCreateEdit, + db : PostgresSession, token=Depends(role_required(["admin", "user"])), - db=Depends(get_postgresql_session), ): """Edit an existing OONI Run link""" log.debug(f"edit oonirun {oonirun_link_id}") @@ -411,7 +411,7 @@ class OONIRunLinkRevisions(BaseModel): ) def get_oonirun_link_revisions( oonirun_link_id: str, - db=Depends(get_postgresql_session), + db : PostgresSession, ): """ Obtain the list of revisions for a certain OONI Run link @@ -451,7 +451,7 @@ def get_oonirun_link_engine_descriptor( }, ), ], - db=Depends(get_postgresql_session), + db : PostgresSession, ): """Fetch an OONI Run link by specifying the revision number""" try: @@ -499,8 +499,8 @@ def get_oonirun_link_revision( }, ), ], + db : PostgresSession, authorization: str = Header("authorization"), - db=Depends(get_postgresql_session), settings=Depends(get_settings), ): """Fetch an OONI Run link by specifying the revision number""" @@ -528,8 +528,8 @@ def get_oonirun_link_revision( ) def get_latest_oonirun_link( oonirun_link_id: str, + db : PostgresSession, authorization: str = Header("authorization"), - db=Depends(get_postgresql_session), settings=Depends(get_settings), ): """Fetch OONIRun descriptor by creation time or the newest one""" @@ -551,6 +551,7 @@ class OONIRunLinkList(BaseModel): @router.get("/v2/oonirun/links", tags=["oonirun"]) def list_oonirun_links( + db : PostgresSession, is_mine: Annotated[ Optional[bool], Query(description="List only the my descriptors"), @@ -560,7 +561,6 @@ def list_oonirun_links( Query(description="List also expired descriptors"), ] = None, authorization: str = Header("authorization"), - db=Depends(get_postgresql_session), settings=Depends(get_settings), ) -> OONIRunLinkList: """List OONIRun descriptors""" From b98c9c06bc26a8f5451766695c66cbf58485d46f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 21 May 2025 13:12:55 +0200 Subject: [PATCH 02/72] Added targets_name and inputs_extra parameters --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 12 +++++++++++- 1 file changed, 11 insertions(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 76b2b3d02..50d4ef52c 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -5,7 +5,7 @@ """ from datetime import datetime, timedelta, timezone -from typing import Dict, List, Optional, Tuple +from typing import Dict, List, Optional, Tuple, Any import logging import sqlalchemy as sa @@ -115,6 +115,16 @@ def validate_intl(cls, v: Dict[str, str]): description="future time after which the ooni run link will be considered expired and no longer editable or usable (defaults to 6 months from now)", ) + targets_name : Optional[str] = Field ( + default=None, + description="string used to specify during creation that the input list should be dynamically generated." + ) + + inputs_extra : Optional[List[Dict[str, Any]]] = Field ( + default = None, + description = "provides a richer JSON array containing extra parameters for each input. If provided, the length of inputs_extra should match the length of inputs." + ) + class OONIRunLink(OONIRunLinkBase): oonirun_link_id: str From 5bcc9ee7ba9f1c8aa2130bb01b07a63a6fd24644 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 21 May 2025 13:38:46 +0200 Subject: [PATCH 03/72] Moved fields to right model --- .../oonirun/src/oonirun/routers/v2.py | 20 +++++++++---------- 1 file changed, 9 insertions(+), 11 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 50d4ef52c..2ff75af07 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -51,6 +51,15 @@ class OONIRunLinkNettest(BaseModel): default=False, title="if this test should be enabled by default for manual runs" ) + targets_name : Optional[str] = Field ( + default=None, + description="string used to specify during creation that the input list should be dynamically generated." + ) + + inputs_extra : Optional[List[Dict[str, Any]]] = Field ( + default = None, + description = "provides a richer JSON array containing extra parameters for each input. If provided, the length of inputs_extra should match the length of inputs." + ) class OONIRunLinkEngineDescriptor(BaseModel): revision: str = Field(title="revision of the nettest descriptor") @@ -115,17 +124,6 @@ def validate_intl(cls, v: Dict[str, str]): description="future time after which the ooni run link will be considered expired and no longer editable or usable (defaults to 6 months from now)", ) - targets_name : Optional[str] = Field ( - default=None, - description="string used to specify during creation that the input list should be dynamically generated." - ) - - inputs_extra : Optional[List[Dict[str, Any]]] = Field ( - default = None, - description = "provides a richer JSON array containing extra parameters for each input. If provided, the length of inputs_extra should match the length of inputs." - ) - - class OONIRunLink(OONIRunLinkBase): oonirun_link_id: str date_created: datetime = Field( From d1c610c30f5b29ca457d602d645b36d7cc930c8e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 21 May 2025 15:59:40 +0200 Subject: [PATCH 04/72] Add migration for targets_name and input_extra --- ...750f_add_targets_name_and_inputs_extra_.py | 32 +++++++++++++++++++ 1 file changed, 32 insertions(+) create mode 100644 ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py diff --git a/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py new file mode 100644 index 000000000..fae75a9df --- /dev/null +++ b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py @@ -0,0 +1,32 @@ +"""Add targets_name and inputs_extra columns to oonirun_nettest + +Revision ID: b860eb79750f +Revises: 8e7ecea5c2f5 +Create Date: 2025-05-21 15:44:32.959349 + +""" +from typing import Sequence, Union + +from alembic import op +import sqlalchemy as sa + + +# revision identifiers, used by Alembic. +revision: str = 'b860eb79750f' +down_revision: Union[str, None] = '8e7ecea5c2f5' +branch_labels: Union[str, Sequence[str], None] = None +depends_on: Union[str, Sequence[str], None] = None + + +def upgrade() -> None: + op.add_column("oonirun_nettest", + sa.Column("targets_name", sa.String(), nullable=True) + ) + op.add_column("oonirun_nettest", + sa.Column("inputs_extra", sa.JSON(), nullable=True) + ) + + +def downgrade() -> None: + op.drop_column("oonirun_nettest", "targets_name") + op.drop_column("oonirun_nettest", "inputs_extra") From 6083adcc6b67cc38420c5ce4c29cb6d379f59337 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 22 May 2025 10:15:59 +0200 Subject: [PATCH 05/72] Add more tests to the sample oonirun link --- .../services/oonirun/tests/test_database.py | 23 +++++++++++++++++++ 1 file changed, 23 insertions(+) diff --git a/ooniapi/services/oonirun/tests/test_database.py b/ooniapi/services/oonirun/tests/test_database.py index 79969c58d..66cfdf316 100644 --- a/ooniapi/services/oonirun/tests/test_database.py +++ b/ooniapi/services/oonirun/tests/test_database.py @@ -45,6 +45,29 @@ "is_manual_run_enabled_default": False, "test_name": "dnscheck", }, + { + "targets_name" : "sample_target", + "options": {}, + "backend_options": {}, + "is_background_run_enabled_default": False, + "is_manual_run_enabled_default": False, + "test_name": "dnscheck", + }, + { + "inputs" : [ + "https://example.com/", + "https://ooni.org/", + ], + "inputs_extra" : [ + {"category_code" : "HUMR"}, + {} + ], + "options": {}, + "backend_options": {}, + "is_background_run_enabled_default": False, + "is_manual_run_enabled_default": False, + "test_name": "dnscheck", + }, ], } From 018cda19bed26f35229a08dd0fde9e49b9c8bd83 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 22 May 2025 10:26:30 +0200 Subject: [PATCH 06/72] Test insert of new nettests --- ooniapi/services/oonirun/tests/test_database.py | 12 ++++++++++++ 1 file changed, 12 insertions(+) diff --git a/ooniapi/services/oonirun/tests/test_database.py b/ooniapi/services/oonirun/tests/test_database.py index 66cfdf316..63274bcda 100644 --- a/ooniapi/services/oonirun/tests/test_database.py +++ b/ooniapi/services/oonirun/tests/test_database.py @@ -150,6 +150,18 @@ def test_upgrade_to_head(postgresql): nettest_index=0, date_created=utcnow_seconds(), ), + models.OONIRunLinkNettest( + **nettests[2], + revision=4, + nettest_index=1, + date_created=utcnow_seconds(), + ), + models.OONIRunLinkNettest( + **nettests[3], + revision=5, + nettest_index=1, + date_created=utcnow_seconds(), + ), ] db.add(db_runlink) db.commit() From 4267822e470938977c5cdf223a0a1a3dc4540c91 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 22 May 2025 10:54:33 +0200 Subject: [PATCH 07/72] Add targets_name and inputs_extra to oonirun --- ooniapi/services/oonirun/src/oonirun/models.py | 2 ++ ooniapi/services/oonirun/tests/test_oonirun.py | 5 +++++ 2 files changed, 7 insertions(+) diff --git a/ooniapi/services/oonirun/src/oonirun/models.py b/ooniapi/services/oonirun/src/oonirun/models.py index efadc5fc0..d4d2df6dd 100644 --- a/ooniapi/services/oonirun/src/oonirun/models.py +++ b/ooniapi/services/oonirun/src/oonirun/models.py @@ -61,3 +61,5 @@ class OONIRunLinkNettest(Base): is_background_run_enabled_default: Mapped[bool] = mapped_column(default=False) is_manual_run_enabled_default: Mapped[bool] = mapped_column(default=False) + targets_name: Mapped[str] = mapped_column(nullable=True) + inputs_extra: Mapped[Dict[str, Any]] = mapped_column(nullable=True) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 737fb4ccb..c8f449c84 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -33,6 +33,8 @@ "https://example.com/", "https://ooni.org/", ], + "targets_name" : None, + "inputs_extra" : None, "options": { "HTTP3Enabled": True, }, @@ -43,6 +45,8 @@ }, { "inputs": [], + "targets_name" : None, + "inputs_extra" : None, "options": {}, "backend_options": {}, "is_background_run_enabled_default": False, @@ -215,6 +219,7 @@ def test_oonirun_full_workflow(client, client_with_user_role, client_with_admin_ assert j["name"] == z["name"] assert j["name_intl"] == z["name_intl"] assert j["description"] == z["description"] + from pprint import pprint assert j["nettests"] == z["nettests"] date_created = datetime.strptime( j["date_created"], "%Y-%m-%dT%H:%M:%S.%fZ" From f0419391ebabb78c10a58562b20c52742b797bd5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 22 May 2025 11:51:13 +0200 Subject: [PATCH 08/72] Add tests to check consistency of inputs_extra --- .../services/oonirun/tests/test_oonirun.py | 40 +++++++++++++++++-- 1 file changed, 36 insertions(+), 4 deletions(-) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index c8f449c84..946e07e8e 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -33,8 +33,8 @@ "https://example.com/", "https://ooni.org/", ], - "targets_name" : None, - "inputs_extra" : None, + "targets_name": None, + "inputs_extra": None, "options": { "HTTP3Enabled": True, }, @@ -45,8 +45,8 @@ }, { "inputs": [], - "targets_name" : None, - "inputs_extra" : None, + "targets_name": None, + "inputs_extra": None, "options": {}, "backend_options": {}, "is_background_run_enabled_default": False, @@ -220,6 +220,7 @@ def test_oonirun_full_workflow(client, client_with_user_role, client_with_admin_ assert j["name_intl"] == z["name_intl"] assert j["description"] == z["description"] from pprint import pprint + assert j["nettests"] == z["nettests"] date_created = datetime.strptime( j["date_created"], "%Y-%m-%dT%H:%M:%S.%fZ" @@ -593,3 +594,34 @@ def test_oonirun_revisions(client, client_with_user_role): r = client.get(f"/api/v2/oonirun/links/404/engine-descriptor/latest") j = r.json() assert r.status_code == 404, r.json() + + +def test_inputs_extra_length(client, client_with_user_role): + z = deepcopy(SAMPLE_OONIRUN) + z["name"] = "integ-test name in English" + nettests = z.pop("nettests") + nettests = nettests[:1] + nettests[0]["inputs_extra"] = [ + { + "provider": "riseupvpn", + } + ] + z['nettests'] = nettests + + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 422, "Should fail when inputs_extra != None and len(inputs_extra) != len(inputs)" + + nettests[0]["inputs_extra"] = [ + { + "provider": "riseupvpn", + }, + { + "provider": "riseupvpn", + } + ] + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, "Appropiate inputs extra size, should pass" + + nettests[0].pop("inputs_extra") + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, "No checks should be performed when inputs_extra is None" From b0c979292712be64307cab70897d899fb77ba75d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 22 May 2025 12:00:01 +0200 Subject: [PATCH 09/72] Add inputs_extra validation --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 10 ++++++++-- 1 file changed, 8 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 2ff75af07..4db92affc 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -12,8 +12,8 @@ from sqlalchemy.orm import Session from fastapi import APIRouter, Depends, Query, HTTPException, Header, Path from pydantic import computed_field, Field -from pydantic.functional_validators import field_validator -from typing_extensions import Annotated +from pydantic.functional_validators import field_validator, model_validator +from typing_extensions import Annotated, Self from .. import models @@ -61,6 +61,12 @@ class OONIRunLinkNettest(BaseModel): description = "provides a richer JSON array containing extra parameters for each input. If provided, the length of inputs_extra should match the length of inputs." ) + @model_validator(mode="after") + def validate_inputs_extra(self) -> Self: + if self.inputs_extra is not None and len(self.inputs) != len(self.inputs_extra): + raise ValueError("When provided, inputs_extra should be the same length as inputs") + return self + class OONIRunLinkEngineDescriptor(BaseModel): revision: str = Field(title="revision of the nettest descriptor") nettests: List[OONIRunLinkNettest] = Field(default=[], title="list of nettests") From 0ab1caed97853868832a154a563f7845dbb3b5ba Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 22 May 2025 12:13:59 +0200 Subject: [PATCH 10/72] Fixed broken test --- ooniapi/services/oonirun/tests/test_database.py | 10 +++++----- 1 file changed, 5 insertions(+), 5 deletions(-) diff --git a/ooniapi/services/oonirun/tests/test_database.py b/ooniapi/services/oonirun/tests/test_database.py index 63274bcda..d67964804 100644 --- a/ooniapi/services/oonirun/tests/test_database.py +++ b/ooniapi/services/oonirun/tests/test_database.py @@ -152,14 +152,14 @@ def test_upgrade_to_head(postgresql): ), models.OONIRunLinkNettest( **nettests[2], - revision=4, - nettest_index=1, + revision=1, + nettest_index=2, date_created=utcnow_seconds(), ), models.OONIRunLinkNettest( **nettests[3], - revision=5, - nettest_index=1, + revision=1, + nettest_index=3, date_created=utcnow_seconds(), ), ] @@ -168,7 +168,7 @@ def test_upgrade_to_head(postgresql): new_row = db.query(models.OONIRunLink).first() assert new_row - assert new_row.nettests[0].revision == 3 + assert new_row.nettests[0].revision == 3, "First one to show up should have the latest revision" db.close() From 69f33b43810468c309d289a62127076fac4363b1 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 22 May 2025 12:14:48 +0200 Subject: [PATCH 11/72] Add TODO comment --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 6 ++++++ 1 file changed, 6 insertions(+) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 4db92affc..2bb57c25a 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -42,6 +42,7 @@ class OONIRunLinkNettest(BaseModel): default=[], title="list of input dictionaries for the nettest" ) options: Dict = Field(default={}, title="options for the nettest") + # TODO(luis): Not in the new spec. Should be removed? backend_options: Dict = Field(default={}, title="options to send to the backend") is_background_run_enabled_default: bool = Field( default=False, @@ -131,6 +132,11 @@ def validate_intl(cls, v: Dict[str, str]): ) class OONIRunLink(OONIRunLinkBase): + + # TODO(luis): spec mentions that there should be a title, description and author here. + # see: https://github.com/ooni/spec/blob/oonirun-v2.1/backends/bk-005-ooni-run-v2.md#response-body + # should we add it? + oonirun_link_id: str date_created: datetime = Field( description="time when the ooni run link was created" From e39a1252dfc8847fe1f608a7dbec179078a9ab58 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 22 May 2025 12:21:38 +0200 Subject: [PATCH 12/72] Removed useless comment --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 3 --- 1 file changed, 3 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 2bb57c25a..ee09639e5 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -133,9 +133,6 @@ def validate_intl(cls, v: Dict[str, str]): class OONIRunLink(OONIRunLinkBase): - # TODO(luis): spec mentions that there should be a title, description and author here. - # see: https://github.com/ooni/spec/blob/oonirun-v2.1/backends/bk-005-ooni-run-v2.md#response-body - # should we add it? oonirun_link_id: str date_created: datetime = Field( From d6fce798033ab4ba1bf8b2809c6884844e2b878f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 23 May 2025 11:52:32 +0200 Subject: [PATCH 13/72] Add filter by revision and test --- .../oonirun/src/oonirun/routers/v2.py | 11 +++++- .../services/oonirun/tests/test_oonirun.py | 37 +++++++++++++++++++ 2 files changed, 46 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index ee09639e5..f6396875c 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -41,8 +41,8 @@ class OONIRunLinkNettest(BaseModel): inputs: List[str] = Field( default=[], title="list of input dictionaries for the nettest" ) + # TODO(luis): Options and backend_options not in the new spec. Should be removed? options: Dict = Field(default={}, title="options for the nettest") - # TODO(luis): Not in the new spec. Should be removed? backend_options: Dict = Field(default={}, title="options to send to the backend") is_background_run_enabled_default: bool = Field( default=False, @@ -577,6 +577,10 @@ def list_oonirun_links( Optional[bool], Query(description="List also expired descriptors"), ] = None, + only_latest: Annotated[ + Optional[bool], + Query(description = "List only descriptors in the latest revision") + ] = True, authorization: str = Header("authorization"), settings=Depends(get_settings), ) -> OONIRunLinkList: @@ -596,7 +600,10 @@ def list_oonirun_links( assert ( row.nettests[-1].revision <= revision ), "nettests must be sorted by revision" - nettests, _ = get_nettests(row, revision) + + # if revision is None, it will get all the nettests, including from old revisions + nettests, _ = get_nettests(row, revision if only_latest else None) + oonirun_link = OONIRunLink( oonirun_link_id=row.oonirun_link_id, name=row.name, diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 946e07e8e..58be2a9b0 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -625,3 +625,40 @@ def test_inputs_extra_length(client, client_with_user_role): nettests[0].pop("inputs_extra") r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 200, "No checks should be performed when inputs_extra is None" + +def test_is_latest_list(client, client_with_user_role): + """ + Test that the only_latest argument in /links filters properly + """ + from pprint import pprint + from sys import stderr + + # Create link + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing list filtering" + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, r.json() + j = r.json() + + # Now update it + id = j['oonirun_link_id'] + z['nettests'][0]['inputs'].append("https://ooni.io/") + r = client_with_user_role.put(f"/api/v2/oonirun/links/{id}", json=z) + assert r.status_code == 200, r.json() + j = r.json() + + + # Check filtering + # Only last revision by default + r = client.get("/api/v2/oonirun/links") + assert r.status_code == 200 + j = r.json() + nts = j['oonirun_links'][0]['nettests'] + assert len(nts) == 2, "There are only 2 nettests in the last revision" + + # All revisions + r = client.get("/api/v2/oonirun/links", params = {"only_latest" : False}) + assert r.status_code == 200 + j = r.json() + nts = j['oonirun_links'][0]['nettests'] + assert len(nts) == 4, "There are 4 nettests between all revisions" From cd9fab9a704aeca4efacd356de39b07fb7081cd3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 23 May 2025 11:53:01 +0200 Subject: [PATCH 14/72] Removed unused imports --- ooniapi/services/oonirun/tests/test_oonirun.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 58be2a9b0..4d2add13c 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -630,8 +630,6 @@ def test_is_latest_list(client, client_with_user_role): """ Test that the only_latest argument in /links filters properly """ - from pprint import pprint - from sys import stderr # Create link z = deepcopy(SAMPLE_OONIRUN) From a2613c59047ee3d9f356dc48f1096e9813abab3c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 23 May 2025 13:07:18 +0200 Subject: [PATCH 15/72] Added missing arguments to engine-descriptor and tests --- .../oonirun/src/oonirun/routers/v2.py | 9 ++++++++ .../services/oonirun/tests/test_oonirun.py | 23 +++++++++++++++++++ 2 files changed, 32 insertions(+) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index f6396875c..d4d751430 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -469,8 +469,17 @@ def get_oonirun_link_engine_descriptor( ), ], db : PostgresSession, + run_type : Annotated[ + Optional[str], + Query(description="Run type", pattern="^(timed|manual)$") + ] = None, + is_charging : Annotated [ + bool, + Query(description="If the probe is charging"), + ] = False, ): """Fetch an OONI Run link by specifying the revision number""" + #TODO Use is_charging and run_type try: revision = int(revision_number) except: diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 4d2add13c..ae84c2da8 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -660,3 +660,26 @@ def test_is_latest_list(client, client_with_user_role): j = r.json() nts = j['oonirun_links'][0]['nettests'] assert len(nts) == 4, "There are 4 nettests between all revisions" + +def test_link_revision_args(client, client_with_user_role): + # Check args parsing for oonirun engine-descriptor + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing descriptor revision" + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code, r.json() + j = r.json() + id = j['oonirun_link_id'] + + # Check that arguments defaults work properly + r = client.get(f"/api/v2/oonirun/links/{id}/engine-descriptor/1") + assert r.status_code == 200, r.json() + + # Try with good arguments + gs = ['timed', 'manual'] + for good in gs: + r = client.get(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", params={"run_type" : good}) + assert r.status_code == 200, r.json() + + # Try with bad arguments + r = client.get(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", params={"run_type" : "bad"}) + assert r.status_code == 422, r.json() From e784589c32ce73bb634f1f18a5d22a244835f392 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 23 May 2025 13:20:17 +0200 Subject: [PATCH 16/72] Add headers for dynamic test lists calculation --- .../services/oonirun/src/oonirun/routers/v2.py | 16 ++++++++++++++++ 1 file changed, 16 insertions(+) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index d4d751430..5b7adfddc 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -477,6 +477,22 @@ def get_oonirun_link_engine_descriptor( bool, Query(description="If the probe is charging"), ] = False, + x_ooni_networkinfo : Annotated[ + Optional[str], #TODO Marked as optional to avoid breaking old proves + Header(description="Expected format: , (), eg AS1234,IT (wifi)") + ] = None, + x_ooni_websitecategorycodes : Annotated[ + Optional[str], #TODO Marked as optional to avoid breaking old proves + Header(description="Comma separated list of category codes that user has chosen to test (eg. NEWS,HUMR)") + ] = None, + x_ooni_credentials : Annotated[ + Optional[str], #TODO Marked as optional to avoid breaking old proves + Header(description="base64 encoded OONI anonymous credentials") + ] = None, + user_agent: Annotated[ + Optional[str], #TODO Marked as optional to avoid breaking old proves + Header(description="Expected format: / () / ()") + ] = None ): """Fetch an OONI Run link by specifying the revision number""" #TODO Use is_charging and run_type From 1e79ef463777dfbf6eb1977a0f3bba9708b0268f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 23 May 2025 14:00:27 +0200 Subject: [PATCH 17/72] Add arguments for dynamic targets list generation --- .../oonirun/src/oonirun/routers/v2.py | 76 ++++++++++--------- 1 file changed, 42 insertions(+), 34 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 5b7adfddc..617d54a20 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -52,22 +52,25 @@ class OONIRunLinkNettest(BaseModel): default=False, title="if this test should be enabled by default for manual runs" ) - targets_name : Optional[str] = Field ( - default=None, - description="string used to specify during creation that the input list should be dynamically generated." + targets_name: Optional[str] = Field( + default=None, + description="string used to specify during creation that the input list should be dynamically generated.", ) - inputs_extra : Optional[List[Dict[str, Any]]] = Field ( - default = None, - description = "provides a richer JSON array containing extra parameters for each input. If provided, the length of inputs_extra should match the length of inputs." + inputs_extra: Optional[List[Dict[str, Any]]] = Field( + default=None, + description="provides a richer JSON array containing extra parameters for each input. If provided, the length of inputs_extra should match the length of inputs.", ) @model_validator(mode="after") def validate_inputs_extra(self) -> Self: if self.inputs_extra is not None and len(self.inputs) != len(self.inputs_extra): - raise ValueError("When provided, inputs_extra should be the same length as inputs") + raise ValueError( + "When provided, inputs_extra should be the same length as inputs" + ) return self + class OONIRunLinkEngineDescriptor(BaseModel): revision: str = Field(title="revision of the nettest descriptor") nettests: List[OONIRunLinkNettest] = Field(default=[], title="list of nettests") @@ -131,8 +134,8 @@ def validate_intl(cls, v: Dict[str, str]): description="future time after which the ooni run link will be considered expired and no longer editable or usable (defaults to 6 months from now)", ) -class OONIRunLink(OONIRunLinkBase): +class OONIRunLink(OONIRunLinkBase): oonirun_link_id: str date_created: datetime = Field( @@ -168,7 +171,7 @@ class OONIRunLinkCreateEdit(OONIRunLinkBase): ) def create_oonirun_link( create_request: OONIRunLinkCreateEdit, - db : PostgresSession, + db: PostgresSession, token=Depends(role_required(["admin", "user"])), ) -> OONIRunLink: """Create a new oonirun link or a new version for an existing one.""" @@ -250,7 +253,7 @@ def create_oonirun_link( def edit_oonirun_link( oonirun_link_id: str, edit_request: OONIRunLinkCreateEdit, - db : PostgresSession, + db: PostgresSession, token=Depends(role_required(["admin", "user"])), ): """Edit an existing OONI Run link""" @@ -428,7 +431,7 @@ class OONIRunLinkRevisions(BaseModel): ) def get_oonirun_link_revisions( oonirun_link_id: str, - db : PostgresSession, + db: PostgresSession, ): """ Obtain the list of revisions for a certain OONI Run link @@ -468,34 +471,39 @@ def get_oonirun_link_engine_descriptor( }, ), ], - db : PostgresSession, - run_type : Annotated[ - Optional[str], - Query(description="Run type", pattern="^(timed|manual)$") + db: PostgresSession, + run_type: Annotated[ + Optional[str], Query(description="Run type", pattern="^(timed|manual)$") ] = None, - is_charging : Annotated [ + is_charging: Annotated[ bool, Query(description="If the probe is charging"), ] = False, - x_ooni_networkinfo : Annotated[ - Optional[str], #TODO Marked as optional to avoid breaking old proves - Header(description="Expected format: , (), eg AS1234,IT (wifi)") + x_ooni_networkinfo: Annotated[ + Optional[str], # TODO Marked as optional to avoid breaking old proves + Header( + description="Expected format: , (), eg AS1234,IT (wifi)" + ), ] = None, - x_ooni_websitecategorycodes : Annotated[ - Optional[str], #TODO Marked as optional to avoid breaking old proves - Header(description="Comma separated list of category codes that user has chosen to test (eg. NEWS,HUMR)") + x_ooni_websitecategorycodes: Annotated[ + Optional[str], # TODO Marked as optional to avoid breaking old proves + Header( + description="Comma separated list of category codes that user has chosen to test (eg. NEWS,HUMR)" + ), ] = None, - x_ooni_credentials : Annotated[ - Optional[str], #TODO Marked as optional to avoid breaking old proves - Header(description="base64 encoded OONI anonymous credentials") + x_ooni_credentials: Annotated[ + Optional[str], # TODO Marked as optional to avoid breaking old proves + Header(description="base64 encoded OONI anonymous credentials"), ] = None, user_agent: Annotated[ - Optional[str], #TODO Marked as optional to avoid breaking old proves - Header(description="Expected format: / () / ()") - ] = None + Optional[str], # TODO Marked as optional to avoid breaking old proves + Header( + description="Expected format: / () / ()" + ), + ] = None, ): """Fetch an OONI Run link by specifying the revision number""" - #TODO Use is_charging and run_type + # TODO Use is_charging and run_type try: revision = int(revision_number) except: @@ -541,7 +549,7 @@ def get_oonirun_link_revision( }, ), ], - db : PostgresSession, + db: PostgresSession, authorization: str = Header("authorization"), settings=Depends(get_settings), ): @@ -570,7 +578,7 @@ def get_oonirun_link_revision( ) def get_latest_oonirun_link( oonirun_link_id: str, - db : PostgresSession, + db: PostgresSession, authorization: str = Header("authorization"), settings=Depends(get_settings), ): @@ -593,7 +601,7 @@ class OONIRunLinkList(BaseModel): @router.get("/v2/oonirun/links", tags=["oonirun"]) def list_oonirun_links( - db : PostgresSession, + db: PostgresSession, is_mine: Annotated[ Optional[bool], Query(description="List only the my descriptors"), @@ -603,8 +611,8 @@ def list_oonirun_links( Query(description="List also expired descriptors"), ] = None, only_latest: Annotated[ - Optional[bool], - Query(description = "List only descriptors in the latest revision") + Optional[bool], + Query(description="List only descriptors in the latest revision"), ] = True, authorization: str = Header("authorization"), settings=Depends(get_settings), From cd4e6c35f30a915ff3f897b3245d6b80a032a86d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Mon, 26 May 2025 15:50:52 +0200 Subject: [PATCH 18/72] Rename postgres session dependency --- .../services/oonirun/src/oonirun/dependencies.py | 2 +- .../services/oonirun/src/oonirun/routers/v2.py | 16 ++++++++-------- 2 files changed, 9 insertions(+), 9 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/dependencies.py b/ooniapi/services/oonirun/src/oonirun/dependencies.py index 6a93b5e30..c6dee68d8 100644 --- a/ooniapi/services/oonirun/src/oonirun/dependencies.py +++ b/ooniapi/services/oonirun/src/oonirun/dependencies.py @@ -20,4 +20,4 @@ def get_postgresql_session(settings: Annotated[Settings, Depends(get_settings)]) finally: db.close() -PostgresSession = Annotated[Session, Depends(get_postgresql_session)] \ No newline at end of file +DependsPostgresSession = Annotated[Session, Depends(get_postgresql_session)] \ No newline at end of file diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 617d54a20..cc8989e05 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -22,7 +22,7 @@ from ..common.auth import ( get_account_id_or_none, ) -from ..dependencies import PostgresSession, get_postgresql_session +from ..dependencies import DependsPostgresSession, get_postgresql_session log = logging.getLogger(__name__) @@ -171,7 +171,7 @@ class OONIRunLinkCreateEdit(OONIRunLinkBase): ) def create_oonirun_link( create_request: OONIRunLinkCreateEdit, - db: PostgresSession, + db: DependsPostgresSession, token=Depends(role_required(["admin", "user"])), ) -> OONIRunLink: """Create a new oonirun link or a new version for an existing one.""" @@ -253,7 +253,7 @@ def create_oonirun_link( def edit_oonirun_link( oonirun_link_id: str, edit_request: OONIRunLinkCreateEdit, - db: PostgresSession, + db: DependsPostgresSession, token=Depends(role_required(["admin", "user"])), ): """Edit an existing OONI Run link""" @@ -431,7 +431,7 @@ class OONIRunLinkRevisions(BaseModel): ) def get_oonirun_link_revisions( oonirun_link_id: str, - db: PostgresSession, + db: DependsPostgresSession, ): """ Obtain the list of revisions for a certain OONI Run link @@ -471,7 +471,7 @@ def get_oonirun_link_engine_descriptor( }, ), ], - db: PostgresSession, + db: DependsPostgresSession, run_type: Annotated[ Optional[str], Query(description="Run type", pattern="^(timed|manual)$") ] = None, @@ -549,7 +549,7 @@ def get_oonirun_link_revision( }, ), ], - db: PostgresSession, + db: DependsPostgresSession, authorization: str = Header("authorization"), settings=Depends(get_settings), ): @@ -578,7 +578,7 @@ def get_oonirun_link_revision( ) def get_latest_oonirun_link( oonirun_link_id: str, - db: PostgresSession, + db: DependsPostgresSession, authorization: str = Header("authorization"), settings=Depends(get_settings), ): @@ -601,7 +601,7 @@ class OONIRunLinkList(BaseModel): @router.get("/v2/oonirun/links", tags=["oonirun"]) def list_oonirun_links( - db: PostgresSession, + db: DependsPostgresSession, is_mine: Annotated[ Optional[bool], Query(description="List only the my descriptors"), From 1868336702ae8d882863497fa65652ae09b98613 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Mon, 26 May 2025 15:53:42 +0200 Subject: [PATCH 19/72] Fix typo --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 8 ++++---- 1 file changed, 4 insertions(+), 4 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index cc8989e05..92d970c96 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -480,23 +480,23 @@ def get_oonirun_link_engine_descriptor( Query(description="If the probe is charging"), ] = False, x_ooni_networkinfo: Annotated[ - Optional[str], # TODO Marked as optional to avoid breaking old proves + Optional[str], # TODO Marked as optional to avoid breaking old probes Header( description="Expected format: , (), eg AS1234,IT (wifi)" ), ] = None, x_ooni_websitecategorycodes: Annotated[ - Optional[str], # TODO Marked as optional to avoid breaking old proves + Optional[str], # TODO Marked as optional to avoid breaking old probes Header( description="Comma separated list of category codes that user has chosen to test (eg. NEWS,HUMR)" ), ] = None, x_ooni_credentials: Annotated[ - Optional[str], # TODO Marked as optional to avoid breaking old proves + Optional[str], # TODO Marked as optional to avoid breaking old probes Header(description="base64 encoded OONI anonymous credentials"), ] = None, user_agent: Annotated[ - Optional[str], # TODO Marked as optional to avoid breaking old proves + Optional[str], # TODO Marked as optional to avoid breaking old probes Header( description="Expected format: / () / ()" ), From 9f929812b7f10daf26ceb1f3221908c9a3a7f263 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 27 May 2025 11:11:53 +0200 Subject: [PATCH 20/72] Prevent both of targets_name and inputs to be present at the same time --- .../oonirun/src/oonirun/routers/v2.py | 14 +++- .../services/oonirun/tests/test_oonirun.py | 71 ++++++++++++++++++- 2 files changed, 83 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 92d970c96..854652c81 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -38,7 +38,7 @@ class OONIRunLinkNettest(BaseModel): test_name: str = Field( default="", title="name of the ooni nettest", min_length=2, max_length=100 ) - inputs: List[str] = Field( + inputs: Optional[List[str]] = Field( default=[], title="list of input dictionaries for the nettest" ) # TODO(luis): Options and backend_options not in the new spec. Should be removed? @@ -70,6 +70,18 @@ def validate_inputs_extra(self) -> Self: ) return self + @model_validator(mode="after") + def validate_no_inputs_and_targets_name(self): + """ + Check that you are not providing targets_name and inputs-inputs_extra in the same request + """ + if self.targets_name is not None and (self.inputs is not None or self.inputs_extra is not None): + raise ValueError( + "When targets_name is provided, you can't provide inputs or inputs_extra" + ) + + return self + class OONIRunLinkEngineDescriptor(BaseModel): revision: str = Field(title="revision of the nettest descriptor") diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index ae84c2da8..4ecd941a7 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -666,7 +666,7 @@ def test_link_revision_args(client, client_with_user_role): z = deepcopy(SAMPLE_OONIRUN) z['name'] = "Testing descriptor revision" r = client_with_user_role.post("/api/v2/oonirun/links", json=z) - assert r.status_code, r.json() + assert r.status_code == 200, r.json() j = r.json() id = j['oonirun_link_id'] @@ -683,3 +683,72 @@ def test_link_revision_args(client, client_with_user_role): # Try with bad arguments r = client.get(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", params={"run_type" : "bad"}) assert r.status_code == 422, r.json() + +def test_inputs_and_targets_name(client_with_user_role): + """ + Test that you can't specify targets_name and inputs in the same request + """ + + # Both + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing no targets and inputs at the same time" + + # Only inputs = OK + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, r.json() + + # Only targets = OK + z['nettests'] = [ + { + "inputs": None, + "targets_name": "example_name", + "inputs_extra": None, + "options": { + "HTTP3Enabled": True, + }, + "backend_options": {}, + "is_background_run_enabled_default": False, + "is_manual_run_enabled_default": False, + "test_name": "web_connectivity", + }, + ] + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, r.json() + + # Both targets and input = error + z['nettests'] = [ + { + "inputs": [ + "https://example.com/", + "https://ooni.org/", + ], + "targets_name": "example_name", + "inputs_extra": None, + "options": { + "HTTP3Enabled": True, + }, + "backend_options": {}, + "is_background_run_enabled_default": False, + "is_manual_run_enabled_default": False, + "test_name": "web_connectivity", + }, + ] + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 422, r.json() + + # Both targets and inputs_extra = error + z['nettests'] = [ + { + "targets_name": "example_name", + "inputs_extra": [{}, {}], + "options": { + "HTTP3Enabled": True, + }, + "backend_options": {}, + "is_background_run_enabled_default": False, + "is_manual_run_enabled_default": False, + "test_name": "web_connectivity", + }, + ] + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 422, r.json() \ No newline at end of file From bf1a695763fb209e110dcbe5dafc43ad290fecd9 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 27 May 2025 11:12:58 +0200 Subject: [PATCH 21/72] Remove unused import --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 854652c81..5c6884be0 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -22,7 +22,7 @@ from ..common.auth import ( get_account_id_or_none, ) -from ..dependencies import DependsPostgresSession, get_postgresql_session +from ..dependencies import DependsPostgresSession log = logging.getLogger(__name__) From 800753d0b5fdb4f12d37330aa99b2d1350fb1ba3 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 27 May 2025 11:13:52 +0200 Subject: [PATCH 22/72] Drop backend options parameter --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 1 - 1 file changed, 1 deletion(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 5c6884be0..6d38a4497 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -43,7 +43,6 @@ class OONIRunLinkNettest(BaseModel): ) # TODO(luis): Options and backend_options not in the new spec. Should be removed? options: Dict = Field(default={}, title="options for the nettest") - backend_options: Dict = Field(default={}, title="options to send to the backend") is_background_run_enabled_default: bool = Field( default=False, title="if this test should be enabled by default for background runs", From 631c61a3425f2a309a3c0f761b7160e0e26448a5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 27 May 2025 11:16:29 +0200 Subject: [PATCH 23/72] Drop backend options parameter --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 4 ---- ooniapi/services/oonirun/tests/test_oonirun.py | 5 ----- 2 files changed, 9 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 6d38a4497..b62a2850f 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -220,7 +220,6 @@ def create_oonirun_link( test_name=nt.test_name, inputs=nt.inputs, options=nt.options, - backend_options=nt.backend_options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, ) @@ -309,7 +308,6 @@ def edit_oonirun_link( test_name=nt.test_name, inputs=nt.inputs, options=nt.options, - backend_options=nt.backend_options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, ) @@ -325,7 +323,6 @@ def edit_oonirun_link( test_name=nt.test_name, inputs=nt.inputs, options=nt.options, - backend_options=nt.backend_options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, oonirun_link=oonirun_link, @@ -380,7 +377,6 @@ def get_nettests( test_name=nt.test_name, inputs=nt.inputs, options=nt.options, - backend_options=nt.backend_options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, ) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 4ecd941a7..4d10a98b8 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -38,7 +38,6 @@ "options": { "HTTP3Enabled": True, }, - "backend_options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -48,7 +47,6 @@ "targets_name": None, "inputs_extra": None, "options": {}, - "backend_options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", @@ -706,7 +704,6 @@ def test_inputs_and_targets_name(client_with_user_role): "options": { "HTTP3Enabled": True, }, - "backend_options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -727,7 +724,6 @@ def test_inputs_and_targets_name(client_with_user_role): "options": { "HTTP3Enabled": True, }, - "backend_options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -744,7 +740,6 @@ def test_inputs_and_targets_name(client_with_user_role): "options": { "HTTP3Enabled": True, }, - "backend_options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", From 60f8775797d070c2eae597b191108ffe8f7ca54f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 27 May 2025 12:58:53 +0200 Subject: [PATCH 24/72] Working on headers parsing --- .../oonirun/src/oonirun/routers/v2.py | 36 +++++++++++++++---- .../services/oonirun/tests/test_oonirun.py | 17 ++++++++- 2 files changed, 46 insertions(+), 7 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index b62a2850f..723d925ac 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -7,6 +7,7 @@ from datetime import datetime, timedelta, timezone from typing import Dict, List, Optional, Tuple, Any import logging +import re import sqlalchemy as sa from sqlalchemy.orm import Session @@ -461,6 +462,30 @@ def get_oonirun_link_revisions( revisions.append(str(r)) return OONIRunLinkRevisions(revisions=revisions) +class XOoniNetworkInfo(BaseModel): + probe_asn : str + probe_cc : str + network_type : str + + @staticmethod + def get_header_pattern() -> str: + return r'^([a-zA-Z0-9]+),([a-zA-Z0-9]+) \(([a-zA-Z0-9]+)\)$' + + @classmethod + def from_header(cls, header : str) -> Self: + pattern = cls.get_header_pattern() + matched = re.match(pattern, header) + + assert matched is not None, "Expected format: , (), eg AS1234,IT (wifi)" + + probe_asn, probe_cc, network_type = matched.groups() + return cls(probe_asn=probe_asn, probe_cc=probe_cc, network_type=network_type) + +HeaderXOoniNetworkInfo = Annotated[Optional[str], Header( + description="Expected format: , (), eg AS1234,IT (wifi)", + pattern=XOoniNetworkInfo.get_header_pattern() + ) +] @router.get( "/v2/oonirun/links/{oonirun_link_id}/engine-descriptor/{revision_number}", @@ -486,12 +511,7 @@ def get_oonirun_link_engine_descriptor( bool, Query(description="If the probe is charging"), ] = False, - x_ooni_networkinfo: Annotated[ - Optional[str], # TODO Marked as optional to avoid breaking old probes - Header( - description="Expected format: , (), eg AS1234,IT (wifi)" - ), - ] = None, + x_ooni_networkinfo: HeaderXOoniNetworkInfo = None, x_ooni_websitecategorycodes: Annotated[ Optional[str], # TODO Marked as optional to avoid breaking old probes Header( @@ -518,6 +538,10 @@ def get_oonirun_link_engine_descriptor( assert revision_number == "latest" revision = None + network_info = None + if x_ooni_networkinfo is not None: + network_info = XOoniNetworkInfo.from_header(x_ooni_networkinfo) + q = db.query(models.OONIRunLink).filter( models.OONIRunLink.oonirun_link_id == oonirun_link_id ) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 4d10a98b8..fa4c5eca2 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -746,4 +746,19 @@ def test_inputs_and_targets_name(client_with_user_role): }, ] r = client_with_user_role.post("/api/v2/oonirun/links", json=z) - assert r.status_code == 422, r.json() \ No newline at end of file + assert r.status_code == 422, r.json() + +def test_x_ooni_networkinfo_header_parsing(client_with_user_role, client): + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing header parsing" + + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, r.json() + j = r.json() + + + headers = { + "X-Ooni-NetworkInfo" : "AS1234,VE (wifi)" + } + r = client.get(f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/{j['revision']}", headers=headers) + assert r.status_code == 200, r.content \ No newline at end of file From 2d701fa25bbc3e72eb7f2aa0941571f8ea114983 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 28 May 2025 13:00:10 +0200 Subject: [PATCH 25/72] Add tests for header parsing; moving arguments out of headers to post --- .../oonirun/src/oonirun/routers/v2.py | 43 ++++++------------- .../services/oonirun/tests/test_oonirun.py | 39 +++++++++++------ 2 files changed, 41 insertions(+), 41 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 723d925ac..0345d79cd 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -481,13 +481,15 @@ def from_header(cls, header : str) -> Self: probe_asn, probe_cc, network_type = matched.groups() return cls(probe_asn=probe_asn, probe_cc=probe_cc, network_type=network_type) -HeaderXOoniNetworkInfo = Annotated[Optional[str], Header( - description="Expected format: , (), eg AS1234,IT (wifi)", - pattern=XOoniNetworkInfo.get_header_pattern() - ) -] +class GetOoniRunLinkEngineDescriptorRequest(BaseModel): + run_type : Optional[str] = Field(description="Run type", pattern="^(timed|manual)$", default="manual") + is_charging : Optional[bool] = Field(description="If the probe is charging", default=False) + probe_asn : Optional[str] = Field(pattern=r"^([a-zA-Z0-9]+)$", default="AS0") + probe_cc : Optional[str] = Field(description="Country code. Ex: VE", default="ZZ") + network_type : Optional[str] = Field(description="Ex: wifi", default=None) + website_category_codes : Optional[List[str]] = Field(description="List of category codes that user has chosen to test (eg. NEWS,HUMR)", default=None) -@router.get( +@router.post( "/v2/oonirun/links/{oonirun_link_id}/engine-descriptor/{revision_number}", tags=["oonirun"], response_model=OONIRunLinkEngineDescriptor, @@ -504,31 +506,18 @@ def get_oonirun_link_engine_descriptor( ), ], db: DependsPostgresSession, - run_type: Annotated[ - Optional[str], Query(description="Run type", pattern="^(timed|manual)$") - ] = None, - is_charging: Annotated[ - bool, - Query(description="If the probe is charging"), - ] = False, - x_ooni_networkinfo: HeaderXOoniNetworkInfo = None, - x_ooni_websitecategorycodes: Annotated[ - Optional[str], # TODO Marked as optional to avoid breaking old probes - Header( - description="Comma separated list of category codes that user has chosen to test (eg. NEWS,HUMR)" - ), - ] = None, - x_ooni_credentials: Annotated[ - Optional[str], # TODO Marked as optional to avoid breaking old probes - Header(description="base64 encoded OONI anonymous credentials"), - ] = None, + request : GetOoniRunLinkEngineDescriptorRequest, user_agent: Annotated[ Optional[str], # TODO Marked as optional to avoid breaking old probes Header( + # TODO(luis) This is crashing when the header is not provided (optional). But maybe this header shouldn't be optional at all? + # pattern=r"^([a-zA-Z0-9\-\_]+)/([a-zA-Z0-9\-\_\.]+) \(([a-zA-Z0-9\ ]+)\) ([a-zA-Z0-9\-\_]+)/([a-zA-Z0-9\-\_\.]+)$", + error_message = "Expected format: / () / ()", description="Expected format: / () / ()" ), ] = None, -): + credentials : Annotated[Optional[bytes], Header(description="base64 encoded OONI anonymous credentials")] = None, + ): """Fetch an OONI Run link by specifying the revision number""" # TODO Use is_charging and run_type try: @@ -538,10 +527,6 @@ def get_oonirun_link_engine_descriptor( assert revision_number == "latest" revision = None - network_info = None - if x_ooni_networkinfo is not None: - network_info = XOoniNetworkInfo.from_header(x_ooni_networkinfo) - q = db.query(models.OONIRunLink).filter( models.OONIRunLink.oonirun_link_id == oonirun_link_id ) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index fa4c5eca2..608d2dbdb 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -217,7 +217,6 @@ def test_oonirun_full_workflow(client, client_with_user_role, client_with_admin_ assert j["name"] == z["name"] assert j["name_intl"] == z["name_intl"] assert j["description"] == z["description"] - from pprint import pprint assert j["nettests"] == z["nettests"] date_created = datetime.strptime( @@ -564,16 +563,18 @@ def test_oonirun_revisions(client, client_with_user_role): assert j["revisions"][0] == "3", "the latest one is 3" ## Fetch nettests for latest - r = client.get( - f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/latest" + r = client.post( + f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/latest", + json={} ) + assert r.status_code == 200, r.json() j_latest = r.json() assert j_latest["revision"] == "3", "revision is 3" assert j_latest["nettests"] == lastest_nettests, "nettests are the same" assert j_latest["date_created"] == latest_date_created, "date created matches" ## Should match latest - r = client.get(f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/3") + r = client.post(f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/3", json={}) assert j_latest == r.json() ## Fetch invalid revision number @@ -589,7 +590,7 @@ def test_oonirun_revisions(client, client_with_user_role): assert r.status_code == 404, r.json() ## Get not-existing engine descriptor - r = client.get(f"/api/v2/oonirun/links/404/engine-descriptor/latest") + r = client.post(f"/api/v2/oonirun/links/404/engine-descriptor/latest", json={}) j = r.json() assert r.status_code == 404, r.json() @@ -669,17 +670,17 @@ def test_link_revision_args(client, client_with_user_role): id = j['oonirun_link_id'] # Check that arguments defaults work properly - r = client.get(f"/api/v2/oonirun/links/{id}/engine-descriptor/1") + r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json={}) assert r.status_code == 200, r.json() # Try with good arguments gs = ['timed', 'manual'] for good in gs: - r = client.get(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", params={"run_type" : good}) + r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json={"run_type" : good}) assert r.status_code == 200, r.json() # Try with bad arguments - r = client.get(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", params={"run_type" : "bad"}) + r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json={"run_type" : "bad"}) assert r.status_code == 422, r.json() def test_inputs_and_targets_name(client_with_user_role): @@ -748,7 +749,7 @@ def test_inputs_and_targets_name(client_with_user_role): r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 422, r.json() -def test_x_ooni_networkinfo_header_parsing(client_with_user_role, client): +def test_x_user_agent_header_parsing(client_with_user_role, client): z = deepcopy(SAMPLE_OONIRUN) z['name'] = "Testing header parsing" @@ -756,9 +757,23 @@ def test_x_ooni_networkinfo_header_parsing(client_with_user_role, client): assert r.status_code == 200, r.json() j = r.json() + # Test with good headers + headers = { + "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2 ooniprobe-engine_1.2.3" + } + r = client.post( + f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/{j['revision']}", + headers=headers, + json={} + ) + assert r.status_code == 200, r.json() headers = { - "X-Ooni-NetworkInfo" : "AS1234,VE (wifi)" + "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2" } - r = client.get(f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/{j['revision']}", headers=headers) - assert r.status_code == 200, r.content \ No newline at end of file + r = client.post( + f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/{j['revision']}", + headers=headers, + json={} + ) + assert r.status_code == 422, r.json() \ No newline at end of file From 0f50f7af38c9de6b1adeaa37fc60d29e3b3dc919 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 28 May 2025 13:54:44 +0200 Subject: [PATCH 26/72] Skip user agent parsing tests for now --- .../oonirun/src/oonirun/routers/v2.py | 12 +-- .../services/oonirun/tests/test_oonirun.py | 81 ++++++++++--------- 2 files changed, 50 insertions(+), 43 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 0345d79cd..3c81a4a3f 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -482,12 +482,12 @@ def from_header(cls, header : str) -> Self: return cls(probe_asn=probe_asn, probe_cc=probe_cc, network_type=network_type) class GetOoniRunLinkEngineDescriptorRequest(BaseModel): - run_type : Optional[str] = Field(description="Run type", pattern="^(timed|manual)$", default="manual") - is_charging : Optional[bool] = Field(description="If the probe is charging", default=False) - probe_asn : Optional[str] = Field(pattern=r"^([a-zA-Z0-9]+)$", default="AS0") - probe_cc : Optional[str] = Field(description="Country code. Ex: VE", default="ZZ") - network_type : Optional[str] = Field(description="Ex: wifi", default=None) - website_category_codes : Optional[List[str]] = Field(description="List of category codes that user has chosen to test (eg. NEWS,HUMR)", default=None) + run_type : str = Field(description="Run type", pattern="^(timed|manual)$") + is_charging : bool = Field(description="If the probe is charging") + probe_asn : str = Field(pattern=r"^([a-zA-Z0-9]+)$") + probe_cc : str = Field(description="Country code. Ex: VE") + network_type : str = Field(description="Ex: wifi") + website_category_codes : List[str] = Field(description="List of category codes that user has chosen to test (eg. NEWS,HUMR)", default=[]) @router.post( "/v2/oonirun/links/{oonirun_link_id}/engine-descriptor/{revision_number}", diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 608d2dbdb..35799b089 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -74,6 +74,14 @@ "expiration_date", ] +SAMPLE_META = { + "run_type" : "timed", + "is_charging" : True, + "probe_asn" : "AS1234", + "probe_cc" : "VE", + "network_type" : "wifi", + "website_category_codes" : [] +} def test_get_version(client): r = client.get("/version") @@ -565,7 +573,7 @@ def test_oonirun_revisions(client, client_with_user_role): ## Fetch nettests for latest r = client.post( f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/latest", - json={} + json=SAMPLE_META ) assert r.status_code == 200, r.json() j_latest = r.json() @@ -574,7 +582,7 @@ def test_oonirun_revisions(client, client_with_user_role): assert j_latest["date_created"] == latest_date_created, "date created matches" ## Should match latest - r = client.post(f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/3", json={}) + r = client.post(f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/3", json=SAMPLE_META) assert j_latest == r.json() ## Fetch invalid revision number @@ -590,7 +598,7 @@ def test_oonirun_revisions(client, client_with_user_role): assert r.status_code == 404, r.json() ## Get not-existing engine descriptor - r = client.post(f"/api/v2/oonirun/links/404/engine-descriptor/latest", json={}) + r = client.post(f"/api/v2/oonirun/links/404/engine-descriptor/latest", json=SAMPLE_META) j = r.json() assert r.status_code == 404, r.json() @@ -669,18 +677,16 @@ def test_link_revision_args(client, client_with_user_role): j = r.json() id = j['oonirun_link_id'] - # Check that arguments defaults work properly - r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json={}) - assert r.status_code == 200, r.json() - # Try with good arguments gs = ['timed', 'manual'] for good in gs: - r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json={"run_type" : good}) + r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json=SAMPLE_META) assert r.status_code == 200, r.json() # Try with bad arguments - r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json={"run_type" : "bad"}) + bm = deepcopy(SAMPLE_META) + bm['run_type'] = "bad" + r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json=bm) assert r.status_code == 422, r.json() def test_inputs_and_targets_name(client_with_user_role): @@ -749,31 +755,32 @@ def test_inputs_and_targets_name(client_with_user_role): r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 422, r.json() -def test_x_user_agent_header_parsing(client_with_user_role, client): - z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing header parsing" - - r = client_with_user_role.post("/api/v2/oonirun/links", json=z) - assert r.status_code == 200, r.json() - j = r.json() - - # Test with good headers - headers = { - "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2 ooniprobe-engine_1.2.3" - } - r = client.post( - f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/{j['revision']}", - headers=headers, - json={} - ) - assert r.status_code == 200, r.json() - - headers = { - "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2" - } - r = client.post( - f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/{j['revision']}", - headers=headers, - json={} - ) - assert r.status_code == 422, r.json() \ No newline at end of file +# TODO(luis) finish this test for checking the parsing of user agent headers +# def test_x_user_agent_header_parsing(client_with_user_role, client): +# z = deepcopy(SAMPLE_OONIRUN) +# z['name'] = "Testing header parsing" + +# r = client_with_user_role.post("/api/v2/oonirun/links", json=z) +# assert r.status_code == 200, r.json() +# j = r.json() + +# # Test with good headers +# headers = { +# "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2 ooniprobe-engine_1.2.3" +# } +# r = client.post( +# f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", +# headers=headers, +# json={} +# ) +# assert r.status_code == 200, r.json() + +# headers = { +# "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2" +# } +# r = client.post( +# f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", +# headers=headers, +# json={} +# ) +# assert r.status_code == 422, r.json() \ No newline at end of file From 5645e77b3909c3bae77248d627e1ec0d4c118736 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 28 May 2025 15:17:10 +0200 Subject: [PATCH 27/72] Move prio to common --- ooniapi/common/src/common/prio.py | 158 ++++++++++++++++++++++++++++++ 1 file changed, 158 insertions(+) create mode 100644 ooniapi/common/src/common/prio.py diff --git a/ooniapi/common/src/common/prio.py b/ooniapi/common/src/common/prio.py new file mode 100644 index 000000000..689395b0e --- /dev/null +++ b/ooniapi/common/src/common/prio.py @@ -0,0 +1,158 @@ + +""" +OONI Probe Services API - reactive URL prioritization + +/api/v1/test-list/urls provides dynamic URL tests lists for web_connectivity +based on the citizenlab URL list and the measurements count from the last +7 days. + +The ooni-update-counters service updates the counters_test_list table at intervals + +The ooni-update-citizenlab service updates the citizenlab table at intervals + +``` +blockdiag { + Probes [color = "#ffeeee"]; + "API: test-list/urls" [color = "#eeeeff"]; + Probes -> "API: receive msmt" -> "Fastpath" -> "DB: fastpath table"; + "DB: fastpath table" -> "ooni-update-counters service" -> "DB: counters_test_list table"; + "DB: counters_test_list table" -> "API: test-list/urls" -> Probes; + "DB: citizenlab table" -> "API: test-list/urls"; +} +``` +""" + +from typing import List, Tuple +import logging + +from .clickhouse_utils import query_click +from .metrics import timer + +from clickhouse_driver import Client as Clickhouse +import sqlalchemy as sa + +log = logging.getLogger(__name__) + +## Reactive algorithm + + +def match_prio_rule(cz, pr: dict) -> bool: + """Match a priority rule to citizenlab entry""" + for k in ["category_code", "domain", "url"]: + if pr[k] not in ("", "*", cz[k]): + return False + + if cz["cc"] != "ZZ" and pr["cc"].upper() not in ("", "*", cz["cc"].upper()): + return False + + return True + + +def compute_priorities(entries: tuple, prio_rules: tuple) -> list: + # Order based on (msmt_cnt / priority) to provide balancing + test_list = [] + for e in entries: + # Calculate priority for an URL + priority = 0 + for pr in prio_rules: + if match_prio_rule(e, pr): + priority += pr["priority"] + + o = dict(e) + o["priority"] = priority + o["weight"] = priority / max(e["msmt_cnt"], 0.1) + test_list.append(o) + + return sorted(test_list, key=lambda k: k["weight"], reverse=True) + + +@timer +def fetch_reactive_url_list( + clickhouse_db: Clickhouse, cc: str, probe_asn: int +) -> tuple: + """Select all citizenlab URLs for the given probe_cc + ZZ + Select measurements count from the current and previous week + using a left outer join (without any info about priority)""" + q = """ + SELECT category_code, domain, url, cc, COALESCE(msmt_cnt, 0) AS msmt_cnt + FROM ( + SELECT domain, url, cc, category_code + FROM citizenlab + WHERE + citizenlab.cc = :cc_low + OR citizenlab.cc = :cc + OR citizenlab.cc = 'ZZ' + ) AS citiz + LEFT OUTER JOIN ( + SELECT input, SUM(msmt_cnt) AS msmt_cnt + FROM counters_asn_test_list + WHERE probe_cc = :cc + AND (week IN (toStartOfWeek(now()), toStartOfWeek(now() - interval 1 week))) + --asn-filter-- + GROUP BY input + ) AS cnt + ON (citiz.url = cnt.input) + """ + if probe_asn != 0: + q = q.replace("--asn-filter--", "AND probe_asn = :asn") + + # support uppercase or lowercase match + r = query_click( + clickhouse_db, + sa.text(q), + dict(cc=cc, cc_low=cc.lower(), asn=probe_asn), + query_prio=1, + ) + return tuple(r) + + +@timer +def fetch_prioritization_rules(clickhouse_db: Clickhouse, cc: str) -> tuple: + sql = """SELECT category_code, cc, domain, url, priority + FROM url_priorities WHERE cc = :cc OR cc = '*' OR cc = '' + """ + q = query_click(clickhouse_db, sa.text(sql), dict(cc=cc), query_prio=1) + return tuple(q) + + +@timer +def generate_test_list( + clickhouse: Clickhouse, + country_code: str, + category_codes: List, + probe_asn: int, + limit: int, + debug: bool, +) -> Tuple[List, Tuple, Tuple]: + """Generate test list based on the amount of measurements in the last + N days""" + entries = fetch_reactive_url_list(clickhouse, country_code, probe_asn) + log.info("fetched %d url entries", len(entries)) + prio_rules = fetch_prioritization_rules(clickhouse, country_code) + log.info("fetched %d priority rules", len(prio_rules)) + li = compute_priorities(entries, prio_rules) + # Filter unwanted category codes, replace ZZ, trim priority <= 0 + out = [] + for entry in li: + if category_codes and entry["category_code"] not in category_codes: + continue + if entry["priority"] <= 0: + continue + + cc = "XX" if entry["cc"] == "ZZ" else entry["cc"].upper() + i = { + "category_code": entry["category_code"], + "url": entry["url"], + "country_code": cc, + } + if debug: + i["msmt_cnt"] = entry["msmt_cnt"] + i["priority"] = entry["priority"] + i["weight"] = entry["weight"] + out.append(i) + if len(out) >= limit: + break + + if debug: + return out, entries, prio_rules + return out, (), () \ No newline at end of file From af5bbf36224b077ffe3d7ffb9de7ad934571b48b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 28 May 2025 15:23:57 +0200 Subject: [PATCH 28/72] Move prio to common --- .../services/ooniprobe/src/ooniprobe/prio.py | 157 ------------------ .../ooniprobe/routers/v1/probe_services.py | 2 +- 2 files changed, 1 insertion(+), 158 deletions(-) delete mode 100644 ooniapi/services/ooniprobe/src/ooniprobe/prio.py diff --git a/ooniapi/services/ooniprobe/src/ooniprobe/prio.py b/ooniapi/services/ooniprobe/src/ooniprobe/prio.py deleted file mode 100644 index b84a9e42d..000000000 --- a/ooniapi/services/ooniprobe/src/ooniprobe/prio.py +++ /dev/null @@ -1,157 +0,0 @@ -""" -OONI Probe Services API - reactive URL prioritization - -/api/v1/test-list/urls provides dynamic URL tests lists for web_connectivity -based on the citizenlab URL list and the measurements count from the last -7 days. - -The ooni-update-counters service updates the counters_test_list table at intervals - -The ooni-update-citizenlab service updates the citizenlab table at intervals - -``` -blockdiag { - Probes [color = "#ffeeee"]; - "API: test-list/urls" [color = "#eeeeff"]; - Probes -> "API: receive msmt" -> "Fastpath" -> "DB: fastpath table"; - "DB: fastpath table" -> "ooni-update-counters service" -> "DB: counters_test_list table"; - "DB: counters_test_list table" -> "API: test-list/urls" -> Probes; - "DB: citizenlab table" -> "API: test-list/urls"; -} -``` -""" - -from typing import List, Tuple -import logging - -from .common.clickhouse_utils import query_click -from .common.metrics import timer - -from clickhouse_driver import Client as Clickhouse -import sqlalchemy as sa - -log = logging.getLogger(__name__) - -## Reactive algorithm - - -def match_prio_rule(cz, pr: dict) -> bool: - """Match a priority rule to citizenlab entry""" - for k in ["category_code", "domain", "url"]: - if pr[k] not in ("", "*", cz[k]): - return False - - if cz["cc"] != "ZZ" and pr["cc"].upper() not in ("", "*", cz["cc"].upper()): - return False - - return True - - -def compute_priorities(entries: tuple, prio_rules: tuple) -> list: - # Order based on (msmt_cnt / priority) to provide balancing - test_list = [] - for e in entries: - # Calculate priority for an URL - priority = 0 - for pr in prio_rules: - if match_prio_rule(e, pr): - priority += pr["priority"] - - o = dict(e) - o["priority"] = priority - o["weight"] = priority / max(e["msmt_cnt"], 0.1) - test_list.append(o) - - return sorted(test_list, key=lambda k: k["weight"], reverse=True) - - -@timer -def fetch_reactive_url_list( - clickhouse_db: Clickhouse, cc: str, probe_asn: int -) -> tuple: - """Select all citizenlab URLs for the given probe_cc + ZZ - Select measurements count from the current and previous week - using a left outer join (without any info about priority)""" - q = """ - SELECT category_code, domain, url, cc, COALESCE(msmt_cnt, 0) AS msmt_cnt - FROM ( - SELECT domain, url, cc, category_code - FROM citizenlab - WHERE - citizenlab.cc = :cc_low - OR citizenlab.cc = :cc - OR citizenlab.cc = 'ZZ' - ) AS citiz - LEFT OUTER JOIN ( - SELECT input, SUM(msmt_cnt) AS msmt_cnt - FROM counters_asn_test_list - WHERE probe_cc = :cc - AND (week IN (toStartOfWeek(now()), toStartOfWeek(now() - interval 1 week))) - --asn-filter-- - GROUP BY input - ) AS cnt - ON (citiz.url = cnt.input) - """ - if probe_asn != 0: - q = q.replace("--asn-filter--", "AND probe_asn = :asn") - - # support uppercase or lowercase match - r = query_click( - clickhouse_db, - sa.text(q), - dict(cc=cc, cc_low=cc.lower(), asn=probe_asn), - query_prio=1, - ) - return tuple(r) - - -@timer -def fetch_prioritization_rules(clickhouse_db: Clickhouse, cc: str) -> tuple: - sql = """SELECT category_code, cc, domain, url, priority - FROM url_priorities WHERE cc = :cc OR cc = '*' OR cc = '' - """ - q = query_click(clickhouse_db, sa.text(sql), dict(cc=cc), query_prio=1) - return tuple(q) - - -@timer -def generate_test_list( - clickhouse: Clickhouse, - country_code: str, - category_codes: List, - probe_asn: int, - limit: int, - debug: bool, -) -> Tuple[List, Tuple, Tuple]: - """Generate test list based on the amount of measurements in the last - N days""" - entries = fetch_reactive_url_list(clickhouse, country_code, probe_asn) - log.info("fetched %d url entries", len(entries)) - prio_rules = fetch_prioritization_rules(clickhouse, country_code) - log.info("fetched %d priority rules", len(prio_rules)) - li = compute_priorities(entries, prio_rules) - # Filter unwanted category codes, replace ZZ, trim priority <= 0 - out = [] - for entry in li: - if category_codes and entry["category_code"] not in category_codes: - continue - if entry["priority"] <= 0: - continue - - cc = "XX" if entry["cc"] == "ZZ" else entry["cc"].upper() - i = { - "category_code": entry["category_code"], - "url": entry["url"], - "country_code": cc, - } - if debug: - i["msmt_cnt"] = entry["msmt_cnt"] - i["priority"] = entry["priority"] - i["weight"] = entry["weight"] - out.append(i) - if len(out) >= limit: - break - - if debug: - return out, entries, prio_rules - return out, (), () diff --git a/ooniapi/services/ooniprobe/src/ooniprobe/routers/v1/probe_services.py b/ooniapi/services/ooniprobe/src/ooniprobe/routers/v1/probe_services.py index 23a301825..90076e52d 100644 --- a/ooniapi/services/ooniprobe/src/ooniprobe/routers/v1/probe_services.py +++ b/ooniapi/services/ooniprobe/src/ooniprobe/routers/v1/probe_services.py @@ -20,7 +20,7 @@ from ...common.auth import create_jwt, decode_jwt, jwt from ...common.config import Settings from ...common.utils import setnocacheresponse -from ...prio import generate_test_list +from ...common.prio import generate_test_list router = APIRouter(prefix="/v1") From 4c02b0380ffdb33bb722ea4c3bd3c7b3360cb665 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 28 May 2025 15:24:22 +0200 Subject: [PATCH 29/72] Move prio to common --- ooniapi/services/ooniprobe/tests/test_prio.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ooniapi/services/ooniprobe/tests/test_prio.py b/ooniapi/services/ooniprobe/tests/test_prio.py index ba1a85b67..362e7066a 100644 --- a/ooniapi/services/ooniprobe/tests/test_prio.py +++ b/ooniapi/services/ooniprobe/tests/test_prio.py @@ -1,4 +1,4 @@ -from ooniprobe import prio +from ooniprobe.common import prio def test_prio(): From aba48e44654f82652591d2af88ee2d9f571cce3f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 28 May 2025 15:57:21 +0200 Subject: [PATCH 30/72] Changed priority list type --- .../versions/b860eb79750f_add_targets_name_and_inputs_extra_.py | 2 +- ooniapi/common/src/common/postgresql.py | 1 + ooniapi/services/oonirun/src/oonirun/models.py | 2 +- 3 files changed, 3 insertions(+), 2 deletions(-) diff --git a/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py index fae75a9df..8163a78bf 100644 --- a/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py +++ b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py @@ -23,7 +23,7 @@ def upgrade() -> None: sa.Column("targets_name", sa.String(), nullable=True) ) op.add_column("oonirun_nettest", - sa.Column("inputs_extra", sa.JSON(), nullable=True) + sa.Column("inputs_extra", sa.ARRAY(sa.JSON()), nullable=True) ) diff --git a/ooniapi/common/src/common/postgresql.py b/ooniapi/common/src/common/postgresql.py index c3359c46e..647d95933 100644 --- a/ooniapi/common/src/common/postgresql.py +++ b/ooniapi/common/src/common/postgresql.py @@ -9,4 +9,5 @@ class Base(DeclarativeBase): Dict[str, Any]: sa.JSON, List[str]: sa.JSON, Dict[str, str]: sa.JSON, + List[Dict[str, Any]] : sa.ARRAY(sa.JSON()) } diff --git a/ooniapi/services/oonirun/src/oonirun/models.py b/ooniapi/services/oonirun/src/oonirun/models.py index d4d2df6dd..f318bfedd 100644 --- a/ooniapi/services/oonirun/src/oonirun/models.py +++ b/ooniapi/services/oonirun/src/oonirun/models.py @@ -62,4 +62,4 @@ class OONIRunLinkNettest(Base): is_background_run_enabled_default: Mapped[bool] = mapped_column(default=False) is_manual_run_enabled_default: Mapped[bool] = mapped_column(default=False) targets_name: Mapped[str] = mapped_column(nullable=True) - inputs_extra: Mapped[Dict[str, Any]] = mapped_column(nullable=True) + inputs_extra: Mapped[List[Dict[str, Any]]] = mapped_column(nullable=True) From 32196044dcc39c75e8f1998696dc3febdcd04c65 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 11:04:07 +0200 Subject: [PATCH 31/72] Added clickhouse to oonirun --- ooniapi/services/oonirun/pyproject.toml | 2 +- .../oonirun/src/oonirun/dependencies.py | 18 +- ooniapi/services/oonirun/src/oonirun/main.py | 21 +- ooniapi/services/oonirun/tests/conftest.py | 23 ++ .../services/oonirun/tests/docker-compose.yml | 12 + .../fixtures/data/url_priorities_us.json | 338 ++++++++++++++++++ .../tests/fixtures/initdb/01-scheme.sql | 260 ++++++++++++++ .../tests/fixtures/initdb/02-fixtures.sql | 24 ++ .../oonirun/tests/fixtures/initdb/init.sh | 4 + 9 files changed, 695 insertions(+), 7 deletions(-) create mode 100644 ooniapi/services/oonirun/tests/docker-compose.yml create mode 100644 ooniapi/services/oonirun/tests/fixtures/data/url_priorities_us.json create mode 100644 ooniapi/services/oonirun/tests/fixtures/initdb/01-scheme.sql create mode 100644 ooniapi/services/oonirun/tests/fixtures/initdb/02-fixtures.sql create mode 100755 ooniapi/services/oonirun/tests/fixtures/initdb/init.sh diff --git a/ooniapi/services/oonirun/pyproject.toml b/ooniapi/services/oonirun/pyproject.toml index 0dc165c5d..958be2c39 100644 --- a/ooniapi/services/oonirun/pyproject.toml +++ b/ooniapi/services/oonirun/pyproject.toml @@ -58,7 +58,7 @@ packages = ["src/oonirun"] artifacts = ["BUILD_LABEL"] [tool.hatch.envs.default] -dependencies = ["pytest", "pytest-cov", "click", "black", "pytest-postgresql", "pytest-asyncio"] +dependencies = ["pytest", "pytest-cov", "click", "black", "pytest-postgresql", "pytest-asyncio", "pytest-docker"] path = ".venv/" [tool.hatch.envs.default.scripts] diff --git a/ooniapi/services/oonirun/src/oonirun/dependencies.py b/ooniapi/services/oonirun/src/oonirun/dependencies.py index c6dee68d8..cee3f1f50 100644 --- a/ooniapi/services/oonirun/src/oonirun/dependencies.py +++ b/ooniapi/services/oonirun/src/oonirun/dependencies.py @@ -1,4 +1,3 @@ -from functools import lru_cache from typing import Annotated from fastapi import Depends @@ -6,11 +5,15 @@ from sqlalchemy import create_engine from sqlalchemy.orm import sessionmaker, Session +from clickhouse_driver import Client as Clickhouse + from .common.config import Settings from .common.dependencies import get_settings -def get_postgresql_session(settings: Annotated[Settings, Depends(get_settings)]): +DependsSettings = Annotated[Settings, Depends(get_settings)] + +def get_postgresql_session(settings: DependsSettings): engine = create_engine(settings.postgresql_url) SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) @@ -20,4 +23,13 @@ def get_postgresql_session(settings: Annotated[Settings, Depends(get_settings)]) finally: db.close() -DependsPostgresSession = Annotated[Session, Depends(get_postgresql_session)] \ No newline at end of file +DependsPostgresSession = Annotated[Session, Depends(get_postgresql_session)] + +def get_clickhouse_session(settings: DependsSettings): + db = Clickhouse.from_url(settings.clickhouse_url) + try: + yield db + finally: + db.disconnect() + +DependsClickhouseSession = Annotated[Clickhouse, Depends(get_clickhouse_session)] \ No newline at end of file diff --git a/ooniapi/services/oonirun/src/oonirun/main.py b/ooniapi/services/oonirun/src/oonirun/main.py index 90ec41978..34d8f22e8 100644 --- a/ooniapi/services/oonirun/src/oonirun/main.py +++ b/ooniapi/services/oonirun/src/oonirun/main.py @@ -11,12 +11,14 @@ from . import models from .routers import v2 -from .dependencies import get_postgresql_session +from .dependencies import DependsPostgresSession, DependsClickhouseSession, DependsSettings from .common.dependencies import get_settings from .common.version import get_build_label, get_pkg_version from .common.version import get_build_label, get_pkg_version from .common.metrics import mount_metrics +from .common.clickhouse_utils import query_click +log = logging.getLogger(__name__) pkg_name = "oonirun" @@ -63,10 +65,23 @@ class HealthStatus(BaseModel): @app.get("/health") async def health( - settings=Depends(get_settings), - db=Depends(get_postgresql_session), + settings : DependsSettings, + db : DependsPostgresSession, + clickhouse : DependsClickhouseSession, ): errors = [] + + try: + query = """ + SELECT COUNT() + FROM fastpath + WHERE measurement_start_time < NOW() AND measurement_start_time > NOW() - INTERVAL 3 HOUR + """ + query_click(db=clickhouse, query=query, query_params={}) + except Exception as e: + errors.append("clickhouse_error") + log.error(e) + try: db.query(models.OONIRunLink).limit(1).all() except Exception as exc: diff --git a/ooniapi/services/oonirun/tests/conftest.py b/ooniapi/services/oonirun/tests/conftest.py index 4c6f716ee..2d940656c 100644 --- a/ooniapi/services/oonirun/tests/conftest.py +++ b/ooniapi/services/oonirun/tests/conftest.py @@ -9,6 +9,7 @@ from oonirun.common.config import Settings from oonirun.common.dependencies import get_settings from oonirun.main import app +from clickhouse_driver import Client as ClickhouseClient def make_override_get_settings(**kw): @@ -92,3 +93,25 @@ def client_with_admin_role(client): jwt_token = create_session_token("0" * 16, "admin") client.headers = {"Authorization": f"Bearer {jwt_token}"} yield client + +def is_clickhouse_running(url): + try: + with ClickhouseClient.from_url(url) as client: + client.execute("SELECT 1") + return True + except Exception: + return False + +@pytest.fixture(scope="session") +def clickhouse_server(docker_ip, docker_services): + port = docker_services.port_for("clickhouse", 9000) + # See password in docker compose + url = "clickhouse://test:test@{}:{}".format(docker_ip, port) + docker_services.wait_until_responsive( + timeout=30.0, pause=0.1, check=lambda: is_clickhouse_running(url) + ) + yield url + +@pytest.fixture(scope="session") +def clickhouse_db(clickhouse_server): + yield ClickhouseClient.from_url(clickhouse_server) \ No newline at end of file diff --git a/ooniapi/services/oonirun/tests/docker-compose.yml b/ooniapi/services/oonirun/tests/docker-compose.yml new file mode 100644 index 000000000..7eb60f627 --- /dev/null +++ b/ooniapi/services/oonirun/tests/docker-compose.yml @@ -0,0 +1,12 @@ +version: '2' +services: + clickhouse: + image: "clickhouse/clickhouse-server" + ports: + - "9000" + volumes: + - ./fixtures:/fixtures + - ./fixtures/initdb:/docker-entrypoint-initdb.d/ + environment: + CLICKHOUSE_USER: test + CLICKHOUSE_PASSWORD: test diff --git a/ooniapi/services/oonirun/tests/fixtures/data/url_priorities_us.json b/ooniapi/services/oonirun/tests/fixtures/data/url_priorities_us.json new file mode 100644 index 000000000..2171415e2 --- /dev/null +++ b/ooniapi/services/oonirun/tests/fixtures/data/url_priorities_us.json @@ -0,0 +1,338 @@ +[ + { + "category_code": "NEWS", + "cc": "*", + "domain": "*", + "priority": 100, + "url": "*" + }, + { + "category_code": "POLR", + "cc": "*", + "domain": "*", + "priority": 100, + "url": "*" + }, + { + "category_code": "HUMR", + "cc": "*", + "domain": "*", + "priority": 100, + "url": "*" + }, + { + "category_code": "LGBT", + "cc": "*", + "domain": "*", + "priority": 100, + "url": "*" + }, + { + "category_code": "ANON", + "cc": "*", + "domain": "*", + "priority": 100, + "url": "*" + }, + { + "category_code": "GRP", + "cc": "*", + "domain": "*", + "priority": 80, + "url": "*" + }, + { + "category_code": "COMT", + "cc": "*", + "domain": "*", + "priority": 80, + "url": "*" + }, + { + "category_code": "MMED", + "cc": "*", + "domain": "*", + "priority": 80, + "url": "*" + }, + { + "category_code": "SRCH", + "cc": "*", + "domain": "*", + "priority": 80, + "url": "*" + }, + { + "category_code": "PUBH", + "cc": "*", + "domain": "*", + "priority": 80, + "url": "*" + }, + { + "category_code": "REL", + "cc": "*", + "domain": "*", + "priority": 60, + "url": "*" + }, + { + "category_code": "XED", + "cc": "*", + "domain": "*", + "priority": 60, + "url": "*" + }, + { + "category_code": "HOST", + "cc": "*", + "domain": "*", + "priority": 60, + "url": "*" + }, + { + "category_code": "ENV", + "cc": "*", + "domain": "*", + "priority": 60, + "url": "*" + }, + { + "category_code": "FILE", + "cc": "*", + "domain": "*", + "priority": 40, + "url": "*" + }, + { + "category_code": "CULTR", + "cc": "*", + "domain": "*", + "priority": 40, + "url": "*" + }, + { + "category_code": "IGO", + "cc": "*", + "domain": "*", + "priority": 40, + "url": "*" + }, + { + "category_code": "GOVT", + "cc": "*", + "domain": "*", + "priority": 40, + "url": "*" + }, + { + "category_code": "DATE", + "cc": "*", + "domain": "*", + "priority": 30, + "url": "*" + }, + { + "category_code": "HATE", + "cc": "*", + "domain": "*", + "priority": 30, + "url": "*" + }, + { + "category_code": "MILX", + "cc": "*", + "domain": "*", + "priority": 30, + "url": "*" + }, + { + "category_code": "PROV", + "cc": "*", + "domain": "*", + "priority": 30, + "url": "*" + }, + { + "category_code": "PORN", + "cc": "*", + "domain": "*", + "priority": 30, + "url": "*" + }, + { + "category_code": "GMB", + "cc": "*", + "domain": "*", + "priority": 30, + "url": "*" + }, + { + "category_code": "ALDR", + "cc": "*", + "domain": "*", + "priority": 30, + "url": "*" + }, + { + "category_code": "GAME", + "cc": "*", + "domain": "*", + "priority": 20, + "url": "*" + }, + { + "category_code": "MISC", + "cc": "*", + "domain": "*", + "priority": 20, + "url": "*" + }, + { + "category_code": "HACK", + "cc": "*", + "domain": "*", + "priority": 20, + "url": "*" + }, + { + "category_code": "ECON", + "cc": "*", + "domain": "*", + "priority": 20, + "url": "*" + }, + { + "category_code": "COMM", + "cc": "*", + "domain": "*", + "priority": 20, + "url": "*" + }, + { + "category_code": "CTRL", + "cc": "*", + "domain": "*", + "priority": 20, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "www.facebook.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "twitter.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "www.instagram.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "www.whatsapp.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "web.whatsapp.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "telegram.org", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "web.telegram.org", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "www.youtube.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "www.tiktok.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "www.viber.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "www.snapchat.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "www.reddit.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "vimeo.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "www.wechat.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "international.qq.com", + "priority": 200, + "url": "*" + }, + { + "category_code": "*", + "cc": "*", + "domain": "signal.org", + "priority": 200, + "url": "*" + }, + { + "category_code": "MISC", + "cc": "US", + "domain": "*", + "priority": -200, + "url": "*" + } +] diff --git a/ooniapi/services/oonirun/tests/fixtures/initdb/01-scheme.sql b/ooniapi/services/oonirun/tests/fixtures/initdb/01-scheme.sql new file mode 100644 index 000000000..b3081dd47 --- /dev/null +++ b/ooniapi/services/oonirun/tests/fixtures/initdb/01-scheme.sql @@ -0,0 +1,260 @@ +-- Create tables for Clickhouse integ tests + +-- Main tables + +CREATE TABLE default.fastpath +( + `measurement_uid` String, + `report_id` String, + `input` String, + `probe_cc` String, + `probe_asn` UInt32, + `test_name` String, + `test_start_time` DateTime, + `measurement_start_time` DateTime, + `filename` String, + `scores` String, + `platform` String, + `anomaly` String, + `confirmed` String, + `msm_failure` String, + `domain` String, + `software_name` String, + `software_version` String, + `control_failure` String, + `blocking_general` Float32, + `is_ssl_expected` Int8, + `page_len` Int32, + `page_len_ratio` Float32, + `server_cc` String, + `server_asn` Int8, + `server_as_name` String, + `update_time` DateTime64(3) MATERIALIZED now64(), + `test_version` String, + `test_runtime` Float32, + `architecture` String, + `engine_name` String, + `engine_version` String, + `blocking_type` String, + `test_helper_address` LowCardinality(String), + `test_helper_type` LowCardinality(String), + `ooni_run_link_id` Nullable(UInt64) +) +ENGINE = ReplacingMergeTree +ORDER BY (measurement_start_time, report_id, input) +SETTINGS index_granularity = 8192; + +CREATE TABLE default.jsonl +( + `report_id` String, + `input` String, + `s3path` String, + `linenum` Int32, + `measurement_uid` String +) +ENGINE = MergeTree +ORDER BY (report_id, input) +SETTINGS index_granularity = 8192; + +CREATE TABLE default.url_priorities ( + `sign` Int8, + `category_code` String, + `cc` String, + `domain` String, + `url` String, + `priority` Int32 +) +ENGINE = CollapsingMergeTree(sign) +ORDER BY (category_code, cc, domain, url, priority) +SETTINGS index_granularity = 1024; + +CREATE TABLE default.citizenlab +( + `domain` String, + `url` String, + `cc` FixedString(32), + `category_code` String +) +ENGINE = ReplacingMergeTree +ORDER BY (domain, url, cc, category_code) +SETTINGS index_granularity = 4; + +CREATE TABLE default.citizenlab_flip AS default.citizenlab; + +CREATE TABLE test_groups ( + `test_name` String, + `test_group` String +) +ENGINE = Join(ANY, LEFT, test_name); + + +-- Auth + +CREATE TABLE accounts +( + `account_id` FixedString(32), + `role` String +) +ENGINE = EmbeddedRocksDB +PRIMARY KEY account_id; + +CREATE TABLE session_expunge +( + `account_id` FixedString(32), + `threshold` DateTime DEFAULT now() +) +ENGINE = EmbeddedRocksDB +PRIMARY KEY account_id; + +-- Materialized views + +CREATE MATERIALIZED VIEW default.counters_test_list +( + `day` DateTime, + `probe_cc` String, + `input` String, + `msmt_cnt` UInt64 +) +ENGINE = SummingMergeTree +PARTITION BY day +ORDER BY (probe_cc, input) +SETTINGS index_granularity = 8192 AS +SELECT + toDate(measurement_start_time) AS day, + probe_cc, + input, + count() AS msmt_cnt +FROM default.fastpath +INNER JOIN default.citizenlab ON fastpath.input = citizenlab.url +WHERE (measurement_start_time < now()) AND (measurement_start_time > (now() - toIntervalDay(8))) AND (test_name = 'web_connectivity') +GROUP BY + day, + probe_cc, + input; + +CREATE MATERIALIZED VIEW default.counters_asn_test_list +( + `week` DateTime, + `probe_cc` String, + `probe_asn` UInt32, + `input` String, + `msmt_cnt` UInt64 +) +ENGINE = SummingMergeTree +ORDER BY (probe_cc, probe_asn, input) +SETTINGS index_granularity = 8192 AS +SELECT + toStartOfWeek(measurement_start_time) AS week, + probe_cc, + probe_asn, + input, + count() AS msmt_cnt +FROM default.fastpath +INNER JOIN default.citizenlab ON fastpath.input = citizenlab.url +WHERE (measurement_start_time < now()) AND (measurement_start_time > (now() - toIntervalDay(8))) AND (test_name = 'web_connectivity') +GROUP BY + week, + probe_cc, + probe_asn, + input; + +CREATE TABLE msmt_feedback +( + `measurement_uid` String, + `account_id` String, + `status` String, + `update_time` DateTime64(3) MATERIALIZED now64() +) +ENGINE = ReplacingMergeTree +ORDER BY (measurement_uid, account_id) +SETTINGS index_granularity = 4; + +CREATE TABLE default.fingerprints_dns +( + `name` String, + `scope` Enum8('nat' = 1, 'isp' = 2, 'prod' = 3, 'inst' = 4, 'vbw' = 5, 'fp' = 6), + `other_names` String, + `location_found` String, + `pattern_type` Enum8('full' = 1, 'prefix' = 2, 'contains' = 3, 'regexp' = 4), + `pattern` String, + `confidence_no_fp` UInt8, + `expected_countries` String, + `source` String, + `exp_url` String, + `notes` String +) +ENGINE = EmbeddedRocksDB +PRIMARY KEY name; + +CREATE TABLE default.fingerprints_http +( + `name` String, + `scope` Enum8('nat' = 1, 'isp' = 2, 'prod' = 3, 'inst' = 4, 'vbw' = 5, 'fp' = 6, 'injb' = 7, 'prov' = 8), + `other_names` String, + `location_found` String, + `pattern_type` Enum8('full' = 1, 'prefix' = 2, 'contains' = 3, 'regexp' = 4), + `pattern` String, + `confidence_no_fp` UInt8, + `expected_countries` String, + `source` String, + `exp_url` String, + `notes` String +) +ENGINE = EmbeddedRocksDB +PRIMARY KEY name; + +CREATE TABLE asnmeta +( + `asn` UInt32, + `org_name` String, + `cc` String, + `changed` Date, + `aut_name` String, + `source` String +) +ENGINE = MergeTree +ORDER BY (asn, changed); + +CREATE TABLE IF NOT EXISTS default.incidents +( + `update_time` DateTime DEFAULT now(), + `create_time` DateTime DEFAULT now(), + `start_time` DateTime DEFAULT now(), + `end_time` Nullable(DateTime), + `creator_account_id` FixedString(32), + `reported_by` String, + `email_address` String, + `id` String, + `title` String, + `text` String, + `event_type` LowCardinality(String), + `published` UInt8, + `deleted` UInt8 DEFAULT 0, + `CCs` Array(FixedString(2)), + `ASNs` Array(UInt32), + `domains` Array(String), + `tags` Array(String), + `links` Array(String), + `test_names` Array(String), + `short_description` String, +) +ENGINE = ReplacingMergeTree(update_time) +ORDER BY (id) +SETTINGS index_granularity = 1; + +CREATE TABLE IF NOT EXISTS default.oonirun +( + `ooni_run_link_id` UInt64, + `descriptor_creation_time` DateTime64(3), + `translation_creation_time` DateTime64(3), + `creator_account_id` FixedString(32), + `archived` UInt8 DEFAULT 0, + `descriptor` String, + `author` String, + `name` String, + `short_description` String, + `icon` String +) +ENGINE = ReplacingMergeTree(translation_creation_time) +ORDER BY (ooni_run_link_id, descriptor_creation_time) +SETTINGS index_granularity = 1; diff --git a/ooniapi/services/oonirun/tests/fixtures/initdb/02-fixtures.sql b/ooniapi/services/oonirun/tests/fixtures/initdb/02-fixtures.sql new file mode 100644 index 000000000..0adf8e8db --- /dev/null +++ b/ooniapi/services/oonirun/tests/fixtures/initdb/02-fixtures.sql @@ -0,0 +1,24 @@ + +-- Populate lookup tables + +INSERT INTO test_groups (test_name, test_group) VALUES ('bridge_reachability', 'circumvention'), ('meek_fronted_requests_test', 'circumvention'), ('psiphon', 'circumvention'), ('riseupvpn', 'circumvention'), ('tcp_connect', 'circumvention'), ('tor', 'circumvention'), ('torsf', 'circumvention'), ('vanilla_tor', 'circumvention'), ('dnscheck', 'experimental'), ('urlgetter', 'experimental'), ('facebook_messenger', 'im'), ('signal', 'im'), ('telegram', 'im'), ('whatsapp', 'im'), ('dns_consistency', 'legacy'), ('http_host', 'legacy'), ('http_requests', 'legacy'), ('multi_protocol_traceroute', 'legacy'), ('http_header_field_manipulation', 'middlebox'), ('http_invalid_request_line', 'middlebox'), ('dash', 'performance'), ('ndt', 'performance')('web_connectivity', 'websites') ; + +-- Create integ test data for Clickhouse + +INSERT INTO citizenlab VALUES ('www.ushmm.org', 'https://www.ushmm.org/', 'ZZ', 'CULTR'); +INSERT INTO citizenlab VALUES ('www.cabofrio.rj.gov.br', 'http://www.cabofrio.rj.gov.br/', 'BR', 'CULTR'); +INSERT INTO citizenlab VALUES ('ncac.org', 'http://ncac.org/', 'ZZ', 'NEWS'); +INSERT INTO citizenlab VALUES ('ncac.org', 'https://ncac.org/', 'ZZ', 'NEWS'); +INSERT INTO citizenlab VALUES ('www.facebook.com','http://www.facebook.com/saakashvilimikheil','ge','NEWS'); +INSERT INTO citizenlab VALUES ('www.facebook.com','http://www.facebook.com/somsakjeam/videos/1283095981743678/','th','POLR'); +INSERT INTO citizenlab VALUES ('www.facebook.com','https://www.facebook.com/','ZZ','GRP'); +INSERT INTO citizenlab VALUES ('facebook.com','http://facebook.com/','ua','GRP'); +INSERT INTO citizenlab VALUES ('facebook.com','https://facebook.com/watch','jo','GRP'); +INSERT INTO citizenlab VALUES ('twitter.com','http://twitter.com/ghonim','kw','POLR'); +INSERT INTO citizenlab VALUES ('twitter.com','http://twitter.com/ghonim','so','POLR'); +INSERT INTO citizenlab VALUES ('twitter.com','https://twitter.com/','ZZ','GRP'); + +-- get_measurement_meta integ tests +INSERT INTO jsonl (report_id, input, s3path, linenum) VALUES ('20210709T004340Z_webconnectivity_MY_4818_n1_YCM7J9mGcEHds2K3', 'https://www.backtrack-linux.org/', 'raw/20210709/00/MY/webconnectivity/2021070900_MY_webconnectivity.n0.2.jsonl.gz', 35) + + diff --git a/ooniapi/services/oonirun/tests/fixtures/initdb/init.sh b/ooniapi/services/oonirun/tests/fixtures/initdb/init.sh new file mode 100755 index 000000000..0ce1f3a4c --- /dev/null +++ b/ooniapi/services/oonirun/tests/fixtures/initdb/init.sh @@ -0,0 +1,4 @@ +#!/bin/sh +set -e + +# Add initialization code here. Example: fetch data, generate it dynamically \ No newline at end of file From 9d9204257e7444be1e9f6812649ef014f5987331 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 12:46:15 +0200 Subject: [PATCH 32/72] Add clickhouse to clients for testing --- ooniapi/services/oonirun/tests/conftest.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/tests/conftest.py b/ooniapi/services/oonirun/tests/conftest.py index 2d940656c..92efe93a3 100644 --- a/ooniapi/services/oonirun/tests/conftest.py +++ b/ooniapi/services/oonirun/tests/conftest.py @@ -41,7 +41,8 @@ def alembic_migration(postgresql): @pytest.fixture def client_with_bad_settings(): app.dependency_overrides[get_settings] = make_override_get_settings( - postgresql_url="postgresql://bad:bad@localhost/bad" + postgresql_url="postgresql://bad:bad@localhost/bad", + clickhouse_url="clickhouse://bad:bad@localhost/bad" ) client = TestClient(app) @@ -49,11 +50,12 @@ def client_with_bad_settings(): @pytest.fixture -def client(alembic_migration): +def client(alembic_migration, clickhouse_server): app.dependency_overrides[get_settings] = make_override_get_settings( postgresql_url=alembic_migration, jwt_encryption_key="super_secure", prometheus_metrics_password="super_secure", + clickhouse_client = clickhouse_server ) client = TestClient(app) From 06e1d0e8ec50bbfb8bab3e00e6e019a914b942ca Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 12:47:19 +0200 Subject: [PATCH 33/72] Set up get_nettest for dynamic lists calculation --- .../oonirun/src/oonirun/dependencies.py | 2 +- ooniapi/services/oonirun/src/oonirun/main.py | 4 +- .../oonirun/src/oonirun/routers/v2.py | 58 ++++++++++++++----- 3 files changed, 48 insertions(+), 16 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/dependencies.py b/ooniapi/services/oonirun/src/oonirun/dependencies.py index cee3f1f50..6ff998c0b 100644 --- a/ooniapi/services/oonirun/src/oonirun/dependencies.py +++ b/ooniapi/services/oonirun/src/oonirun/dependencies.py @@ -32,4 +32,4 @@ def get_clickhouse_session(settings: DependsSettings): finally: db.disconnect() -DependsClickhouseSession = Annotated[Clickhouse, Depends(get_clickhouse_session)] \ No newline at end of file +DependsClickhouseClient = Annotated[Clickhouse, Depends(get_clickhouse_session)] \ No newline at end of file diff --git a/ooniapi/services/oonirun/src/oonirun/main.py b/ooniapi/services/oonirun/src/oonirun/main.py index 34d8f22e8..5a431eeab 100644 --- a/ooniapi/services/oonirun/src/oonirun/main.py +++ b/ooniapi/services/oonirun/src/oonirun/main.py @@ -11,7 +11,7 @@ from . import models from .routers import v2 -from .dependencies import DependsPostgresSession, DependsClickhouseSession, DependsSettings +from .dependencies import DependsPostgresSession, DependsClickhouseClient, DependsSettings from .common.dependencies import get_settings from .common.version import get_build_label, get_pkg_version from .common.version import get_build_label, get_pkg_version @@ -67,7 +67,7 @@ class HealthStatus(BaseModel): async def health( settings : DependsSettings, db : DependsPostgresSession, - clickhouse : DependsClickhouseSession, + clickhouse : DependsClickhouseClient, ): errors = [] diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 3c81a4a3f..4e4e1fd7d 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -23,7 +23,8 @@ from ..common.auth import ( get_account_id_or_none, ) -from ..dependencies import DependsPostgresSession +from ..common.prio import generate_test_list +from ..dependencies import DependsPostgresSession, DependsClickhouseClient log = logging.getLogger(__name__) @@ -34,6 +35,13 @@ def utcnow_seconds(): return datetime.now(timezone.utc).replace(microsecond=0) +class OonirunMeta(BaseModel): + run_type : str = Field(description="Run type", pattern="^(timed|manual)$") + is_charging : bool = Field(description="If the probe is charging") + probe_asn : str = Field(pattern=r"^([a-zA-Z0-9]+)$") + probe_cc : str = Field(description="Country code. Ex: VE") + network_type : str = Field(description="Ex: wifi") + website_category_codes : List[str] = Field(description="List of category codes that user has chosen to test (eg. NEWS,HUMR)", default=[]) class OONIRunLinkNettest(BaseModel): test_name: str = Field( @@ -363,20 +371,50 @@ def edit_oonirun_link( is_mine=oonirun_link.creator_account_id == account_id, ) +def make_nettests_from_targets_name(targets_name : str, meta : OonirunMeta) -> Tuple[List[str], List[Dict[str, Any]]]: + if targets_name == "websites_list_prioritized": + return make_nettest_websites_list_prioritized(meta) + + raise ValueError("Unknown target name: " + targets_name) + + +def make_nettest_websites_list_prioritized(meta : OonirunMeta) -> Tuple[List[str], List[Dict[str, Any]]]: + """Generates an inputs list using prio. + Returns: + Tuple[List[str], List[Dict[str, Any]]]: (Inputs, InputsExtra) + """ + + raise NotImplementedError("websites_list_prioritized not yet implemented") + def get_nettests( - oonirun_link: models.OONIRunLink, revision: Optional[int] + oonirun_link: models.OONIRunLink, revision: Optional[int], meta : Optional[OonirunMeta] = None ) -> Tuple[List[OONIRunLinkNettest], datetime]: + """Computes a list of nettests related to the given oonirun link + + The `meta` parameter is required for the dynamic tests list calculation. If not provided, + it will skip it. + """ + date_created = oonirun_link.nettests[0].date_created nettests = [] for nt in oonirun_link.nettests: if revision and nt.revision != revision: continue date_created = nt.date_created + inputs, inputs_extra = nt.inputs, nt.inputs_extra + targets_name = nt.targets_name + if nt.targets_name is not None and meta is not None: + inputs, inputs_extra = make_nettests_from_targets_name(nt.targets_name, meta) + # it will crash if we add inputs and targets_name at the same time + targets_name = None + nettests.append( OONIRunLinkNettest( + targets_name=targets_name, test_name=nt.test_name, - inputs=nt.inputs, + inputs=inputs, + inputs_extra=inputs_extra, options=nt.options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, @@ -389,6 +427,7 @@ def make_oonirun_link( db: Session, oonirun_link_id: str, account_id: Optional[str], + meta : Optional[OonirunMeta] = None, revision: Optional[int] = None, ): q = db.query(models.OONIRunLink).filter( @@ -407,7 +446,7 @@ def make_oonirun_link( assert isinstance(revision, int) - nettests, date_created = get_nettests(res, revision) + nettests, date_created = get_nettests(res, revision, meta) return OONIRunLink( oonirun_link_id=res.oonirun_link_id, name=res.name, @@ -481,13 +520,6 @@ def from_header(cls, header : str) -> Self: probe_asn, probe_cc, network_type = matched.groups() return cls(probe_asn=probe_asn, probe_cc=probe_cc, network_type=network_type) -class GetOoniRunLinkEngineDescriptorRequest(BaseModel): - run_type : str = Field(description="Run type", pattern="^(timed|manual)$") - is_charging : bool = Field(description="If the probe is charging") - probe_asn : str = Field(pattern=r"^([a-zA-Z0-9]+)$") - probe_cc : str = Field(description="Country code. Ex: VE") - network_type : str = Field(description="Ex: wifi") - website_category_codes : List[str] = Field(description="List of category codes that user has chosen to test (eg. NEWS,HUMR)", default=[]) @router.post( "/v2/oonirun/links/{oonirun_link_id}/engine-descriptor/{revision_number}", @@ -506,7 +538,7 @@ def get_oonirun_link_engine_descriptor( ), ], db: DependsPostgresSession, - request : GetOoniRunLinkEngineDescriptorRequest, + meta : OonirunMeta, user_agent: Annotated[ Optional[str], # TODO Marked as optional to avoid breaking old probes Header( @@ -541,7 +573,7 @@ def get_oonirun_link_engine_descriptor( revision = latest_revision assert isinstance(revision, int) - nettests, date_created = get_nettests(res, revision) + nettests, date_created = get_nettests(res, revision, meta) return OONIRunLinkEngineDescriptor( nettests=nettests, date_created=date_created, From ea6cafb0ef3952e92e2653b831aa5b5f862ffa3d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 13:17:06 +0200 Subject: [PATCH 34/72] Enforce targets_name/inputs validation only on write ops --- .../oonirun/src/oonirun/routers/v2.py | 19 ++++++++++++++++++- 1 file changed, 18 insertions(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 4e4e1fd7d..d3ab9a3f6 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -78,7 +78,6 @@ def validate_inputs_extra(self) -> Self: ) return self - @model_validator(mode="after") def validate_no_inputs_and_targets_name(self): """ Check that you are not providing targets_name and inputs-inputs_extra in the same request @@ -204,6 +203,15 @@ def create_oonirun_link( status_code=400, detail="email_address must match the email address of the user who created the oonirun link", ) + + for nt in create_request.nettests: + try: + nt.validate_no_inputs_and_targets_name() + except ValueError as e: + raise HTTPException( + status_code = 422, + detail = {"error": str(e)} + ) now = utcnow_seconds() @@ -279,6 +287,15 @@ def edit_oonirun_link( log.debug(f"edit oonirun {oonirun_link_id}") account_id = token["account_id"] + for nt in edit_request.nettests: + try: + nt.validate_no_inputs_and_targets_name() + except ValueError as e: + raise HTTPException( + status_code = 422, + detail = {"error": str(e)} + ) + now = utcnow_seconds() q = db.query(models.OONIRunLink).filter( From 62b7864683ac6e2753e03078528cbc1676582ad2 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 13:29:57 +0200 Subject: [PATCH 35/72] Rename function to dynamically compute test lists --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 4 ++-- 1 file changed, 2 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index d3ab9a3f6..50b7c4e61 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -388,7 +388,7 @@ def edit_oonirun_link( is_mine=oonirun_link.creator_account_id == account_id, ) -def make_nettests_from_targets_name(targets_name : str, meta : OonirunMeta) -> Tuple[List[str], List[Dict[str, Any]]]: +def make_test_lists_from_targets_name(targets_name : str, meta : OonirunMeta) -> Tuple[List[str], List[Dict[str, Any]]]: if targets_name == "websites_list_prioritized": return make_nettest_websites_list_prioritized(meta) @@ -422,7 +422,7 @@ def get_nettests( inputs, inputs_extra = nt.inputs, nt.inputs_extra targets_name = nt.targets_name if nt.targets_name is not None and meta is not None: - inputs, inputs_extra = make_nettests_from_targets_name(nt.targets_name, meta) + inputs, inputs_extra = make_test_lists_from_targets_name(nt.targets_name, meta) # it will crash if we add inputs and targets_name at the same time targets_name = None From 0c9ddf692e3f154fb9aa89fbf39565d5796534de Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 14:11:58 +0200 Subject: [PATCH 36/72] Add dynamic test lists calculation to engine-descriptor endpoint --- .../oonirun/src/oonirun/routers/v2.py | 32 ++++++++++++++----- 1 file changed, 24 insertions(+), 8 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 50b7c4e61..e08696c9c 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -6,6 +6,7 @@ from datetime import datetime, timedelta, timezone from typing import Dict, List, Optional, Tuple, Any +from typing_extensions import Annotated, Self import logging import re @@ -14,7 +15,8 @@ from fastapi import APIRouter, Depends, Query, HTTPException, Header, Path from pydantic import computed_field, Field from pydantic.functional_validators import field_validator, model_validator -from typing_extensions import Annotated, Self + +from clickhouse_driver.client import Client as Clickhouse from .. import models @@ -43,6 +45,9 @@ class OonirunMeta(BaseModel): network_type : str = Field(description="Ex: wifi") website_category_codes : List[str] = Field(description="List of category codes that user has chosen to test (eg. NEWS,HUMR)", default=[]) + def probe_asn_int(self) -> int: + return int(self.probe_asn.replace("AS", "")) + class OONIRunLinkNettest(BaseModel): test_name: str = Field( default="", title="name of the ooni nettest", min_length=2, max_length=100 @@ -388,29 +393,38 @@ def edit_oonirun_link( is_mine=oonirun_link.creator_account_id == account_id, ) -def make_test_lists_from_targets_name(targets_name : str, meta : OonirunMeta) -> Tuple[List[str], List[Dict[str, Any]]]: +def make_test_lists_from_targets_name(targets_name : str, meta : OonirunMeta, clickhouse : Clickhouse) -> Tuple[List[str], List[Dict[str, Any]]]: if targets_name == "websites_list_prioritized": - return make_nettest_websites_list_prioritized(meta) + return make_nettest_websites_list_prioritized(meta, clickhouse) raise ValueError("Unknown target name: " + targets_name) -def make_nettest_websites_list_prioritized(meta : OonirunMeta) -> Tuple[List[str], List[Dict[str, Any]]]: +def make_nettest_websites_list_prioritized(meta : OonirunMeta, clickhouse : Clickhouse) -> Tuple[List[str], List[Dict[str, Any]]]: """Generates an inputs list using prio. Returns: Tuple[List[str], List[Dict[str, Any]]]: (Inputs, InputsExtra) """ - raise NotImplementedError("websites_list_prioritized not yet implemented") + if meta.run_type == "manual": + url_limit = 9999 # same as prio.py + elif meta.is_charging: + url_limit = 100 + else: + url_limit = 20 + urls, _1, _2 = generate_test_list(clickhouse, meta.probe_cc, meta.website_category_codes, meta.probe_asn_int(), url_limit, False) + + return urls, [{}] * len(urls) def get_nettests( - oonirun_link: models.OONIRunLink, revision: Optional[int], meta : Optional[OonirunMeta] = None + oonirun_link: models.OONIRunLink, revision: Optional[int], meta : Optional[OonirunMeta] = None, clickhouse : Optional[Clickhouse] = None ) -> Tuple[List[OONIRunLinkNettest], datetime]: """Computes a list of nettests related to the given oonirun link The `meta` parameter is required for the dynamic tests list calculation. If not provided, it will skip it. + """ date_created = oonirun_link.nettests[0].date_created @@ -422,7 +436,8 @@ def get_nettests( inputs, inputs_extra = nt.inputs, nt.inputs_extra targets_name = nt.targets_name if nt.targets_name is not None and meta is not None: - inputs, inputs_extra = make_test_lists_from_targets_name(nt.targets_name, meta) + assert clickhouse is not None, "Clickhouse is required to compute the dynamic lists" + inputs, inputs_extra = make_test_lists_from_targets_name(nt.targets_name, meta, clickhouse) # it will crash if we add inputs and targets_name at the same time targets_name = None @@ -555,6 +570,7 @@ def get_oonirun_link_engine_descriptor( ), ], db: DependsPostgresSession, + clickhouse: DependsClickhouseClient, meta : OonirunMeta, user_agent: Annotated[ Optional[str], # TODO Marked as optional to avoid breaking old probes @@ -590,7 +606,7 @@ def get_oonirun_link_engine_descriptor( revision = latest_revision assert isinstance(revision, int) - nettests, date_created = get_nettests(res, revision, meta) + nettests, date_created = get_nettests(res, revision, meta, clickhouse) return OONIRunLinkEngineDescriptor( nettests=nettests, date_created=date_created, From 32d524b02f4a2b41c0790d6f15b59ba0c59f7b2b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 14:15:21 +0200 Subject: [PATCH 37/72] Improve validation of inputs_extra field --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 3 +-- 1 file changed, 1 insertion(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index e08696c9c..25e407d11 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -77,7 +77,7 @@ class OONIRunLinkNettest(BaseModel): @model_validator(mode="after") def validate_inputs_extra(self) -> Self: - if self.inputs_extra is not None and len(self.inputs) != len(self.inputs_extra): + if self.inputs_extra is not None and (self.inputs is None or len(self.inputs) != len(self.inputs_extra)): raise ValueError( "When provided, inputs_extra should be the same length as inputs" ) @@ -584,7 +584,6 @@ def get_oonirun_link_engine_descriptor( credentials : Annotated[Optional[bytes], Header(description="base64 encoded OONI anonymous credentials")] = None, ): """Fetch an OONI Run link by specifying the revision number""" - # TODO Use is_charging and run_type try: revision = int(revision_number) except: From 8e4279f382451ecc5cd7c5bf2fc6d421a46ae961 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 14:18:05 +0200 Subject: [PATCH 38/72] Add todo --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 1 + 1 file changed, 1 insertion(+) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 25e407d11..8ff88c525 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -65,6 +65,7 @@ class OONIRunLinkNettest(BaseModel): default=False, title="if this test should be enabled by default for manual runs" ) + # TODO(luis): Add validation for expected variants of targets_name targets_name: Optional[str] = Field( default=None, description="string used to specify during creation that the input list should be dynamically generated.", From 80db37ad9507ed172ff87f2b15ffb90ed10fd650 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 16:48:22 +0200 Subject: [PATCH 39/72] Fix bad creation of oonirun links with targets_name --- .../oonirun/src/oonirun/routers/v2.py | 3 ++ ooniapi/services/oonirun/tests/conftest.py | 2 +- .../services/oonirun/tests/test_oonirun.py | 53 ++++++++++++++++++- 3 files changed, 56 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 8ff88c525..1255dcb2f 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -243,6 +243,7 @@ def create_oonirun_link( test_name=nt.test_name, inputs=nt.inputs, options=nt.options, + targets_name=nt.targets_name, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, ) @@ -337,6 +338,7 @@ def edit_oonirun_link( assert nt.nettest_index == nettest_index, "inconsistent nettest index" latest_nettests.append( OONIRunLinkNettest( + targets_name=nt.targets_name, test_name=nt.test_name, inputs=nt.inputs, options=nt.options, @@ -349,6 +351,7 @@ def edit_oonirun_link( latest_revision += 1 for nettest_index, nt in enumerate(edit_request.nettests): new_nettest = models.OONIRunLinkNettest( + targets_name=nt.targets_name, revision=latest_revision, nettest_index=nettest_index, date_created=now, diff --git a/ooniapi/services/oonirun/tests/conftest.py b/ooniapi/services/oonirun/tests/conftest.py index 92efe93a3..a534bc61b 100644 --- a/ooniapi/services/oonirun/tests/conftest.py +++ b/ooniapi/services/oonirun/tests/conftest.py @@ -55,7 +55,7 @@ def client(alembic_migration, clickhouse_server): postgresql_url=alembic_migration, jwt_encryption_key="super_secure", prometheus_metrics_password="super_secure", - clickhouse_client = clickhouse_server + clickhouse_url = clickhouse_server ) client = TestClient(app) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 35799b089..2b6cc3475 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -694,7 +694,6 @@ def test_inputs_and_targets_name(client_with_user_role): Test that you can't specify targets_name and inputs in the same request """ - # Both z = deepcopy(SAMPLE_OONIRUN) z['name'] = "Testing no targets and inputs at the same time" @@ -755,6 +754,58 @@ def test_inputs_and_targets_name(client_with_user_role): r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 422, r.json() +def test_creation_with_targets_name(client_with_user_role): + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing dynamic test lists calculation" + z['nettests'][0]['inputs'] = None + z['nettests'][0]['targets_name'] = "websites_list_prioritized" + z['nettests'] = z['nettests'][:1] + + # Create + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, r.json() + j = r.json() + + # Retrieve + r = client_with_user_role.get(f"/api/v2/oonirun/links/{j['oonirun_link_id']}") + assert r.status_code == 200, r.json() + j = r.json() + + # Does it have the targets name? + assert j['nettests'][0]['targets_name'] == "websites_list_prioritized", "Missing targets_name" + + # now test that you can edit + z['nettests'][0]['targets_name'] = "new_value" + r = client_with_user_role.put(f"/api/v2/oonirun/links/{j['oonirun_link_id']}", json=z) + assert r.status_code == 200, r.json() + + # Retrieve again + r = client_with_user_role.get(f"/api/v2/oonirun/links/{j['oonirun_link_id']}") + assert r.status_code == 200, r.json() + j = r.json() + + assert j['nettests'][0]['targets_name'] == 'new_value', "Value of nettest should be changed by now" + + + + +def test_dynamic_test_lists_calculation(client_with_user_role): + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing dynamic test lists calculation" + z['nettests'][0]['inputs'] = None + z['nettests'][0]['targets_name'] = "websites_list_prioritized" + z['nettests'] = z['nettests'][:1] + + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, r.json() + j = r.json() + + r = client_with_user_role.post(f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", json=SAMPLE_META) + assert r.status_code == 200, r.json() + + j = r.json() + + # TODO(luis) finish this test for checking the parsing of user agent headers # def test_x_user_agent_header_parsing(client_with_user_role, client): # z = deepcopy(SAMPLE_OONIRUN) From 2705195b4187cca9ba909243cd3b8b81504be586 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 29 May 2025 17:03:29 +0200 Subject: [PATCH 40/72] Fix bug assigning None to targets_name when not needed --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 2 -- ooniapi/services/oonirun/tests/test_oonirun.py | 6 +++--- 2 files changed, 3 insertions(+), 5 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 1255dcb2f..c592ab460 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -442,8 +442,6 @@ def get_nettests( if nt.targets_name is not None and meta is not None: assert clickhouse is not None, "Clickhouse is required to compute the dynamic lists" inputs, inputs_extra = make_test_lists_from_targets_name(nt.targets_name, meta, clickhouse) - # it will crash if we add inputs and targets_name at the same time - targets_name = None nettests.append( OONIRunLinkNettest( diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 2b6cc3475..70be3d18f 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -785,9 +785,6 @@ def test_creation_with_targets_name(client_with_user_role): j = r.json() assert j['nettests'][0]['targets_name'] == 'new_value', "Value of nettest should be changed by now" - - - def test_dynamic_test_lists_calculation(client_with_user_role): z = deepcopy(SAMPLE_OONIRUN) @@ -804,6 +801,9 @@ def test_dynamic_test_lists_calculation(client_with_user_role): assert r.status_code == 200, r.json() j = r.json() + assert j['nettests'][0]['targets_name'] == "websites_list_prioritized" + + # TODO(luis) Finish this test # TODO(luis) finish this test for checking the parsing of user agent headers From f2ebdc15ae045a11fd3f900520f57fe0a9a69e29 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 30 May 2025 12:20:36 +0200 Subject: [PATCH 41/72] Add header parsing for engine-descriptor endpoint --- .../oonirun/src/oonirun/routers/v2.py | 13 ++- .../services/oonirun/tests/test_database.py | 1 - .../services/oonirun/tests/test_oonirun.py | 81 +++++++++++-------- 3 files changed, 58 insertions(+), 37 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index c592ab460..db315c7c1 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -555,6 +555,7 @@ def from_header(cls, header : str) -> Self: return cls(probe_asn=probe_asn, probe_cc=probe_cc, network_type=network_type) +USER_AGENT_PATTERN = r"^([a-zA-Z0-9\-\_]+)/([a-zA-Z0-9\-\_\.]+) \(([a-zA-Z0-9\ ]+)\) ([a-zA-Z0-9\-\_]+)/([a-zA-Z0-9\-\_\.]+) \(([a-zA-Z0-9\-\_\.]+)\)$" @router.post( "/v2/oonirun/links/{oonirun_link_id}/engine-descriptor/{revision_number}", tags=["oonirun"], @@ -574,11 +575,10 @@ def get_oonirun_link_engine_descriptor( db: DependsPostgresSession, clickhouse: DependsClickhouseClient, meta : OonirunMeta, - user_agent: Annotated[ + useragent: Annotated[ Optional[str], # TODO Marked as optional to avoid breaking old probes Header( - # TODO(luis) This is crashing when the header is not provided (optional). But maybe this header shouldn't be optional at all? - # pattern=r"^([a-zA-Z0-9\-\_]+)/([a-zA-Z0-9\-\_\.]+) \(([a-zA-Z0-9\ ]+)\) ([a-zA-Z0-9\-\_]+)/([a-zA-Z0-9\-\_\.]+)$", + pattern=USER_AGENT_PATTERN, error_message = "Expected format: / () / ()", description="Expected format: / () / ()" ), @@ -592,6 +592,13 @@ def get_oonirun_link_engine_descriptor( # We can assert it, since we are doing validation assert revision_number == "latest" revision = None + + if useragent is not None: + result = re.match(USER_AGENT_PATTERN, useragent) + # Validated by fastapi + assert result is not None + software_name, software_version, platform, engine_name, engine_version, engine_version_full = result.groups() + # TODO Log this metadata q = db.query(models.OONIRunLink).filter( models.OONIRunLink.oonirun_link_id == oonirun_link_id diff --git a/ooniapi/services/oonirun/tests/test_database.py b/ooniapi/services/oonirun/tests/test_database.py index d67964804..cf181629c 100644 --- a/ooniapi/services/oonirun/tests/test_database.py +++ b/ooniapi/services/oonirun/tests/test_database.py @@ -7,7 +7,6 @@ import sqlalchemy as sa from sqlalchemy.orm import sessionmaker from oonirun import models -from oonirun.dependencies import get_postgresql_session from sqlalchemy import create_engine SAMPLE_OONIRUN = { diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 70be3d18f..6cb8dde87 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -6,13 +6,8 @@ from datetime import datetime, timedelta, timezone import time -from oonirun import models from oonirun.routers.v2 import utcnow_seconds -import pytest -import sqlalchemy as sa -from sqlalchemy.orm import sessionmaker -from sqlalchemy import create_engine SAMPLE_OONIRUN = { "name": "", @@ -51,6 +46,15 @@ "is_manual_run_enabled_default": False, "test_name": "dnscheck", }, + { + "inputs": None, + "targets_name": "websites_list_prioritized", + "inputs_extra": None, + "options": {}, + "is_background_run_enabled_default": False, + "is_manual_run_enabled_default": False, + "test_name": "dnscheck", + }, ], } @@ -807,31 +811,42 @@ def test_dynamic_test_lists_calculation(client_with_user_role): # TODO(luis) finish this test for checking the parsing of user agent headers -# def test_x_user_agent_header_parsing(client_with_user_role, client): -# z = deepcopy(SAMPLE_OONIRUN) -# z['name'] = "Testing header parsing" - -# r = client_with_user_role.post("/api/v2/oonirun/links", json=z) -# assert r.status_code == 200, r.json() -# j = r.json() - -# # Test with good headers -# headers = { -# "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2 ooniprobe-engine_1.2.3" -# } -# r = client.post( -# f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", -# headers=headers, -# json={} -# ) -# assert r.status_code == 200, r.json() - -# headers = { -# "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2" -# } -# r = client.post( -# f"/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", -# headers=headers, -# json={} -# ) -# assert r.status_code == 422, r.json() \ No newline at end of file +def test_x_user_agent_header_parsing(client_with_user_role, client): + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing header parsing" + + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, r.json() + j = r.json() + + # Test with good headers + headers = { + "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2 (ooniprobe-engine_1.2.3)" + } + + r = client_with_user_role.post(f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", json=SAMPLE_META) + r = client.post( + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + headers=headers, + json=SAMPLE_META + ) + assert r.status_code == 200, r.json() + + # Should be able to skip the header + r = client_with_user_role.post(f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", json=SAMPLE_META) + r = client.post( + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + json=SAMPLE_META + ) + assert r.status_code == 200, r.json() + + # Bad header + headers = { + "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2" + } + r = client.post( + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + headers=headers, + json=SAMPLE_META + ) + assert r.status_code == 422, r.json() \ No newline at end of file From 09515db4b7ccc36f647d9c890c43b31216d162ed Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 30 May 2025 13:21:04 +0200 Subject: [PATCH 42/72] Fix broken test --- ooniapi/services/oonirun/tests/test_oonirun.py | 13 +++++++++++-- 1 file changed, 11 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 6cb8dde87..229518eb2 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -582,6 +582,15 @@ def test_oonirun_revisions(client, client_with_user_role): assert r.status_code == 200, r.json() j_latest = r.json() assert j_latest["revision"] == "3", "revision is 3" + from pprint import pprint + + # The engine-descriptor returns a list along with targets name on reading + lastest_nettests[2]['inputs'] = [] + lastest_nettests[2]['inputs_extra'] = [] + print("Given:") + pprint(j_latest['nettests']) + print("Expected:") + pprint(lastest_nettests) assert j_latest["nettests"] == lastest_nettests, "nettests are the same" assert j_latest["date_created"] == latest_date_created, "date created matches" @@ -663,14 +672,14 @@ def test_is_latest_list(client, client_with_user_role): assert r.status_code == 200 j = r.json() nts = j['oonirun_links'][0]['nettests'] - assert len(nts) == 2, "There are only 2 nettests in the last revision" + assert len(nts) == 3, "There are only 3 nettests in the last revision" # All revisions r = client.get("/api/v2/oonirun/links", params = {"only_latest" : False}) assert r.status_code == 200 j = r.json() nts = j['oonirun_links'][0]['nettests'] - assert len(nts) == 4, "There are 4 nettests between all revisions" + assert len(nts) == 6, "There are 6 nettests between all revisions" def test_link_revision_args(client, client_with_user_role): # Check args parsing for oonirun engine-descriptor From ec9f7b364f28884164f0a9e45f2b72d02150a996 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 30 May 2025 13:28:17 +0200 Subject: [PATCH 43/72] Fix broken test --- ooniapi/services/oonirun/tests/test_oonirun.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 229518eb2..d2da3f4c6 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -417,7 +417,7 @@ def test_oonirun_full_workflow(client, client_with_user_role, client_with_admin_ r = client_with_user_role.get(f"/api/v2/oonirun/links/{oonirun_link_id}") assert r.status_code == 200, r.json() descs = r.json()["nettests"] - assert len(descs) == 2, r.json() + assert len(descs) == 3, r.json() ## List descriptors r = client_with_user_role.get(f"/api/v2/oonirun/links") From 22a4eb86adaa9aab6c0d2a331915d8531bf91db5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 30 May 2025 15:44:28 +0200 Subject: [PATCH 44/72] Add integration test for dynamic lists calculation --- .../oonirun/src/oonirun/routers/v2.py | 15 ++++- .../oonirun/tests/integ/test_dynamic_lists.py | 59 +++++++++++++++++++ 2 files changed, 72 insertions(+), 2 deletions(-) create mode 100644 ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index db315c7c1..cf12fe403 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -416,9 +416,18 @@ def make_nettest_websites_list_prioritized(meta : OonirunMeta, clickhouse : Clic url_limit = 100 else: url_limit = 20 - urls, _1, _2 = generate_test_list(clickhouse, meta.probe_cc, meta.website_category_codes, meta.probe_asn_int(), url_limit, False) + tests, _1, _2 = generate_test_list(clickhouse, meta.probe_cc, meta.website_category_codes, meta.probe_asn_int(), url_limit, False) - return urls, [{}] * len(urls) + inputs = [] + inputs_extra = [] + for test in tests: + url = test['url'] + del test['url'] + inputs.append(url) + inputs_extra.append(test) + + + return inputs, inputs_extra def get_nettests( @@ -443,6 +452,8 @@ def get_nettests( assert clickhouse is not None, "Clickhouse is required to compute the dynamic lists" inputs, inputs_extra = make_test_lists_from_targets_name(nt.targets_name, meta, clickhouse) + from pprint import pprint + pprint(inputs) nettests.append( OONIRunLinkNettest( targets_name=targets_name, diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py new file mode 100644 index 000000000..243eac55f --- /dev/null +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -0,0 +1,59 @@ +from copy import deepcopy +import json +from pathlib import Path +from oonirun.common.clickhouse_utils import insert_click +import pytest +from ..test_oonirun import SAMPLE_OONIRUN, SAMPLE_META + +def postj(client, url, **kw): + response = client.post(url, json=kw) + assert response.status_code == 200 + return response.json() + +@pytest.fixture +def url_priorities(clickhouse_db): + path = Path("tests/fixtures/data") + filename = "url_priorities_us.json" + file = Path(path, filename) + + with file.open("r") as f: + j = json.load(f) + + # 'sign' is created with default value 0, causing a db error. + # use 1 to prevent it + for row in j: + row["sign"] = 1 + + query = "INSERT INTO url_priorities (sign, category_code, cc, domain, url, priority) VALUES" + insert_click(clickhouse_db, query, j) + +def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing header parsing" + z['nettests'][0]['targets_name'] = 'websites_list_prioritized' + z['nettests'][0]['inputs'] = None + z['nettests'][0]['inputs_extra'] = None + z['nettests'] = z['nettests'][:1] + + # Create a link + j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) + orlid = j['oonirun_link_id'] + + j = { + "run_type" : "timed", + "is_charging" : True, + "probe_asn" : "AS1234", + "probe_cc" : "VE", + "network_type" : "wifi", + "website_category_codes" : [] + } + + r = client_with_user_role.post( + f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", + json=j + ) + assert r.status_code == 200, r.json() + j = r.json() + + urls = j["nettests"][0]["inputs"] + assert len(urls) > 1, urls \ No newline at end of file From d0062c27cf1476448a339a75b9072214a07a3940 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 30 May 2025 16:13:16 +0200 Subject: [PATCH 45/72] Remove useless prints --- .../oonirun/tests/integ/test_dynamic_lists.py | 11 +---------- ooniapi/services/oonirun/tests/test_oonirun.py | 5 ----- 2 files changed, 1 insertion(+), 15 deletions(-) diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index 243eac55f..b566b9526 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -39,18 +39,9 @@ def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) orlid = j['oonirun_link_id'] - j = { - "run_type" : "timed", - "is_charging" : True, - "probe_asn" : "AS1234", - "probe_cc" : "VE", - "network_type" : "wifi", - "website_category_codes" : [] - } - r = client_with_user_role.post( f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", - json=j + json=SAMPLE_META ) assert r.status_code == 200, r.json() j = r.json() diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index d2da3f4c6..b76e1b3f8 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -582,15 +582,10 @@ def test_oonirun_revisions(client, client_with_user_role): assert r.status_code == 200, r.json() j_latest = r.json() assert j_latest["revision"] == "3", "revision is 3" - from pprint import pprint # The engine-descriptor returns a list along with targets name on reading lastest_nettests[2]['inputs'] = [] lastest_nettests[2]['inputs_extra'] = [] - print("Given:") - pprint(j_latest['nettests']) - print("Expected:") - pprint(lastest_nettests) assert j_latest["nettests"] == lastest_nettests, "nettests are the same" assert j_latest["date_created"] == latest_date_created, "date created matches" From 673c907b7a262a48b9ea9bfabb41724d190b1359 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 30 May 2025 16:32:00 +0200 Subject: [PATCH 46/72] Fix broken tests --- ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py | 4 +++- 1 file changed, 3 insertions(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index b566b9526..93d0713f8 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -10,7 +10,7 @@ def postj(client, url, **kw): assert response.status_code == 200 return response.json() -@pytest.fixture +@pytest.fixture(scope="module") def url_priorities(clickhouse_db): path = Path("tests/fixtures/data") filename = "url_priorities_us.json" @@ -26,6 +26,8 @@ def url_priorities(clickhouse_db): query = "INSERT INTO url_priorities (sign, category_code, cc, domain, url, priority) VALUES" insert_click(clickhouse_db, query, j) + yield + clickhouse_db.execute("TRUNCATE TABLE url_priorities") def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): z = deepcopy(SAMPLE_OONIRUN) From 1a1f2039bcdb18440e24e904e589cbead5c2ab56 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Fri, 30 May 2025 16:32:47 +0200 Subject: [PATCH 47/72] Remove print --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index cf12fe403..46ef5f11b 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -452,8 +452,6 @@ def get_nettests( assert clickhouse is not None, "Clickhouse is required to compute the dynamic lists" inputs, inputs_extra = make_test_lists_from_targets_name(nt.targets_name, meta, clickhouse) - from pprint import pprint - pprint(inputs) nettests.append( OONIRunLinkNettest( targets_name=targets_name, From 083e2861f98c598b72ed633145a08c5d86dd151b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Mon, 2 Jun 2025 10:50:47 +0200 Subject: [PATCH 48/72] Add test for filtering with category codes --- .../oonirun/tests/integ/test_dynamic_lists.py | 32 +++++++++++++++++-- 1 file changed, 30 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index 93d0713f8..15d492630 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -29,6 +29,7 @@ def url_priorities(clickhouse_db): yield clickhouse_db.execute("TRUNCATE TABLE url_priorities") + def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): z = deepcopy(SAMPLE_OONIRUN) z['name'] = "Testing header parsing" @@ -41,7 +42,8 @@ def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) orlid = j['oonirun_link_id'] - r = client_with_user_role.post( + # Get link + r = client.post( f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", json=SAMPLE_META ) @@ -49,4 +51,30 @@ def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): j = r.json() urls = j["nettests"][0]["inputs"] - assert len(urls) > 1, urls \ No newline at end of file + assert len(urls) > 1, urls + +def test_check_in_url_category_news(client, client_with_user_role): + """ + Test that you can filter by category codes + """ + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing header parsing" + z['nettests'][0]['targets_name'] = 'websites_list_prioritized' + z['nettests'][0]['inputs'] = None + z['nettests'][0]['inputs_extra'] = None + z['nettests'] = z['nettests'][:1] + + # Create a link + j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) + orlid = j['oonirun_link_id'] + + # fetch the link + meta = deepcopy(SAMPLE_META) + meta['website_category_codes'] = ["NEWS"] + j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + inputs = j["nettests"][0]["inputs"] + inputs_extra = j['nettests'][0]["inputs_extra"] + assert len(inputs), inputs + assert len(inputs) == len(inputs_extra) + for extra in inputs_extra: + assert extra["category_code"] == "NEWS" From b9ec6318df9a2d4f2973648417f93fe3398a5739 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Mon, 2 Jun 2025 16:00:28 +0200 Subject: [PATCH 49/72] Add setup for creating measurements for testing url priorization --- .../tests/fixtures/data/measurements.json | 38 +++++++++++++++++ .../oonirun/tests/integ/test_dynamic_lists.py | 41 +++++++++++++++++-- 2 files changed, 75 insertions(+), 4 deletions(-) create mode 100644 ooniapi/services/oonirun/tests/fixtures/data/measurements.json diff --git a/ooniapi/services/oonirun/tests/fixtures/data/measurements.json b/ooniapi/services/oonirun/tests/fixtures/data/measurements.json new file mode 100644 index 000000000..cb41d3e8b --- /dev/null +++ b/ooniapi/services/oonirun/tests/fixtures/data/measurements.json @@ -0,0 +1,38 @@ +[ + { + "measurement_uid": "20240815000001.959692_TH_webconnectivity_29f8e1b5b07606c7", + "report_id": "20240814T220910Z_webconnectivity_TH_45629_n1_FWtmywdrVDFlEAfy", + "input": "http://twitter.com/", + "probe_cc": "ES", + "probe_asn": 45629, + "test_name": "web_connectivity", + "test_start_time": "2024-08-14 22:09:10", + "measurement_start_time": "2024-08-15 00:00:01", + "filename": "", + "scores": "{\"blocking_general\": 0.0,\"blocking_global\": 0.0,\"blocking_country\": 0.0,\"blocking_isp\": 0.0,\"blocking_local\": 0.0}", + "platform": "linux", + "anomaly": "f", + "confirmed": "f", + "msm_failure": "f", + "domain": "twitter.com", + "software_name": "ooniprobe-cli", + "software_version": "3.20.0", + "control_failure": "", + "blocking_general": 0, + "is_ssl_expected": 0, + "page_len": 0, + "page_len_ratio": 0, + "server_cc": "", + "server_asn": 0, + "server_as_name": "", + "test_version": "0.4.3", + "test_runtime": 7.9002676, + "architecture": "amd64", + "engine_name": "ooniprobe-engine", + "engine_version": "3.20.0", + "blocking_type": "", + "test_helper_address": "https://3.th.ooni.org", + "test_helper_type": "https", + "ooni_run_link_id": null + } +] \ No newline at end of file diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index 15d492630..6c7ac85f0 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -4,6 +4,8 @@ from oonirun.common.clickhouse_utils import insert_click import pytest from ..test_oonirun import SAMPLE_OONIRUN, SAMPLE_META +from datetime import datetime, timedelta, UTC +import random def postj(client, url, **kw): response = client.post(url, json=kw) @@ -11,10 +13,13 @@ def postj(client, url, **kw): return response.json() @pytest.fixture(scope="module") -def url_priorities(clickhouse_db): - path = Path("tests/fixtures/data") +def fixtures_data_dir(): + yield Path("tests/fixtures/data") + +@pytest.fixture(scope="module") +def url_priorities(clickhouse_db, fixtures_data_dir): filename = "url_priorities_us.json" - file = Path(path, filename) + file = Path(fixtures_data_dir, filename) with file.open("r") as f: j = json.load(f) @@ -29,6 +34,29 @@ def url_priorities(clickhouse_db): yield clickhouse_db.execute("TRUNCATE TABLE url_priorities") +def generate_random_date_last_7_days() -> datetime: + start = datetime.now(tz=UTC) + timedelta(days=7) + + # return a random date between 7 days ago and now + return start + timedelta(seconds=random.randrange(0, 3600 * 24 * 7)) + +@pytest.fixture(scope="module") +def measurements(clickhouse_db, fixtures_data_dir): + msmnts_dir = Path(fixtures_data_dir, "measurements.json") + with open(msmnts_dir, "r") as f: + measurements = json.load(f) + + for ms in measurements: + date = generate_random_date_last_7_days() + ms['measurement_start_time'] = date + ms['test_start_time'] = date + + query = "INSERT INTO fastpath VALUES" + insert_click(clickhouse_db, query, measurements) + + yield + clickhouse_db.execute("TRUNCATE TABLE url_priorities") + def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): z = deepcopy(SAMPLE_OONIRUN) @@ -53,7 +81,7 @@ def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): urls = j["nettests"][0]["inputs"] assert len(urls) > 1, urls -def test_check_in_url_category_news(client, client_with_user_role): +def test_check_in_url_category_news(client, client_with_user_role, url_priorities): """ Test that you can filter by category codes """ @@ -78,3 +106,8 @@ def test_check_in_url_category_news(client, client_with_user_role): assert len(inputs) == len(inputs_extra) for extra in inputs_extra: assert extra["category_code"] == "NEWS" + +def test_priorization_with_measurements(client, client_with_user_role, url_priorities, measurements): + """ + Test priorization including measurements + """ \ No newline at end of file From ec10d06491bdf151f2cb7448ec9338f532d51a9c Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 3 Jun 2025 11:44:15 +0200 Subject: [PATCH 50/72] Add test for prioritization with measurements --- .../tests/fixtures/data/measurements.json | 148 +++++++++++++++++- .../fixtures/data/url_priorities_us.json | 2 +- .../oonirun/tests/integ/test_dynamic_lists.py | 53 ++++++- 3 files changed, 193 insertions(+), 10 deletions(-) diff --git a/ooniapi/services/oonirun/tests/fixtures/data/measurements.json b/ooniapi/services/oonirun/tests/fixtures/data/measurements.json index cb41d3e8b..32b405acb 100644 --- a/ooniapi/services/oonirun/tests/fixtures/data/measurements.json +++ b/ooniapi/services/oonirun/tests/fixtures/data/measurements.json @@ -2,9 +2,9 @@ { "measurement_uid": "20240815000001.959692_TH_webconnectivity_29f8e1b5b07606c7", "report_id": "20240814T220910Z_webconnectivity_TH_45629_n1_FWtmywdrVDFlEAfy", - "input": "http://twitter.com/", + "input": "https://twitter.com/", "probe_cc": "ES", - "probe_asn": 45629, + "probe_asn": 1234, "test_name": "web_connectivity", "test_start_time": "2024-08-14 22:09:10", "measurement_start_time": "2024-08-15 00:00:01", @@ -34,5 +34,149 @@ "test_helper_address": "https://3.th.ooni.org", "test_helper_type": "https", "ooni_run_link_id": null + }, + { + "measurement_uid": "20240815000001.959692_TH_webconnectivity_29f8e1b5b07606c7", + "report_id": "20240814T220910Z_webconnectivity_TH_45629_n1_FWtmywdrVDFlEAfy", + "input": "https://twitter.com/", + "probe_cc": "ES", + "probe_asn": 1234, + "test_name": "web_connectivity", + "test_start_time": "2024-08-14 22:09:10", + "measurement_start_time": "2024-08-15 00:00:01", + "filename": "", + "scores": "{\"blocking_general\": 0.0,\"blocking_global\": 0.0,\"blocking_country\": 0.0,\"blocking_isp\": 0.0,\"blocking_local\": 0.0}", + "platform": "linux", + "anomaly": "f", + "confirmed": "f", + "msm_failure": "f", + "domain": "twitter.com", + "software_name": "ooniprobe-cli", + "software_version": "3.20.0", + "control_failure": "", + "blocking_general": 0, + "is_ssl_expected": 0, + "page_len": 0, + "page_len_ratio": 0, + "server_cc": "", + "server_asn": 0, + "server_as_name": "", + "test_version": "0.4.3", + "test_runtime": 7.9002676, + "architecture": "amd64", + "engine_name": "ooniprobe-engine", + "engine_version": "3.20.0", + "blocking_type": "", + "test_helper_address": "https://3.th.ooni.org", + "test_helper_type": "https", + "ooni_run_link_id": null + }, + { + "measurement_uid": "20240815000001.959692_TH_webconnectivity_29f8e1b5b07606c7", + "report_id": "20240814T220910Z_webconnectivity_TH_45629_n1_FWtmywdrVDFlEAfy", + "input": "https://twitter.com/", + "probe_cc": "ES", + "probe_asn": 1234, + "test_name": "web_connectivity", + "test_start_time": "2024-08-14 22:09:10", + "measurement_start_time": "2024-08-15 00:00:01", + "filename": "", + "scores": "{\"blocking_general\": 0.0,\"blocking_global\": 0.0,\"blocking_country\": 0.0,\"blocking_isp\": 0.0,\"blocking_local\": 0.0}", + "platform": "linux", + "anomaly": "f", + "confirmed": "f", + "msm_failure": "f", + "domain": "twitter.com", + "software_name": "ooniprobe-cli", + "software_version": "3.20.0", + "control_failure": "", + "blocking_general": 0, + "is_ssl_expected": 0, + "page_len": 0, + "page_len_ratio": 0, + "server_cc": "", + "server_asn": 0, + "server_as_name": "", + "test_version": "0.4.3", + "test_runtime": 7.9002676, + "architecture": "amd64", + "engine_name": "ooniprobe-engine", + "engine_version": "3.20.0", + "blocking_type": "", + "test_helper_address": "https://3.th.ooni.org", + "test_helper_type": "https", + "ooni_run_link_id": null + }, + { + "measurement_uid": "20240815000001.959692_TH_webconnectivity_29f8e1b5b07606c7", + "report_id": "20240814T220910Z_webconnectivity_TH_45629_n1_FWtmywdrVDFlEAfy", + "input": "https://www.facebook.com/", + "probe_cc": "IT", + "probe_asn": 1234, + "test_name": "web_connectivity", + "test_start_time": "2024-08-14 22:09:10", + "measurement_start_time": "2024-08-15 00:00:01", + "filename": "", + "scores": "{\"blocking_general\": 0.0,\"blocking_global\": 0.0,\"blocking_country\": 0.0,\"blocking_isp\": 0.0,\"blocking_local\": 0.0}", + "platform": "linux", + "anomaly": "f", + "confirmed": "f", + "msm_failure": "f", + "domain": "www.facebook.com", + "software_name": "ooniprobe-cli", + "software_version": "3.20.0", + "control_failure": "", + "blocking_general": 0, + "is_ssl_expected": 0, + "page_len": 0, + "page_len_ratio": 0, + "server_cc": "", + "server_asn": 0, + "server_as_name": "", + "test_version": "0.4.3", + "test_runtime": 7.9002676, + "architecture": "amd64", + "engine_name": "ooniprobe-engine", + "engine_version": "3.20.0", + "blocking_type": "", + "test_helper_address": "https://3.th.ooni.org", + "test_helper_type": "https", + "ooni_run_link_id": null + }, + { + "measurement_uid": "20240815000001.959692_TH_webconnectivity_29f8e1b5b07606c7", + "report_id": "20240814T220910Z_webconnectivity_TH_45629_n1_FWtmywdrVDFlEAfy", + "input": "https://www.facebook.com/", + "probe_cc": "IT", + "probe_asn": 1234, + "test_name": "web_connectivity", + "test_start_time": "2024-08-14 22:09:10", + "measurement_start_time": "2024-08-15 00:00:01", + "filename": "", + "scores": "{\"blocking_general\": 0.0,\"blocking_global\": 0.0,\"blocking_country\": 0.0,\"blocking_isp\": 0.0,\"blocking_local\": 0.0}", + "platform": "linux", + "anomaly": "f", + "confirmed": "f", + "msm_failure": "f", + "domain": "www.facebook.com", + "software_name": "ooniprobe-cli", + "software_version": "3.20.0", + "control_failure": "", + "blocking_general": 0, + "is_ssl_expected": 0, + "page_len": 0, + "page_len_ratio": 0, + "server_cc": "", + "server_asn": 0, + "server_as_name": "", + "test_version": "0.4.3", + "test_runtime": 7.9002676, + "architecture": "amd64", + "engine_name": "ooniprobe-engine", + "engine_version": "3.20.0", + "blocking_type": "", + "test_helper_address": "https://3.th.ooni.org", + "test_helper_type": "https", + "ooni_run_link_id": null } ] \ No newline at end of file diff --git a/ooniapi/services/oonirun/tests/fixtures/data/url_priorities_us.json b/ooniapi/services/oonirun/tests/fixtures/data/url_priorities_us.json index 2171415e2..3259dffa7 100644 --- a/ooniapi/services/oonirun/tests/fixtures/data/url_priorities_us.json +++ b/ooniapi/services/oonirun/tests/fixtures/data/url_priorities_us.json @@ -227,7 +227,7 @@ "category_code": "*", "cc": "*", "domain": "twitter.com", - "priority": 200, + "priority": 201, "url": "*" }, { diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index 6c7ac85f0..2332b7f62 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -1,7 +1,7 @@ from copy import deepcopy import json from pathlib import Path -from oonirun.common.clickhouse_utils import insert_click +from oonirun.common.clickhouse_utils import insert_click, query_click import pytest from ..test_oonirun import SAMPLE_OONIRUN, SAMPLE_META from datetime import datetime, timedelta, UTC @@ -35,10 +35,10 @@ def url_priorities(clickhouse_db, fixtures_data_dir): clickhouse_db.execute("TRUNCATE TABLE url_priorities") def generate_random_date_last_7_days() -> datetime: - start = datetime.now(tz=UTC) + timedelta(days=7) + start = datetime.now(tz=UTC) - timedelta(days=7) # return a random date between 7 days ago and now - return start + timedelta(seconds=random.randrange(0, 3600 * 24 * 7)) + return start + timedelta(seconds=random.randrange(3600 * 24, 3600 * 24 * 7 - 3600 * 24)) @pytest.fixture(scope="module") def measurements(clickhouse_db, fixtures_data_dir): @@ -60,7 +60,7 @@ def measurements(clickhouse_db, fixtures_data_dir): def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing header parsing" + z['name'] = "Testing simple prioritizationpri" z['nettests'][0]['targets_name'] = 'websites_list_prioritized' z['nettests'][0]['inputs'] = None z['nettests'][0]['inputs_extra'] = None @@ -86,7 +86,7 @@ def test_check_in_url_category_news(client, client_with_user_role, url_prioritie Test that you can filter by category codes """ z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing header parsing" + z['name'] = "Categories filtering" z['nettests'][0]['targets_name'] = 'websites_list_prioritized' z['nettests'][0]['inputs'] = None z['nettests'][0]['inputs_extra'] = None @@ -107,7 +107,46 @@ def test_check_in_url_category_news(client, client_with_user_role, url_prioritie for extra in inputs_extra: assert extra["category_code"] == "NEWS" -def test_priorization_with_measurements(client, client_with_user_role, url_priorities, measurements): +def test_prioritization_with_measurements(client, client_with_user_role, url_priorities, measurements): """ Test priorization including measurements - """ \ No newline at end of file + """ + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing header parsing" + z['nettests'][0]['targets_name'] = 'websites_list_prioritized' + z['nettests'][0]['inputs'] = None + z['nettests'][0]['inputs_extra'] = None + z['nettests'] = z['nettests'][:1] + + # Create a link + j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) + orlid = j['oonirun_link_id'] + + + # fetch the link + meta = deepcopy(SAMPLE_META) + # In ES we have more measurements for twitter, (see tests/fixtures/data/measurements.json) + # so twitter should NOT show up first + meta['probe_cc'] = 'ES' + j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + inputs = j["nettests"][0]["inputs"] + assert len(inputs), inputs + assert "twitter.com" not in inputs[0], "Twitter should not be the first one" + + # Twitter with a different asn can be first + meta['probe_cc'] = 'ES' + meta['probe_asn'] = 'AS9999' + j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + inputs = j["nettests"][0]["inputs"] + assert len(inputs), inputs + assert "twitter.com" in inputs[0], "Twitter should be the first one" + + + # Similarly, in IT twitter should be first, and facebook last + meta['probe_cc'] = 'IT' + meta['probe_asn'] = 'AS1234' + j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + inputs = j["nettests"][0]["inputs"] + assert len(inputs), inputs + assert "twitter.com" in inputs[0], "Twitter should be the first one" + assert "facebook.com" in inputs[-1], "Facebook should be the last one" From e2b22801aff7db18dfabee3e1c2dd90cb1c6cc72 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 3 Jun 2025 12:27:03 +0200 Subject: [PATCH 51/72] Add url basic url priorities tests --- .../tests/fixtures/initdb/02-fixtures.sql | 1 + .../oonirun/tests/integ/test_dynamic_lists.py | 40 ++++++++++++++++++- 2 files changed, 40 insertions(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/tests/fixtures/initdb/02-fixtures.sql b/ooniapi/services/oonirun/tests/fixtures/initdb/02-fixtures.sql index 0adf8e8db..daaab63ef 100644 --- a/ooniapi/services/oonirun/tests/fixtures/initdb/02-fixtures.sql +++ b/ooniapi/services/oonirun/tests/fixtures/initdb/02-fixtures.sql @@ -17,6 +17,7 @@ INSERT INTO citizenlab VALUES ('facebook.com','https://facebook.com/watch','jo', INSERT INTO citizenlab VALUES ('twitter.com','http://twitter.com/ghonim','kw','POLR'); INSERT INTO citizenlab VALUES ('twitter.com','http://twitter.com/ghonim','so','POLR'); INSERT INTO citizenlab VALUES ('twitter.com','https://twitter.com/','ZZ','GRP'); +INSERT INTO citizenlab VALUES ('ooni.org','https://ooni.org/','ZZ','GRP'); -- get_measurement_meta integ tests INSERT INTO jsonl (report_id, input, s3path, linenum) VALUES ('20210709T004340Z_webconnectivity_MY_4818_n1_YCM7J9mGcEHds2K3', 'https://www.backtrack-linux.org/', 'raw/20210709/00/MY/webconnectivity/2021070900_MY_webconnectivity.n0.2.jsonl.gz', 35) diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index 2332b7f62..0b9658db7 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -1,7 +1,7 @@ from copy import deepcopy import json from pathlib import Path -from oonirun.common.clickhouse_utils import insert_click, query_click +from oonirun.common.clickhouse_utils import insert_click import pytest from ..test_oonirun import SAMPLE_OONIRUN, SAMPLE_META from datetime import datetime, timedelta, UTC @@ -150,3 +150,41 @@ def test_prioritization_with_measurements(client, client_with_user_role, url_pri assert len(inputs), inputs assert "twitter.com" in inputs[0], "Twitter should be the first one" assert "facebook.com" in inputs[-1], "Facebook should be the last one" + +@pytest.fixture +def super_prioritized_website(clickhouse_db): + values = { + "category_code": "*", + "cc": "*", + "domain": "ooni.org", + "priority": 99999, + "url": "*", + "sign" : 1 + } + query = "INSERT INTO url_priorities (sign, category_code, cc, domain, url, priority) VALUES" + insert_click(clickhouse_db, query, [values]) + yield + clickhouse_db.execute("DELETE FROM url_priorities WHERE domain='www.ooni.com'") + + + +def test_priorities_basic(client, client_with_user_role, measurements, url_priorities, super_prioritized_website): + z = deepcopy(SAMPLE_OONIRUN) + z['name'] = "Testing header parsing" + z['nettests'][0]['targets_name'] = 'websites_list_prioritized' + z['nettests'][0]['inputs'] = None + z['nettests'][0]['inputs_extra'] = None + z['nettests'] = z['nettests'][:1] + + # Create a link + j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) + orlid = j['oonirun_link_id'] + + meta = deepcopy(SAMPLE_META) + meta['probe_cc'] = 'ES' + j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + inputs = j["nettests"][0]["inputs"] + assert len(inputs), inputs + from pprint import pprint + pprint(inputs) + assert "ooni.org" in inputs[0], "Ooni should be the first one" \ No newline at end of file From 87690924aa4a89c9f894ca5da6653ece11dfe4c4 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 3 Jun 2025 12:32:32 +0200 Subject: [PATCH 52/72] Remove useless print --- ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py | 2 -- 1 file changed, 2 deletions(-) diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index 0b9658db7..e03166a89 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -185,6 +185,4 @@ def test_priorities_basic(client, client_with_user_role, measurements, url_prior j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) inputs = j["nettests"][0]["inputs"] assert len(inputs), inputs - from pprint import pprint - pprint(inputs) assert "ooni.org" in inputs[0], "Ooni should be the first one" \ No newline at end of file From b507c99d51aa2ef7e25da1f30bdedc3d9b946a45 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 3 Jun 2025 13:18:02 +0200 Subject: [PATCH 53/72] Simplify header format --- .../oonirun/src/oonirun/routers/v2.py | 26 +++---------------- .../services/oonirun/tests/test_oonirun.py | 4 +-- 2 files changed, 5 insertions(+), 25 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 46ef5f11b..8539167a1 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -544,27 +544,7 @@ def get_oonirun_link_revisions( revisions.append(str(r)) return OONIRunLinkRevisions(revisions=revisions) -class XOoniNetworkInfo(BaseModel): - probe_asn : str - probe_cc : str - network_type : str - - @staticmethod - def get_header_pattern() -> str: - return r'^([a-zA-Z0-9]+),([a-zA-Z0-9]+) \(([a-zA-Z0-9]+)\)$' - - @classmethod - def from_header(cls, header : str) -> Self: - pattern = cls.get_header_pattern() - matched = re.match(pattern, header) - - assert matched is not None, "Expected format: , (), eg AS1234,IT (wifi)" - - probe_asn, probe_cc, network_type = matched.groups() - return cls(probe_asn=probe_asn, probe_cc=probe_cc, network_type=network_type) - - -USER_AGENT_PATTERN = r"^([a-zA-Z0-9\-\_]+)/([a-zA-Z0-9\-\_\.]+) \(([a-zA-Z0-9\ ]+)\) ([a-zA-Z0-9\-\_]+)/([a-zA-Z0-9\-\_\.]+) \(([a-zA-Z0-9\-\_\.]+)\)$" +USER_AGENT_PATTERN = r"^([a-zA-Z0-9\-\_]+),([a-zA-Z0-9\-\_\.]+),([a-zA-Z0-9\ ]+),([a-zA-Z0-9\-\_]+),([a-zA-Z0-9\-\_\.]+),([a-zA-Z0-9\-\_\.]+)$" @router.post( "/v2/oonirun/links/{oonirun_link_id}/engine-descriptor/{revision_number}", tags=["oonirun"], @@ -588,8 +568,8 @@ def get_oonirun_link_engine_descriptor( Optional[str], # TODO Marked as optional to avoid breaking old probes Header( pattern=USER_AGENT_PATTERN, - error_message = "Expected format: / () / ()", - description="Expected format: / () / ()" + error_message = "Expected format: ,,,,,", + description="Expected format: ,,,,," ), ] = None, credentials : Annotated[Optional[bytes], Header(description="base64 encoded OONI anonymous credentials")] = None, diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index b76e1b3f8..ef4c5cdb3 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -825,7 +825,7 @@ def test_x_user_agent_header_parsing(client_with_user_role, client): # Test with good headers headers = { - "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2 (ooniprobe-engine_1.2.3)" + "UserAgent" : "ooniprobe-android-unattended,3.8.2,android,ooniprobe-engine,3.17.2,ooniprobe-engine_1.2.3" } r = client_with_user_role.post(f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", json=SAMPLE_META) @@ -846,7 +846,7 @@ def test_x_user_agent_header_parsing(client_with_user_role, client): # Bad header headers = { - "UserAgent" : "ooniprobe-android-unattended/3.8.2 (android) ooniprobe-engine/3.17.2" + "UserAgent" : "ooniprobe-android-unattended,3.8.2,android,ooniprobe-engine,3.17.2" } r = client.post( f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", From 248a89bb60c3e4bb823401acd387ec5d1d431449 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 3 Jun 2025 13:32:22 +0200 Subject: [PATCH 54/72] Simplify get_nettests function --- .../oonirun/src/oonirun/routers/v2.py | 21 ++++++++++++------- 1 file changed, 14 insertions(+), 7 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 8539167a1..dc01c3762 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -431,8 +431,7 @@ def make_nettest_websites_list_prioritized(meta : OonirunMeta, clickhouse : Clic def get_nettests( - oonirun_link: models.OONIRunLink, revision: Optional[int], meta : Optional[OonirunMeta] = None, clickhouse : Optional[Clickhouse] = None -) -> Tuple[List[OONIRunLinkNettest], datetime]: + oonirun_link: models.OONIRunLink, revision: Optional[int]) -> Tuple[List[OONIRunLinkNettest], datetime]: """Computes a list of nettests related to the given oonirun link The `meta` parameter is required for the dynamic tests list calculation. If not provided, @@ -448,9 +447,6 @@ def get_nettests( date_created = nt.date_created inputs, inputs_extra = nt.inputs, nt.inputs_extra targets_name = nt.targets_name - if nt.targets_name is not None and meta is not None: - assert clickhouse is not None, "Clickhouse is required to compute the dynamic lists" - inputs, inputs_extra = make_test_lists_from_targets_name(nt.targets_name, meta, clickhouse) nettests.append( OONIRunLinkNettest( @@ -465,6 +461,13 @@ def get_nettests( ) return nettests, date_created +def compute_dynamic_test_lists(clickhouse : Clickhouse, nettest : OONIRunLinkNettest, meta : OonirunMeta): + if nettest.targets_name is None: + return # No need of dynamic lists + + inputs, inputs_extra = make_test_lists_from_targets_name(nettest.targets_name, meta, clickhouse) + nettest.inputs = inputs + nettest.inputs_extra = inputs_extra def make_oonirun_link( db: Session, @@ -489,7 +492,7 @@ def make_oonirun_link( assert isinstance(revision, int) - nettests, date_created = get_nettests(res, revision, meta) + nettests, date_created = get_nettests(res, revision) return OONIRunLink( oonirun_link_id=res.oonirun_link_id, name=res.name, @@ -603,7 +606,11 @@ def get_oonirun_link_engine_descriptor( revision = latest_revision assert isinstance(revision, int) - nettests, date_created = get_nettests(res, revision, meta, clickhouse) + + nettests, date_created = get_nettests(res, revision) + for nt in nettests: + compute_dynamic_test_lists(clickhouse, nt, meta) + return OONIRunLinkEngineDescriptor( nettests=nettests, date_created=date_created, From f389bd66907a063c77093a872eb3285ec7bdb1b8 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 3 Jun 2025 13:33:47 +0200 Subject: [PATCH 55/72] Revert "Simplify get_nettests function" This reverts commit 248a89bb60c3e4bb823401acd387ec5d1d431449. --- .../oonirun/src/oonirun/routers/v2.py | 21 +++++++------------ 1 file changed, 7 insertions(+), 14 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index dc01c3762..8539167a1 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -431,7 +431,8 @@ def make_nettest_websites_list_prioritized(meta : OonirunMeta, clickhouse : Clic def get_nettests( - oonirun_link: models.OONIRunLink, revision: Optional[int]) -> Tuple[List[OONIRunLinkNettest], datetime]: + oonirun_link: models.OONIRunLink, revision: Optional[int], meta : Optional[OonirunMeta] = None, clickhouse : Optional[Clickhouse] = None +) -> Tuple[List[OONIRunLinkNettest], datetime]: """Computes a list of nettests related to the given oonirun link The `meta` parameter is required for the dynamic tests list calculation. If not provided, @@ -447,6 +448,9 @@ def get_nettests( date_created = nt.date_created inputs, inputs_extra = nt.inputs, nt.inputs_extra targets_name = nt.targets_name + if nt.targets_name is not None and meta is not None: + assert clickhouse is not None, "Clickhouse is required to compute the dynamic lists" + inputs, inputs_extra = make_test_lists_from_targets_name(nt.targets_name, meta, clickhouse) nettests.append( OONIRunLinkNettest( @@ -461,13 +465,6 @@ def get_nettests( ) return nettests, date_created -def compute_dynamic_test_lists(clickhouse : Clickhouse, nettest : OONIRunLinkNettest, meta : OonirunMeta): - if nettest.targets_name is None: - return # No need of dynamic lists - - inputs, inputs_extra = make_test_lists_from_targets_name(nettest.targets_name, meta, clickhouse) - nettest.inputs = inputs - nettest.inputs_extra = inputs_extra def make_oonirun_link( db: Session, @@ -492,7 +489,7 @@ def make_oonirun_link( assert isinstance(revision, int) - nettests, date_created = get_nettests(res, revision) + nettests, date_created = get_nettests(res, revision, meta) return OONIRunLink( oonirun_link_id=res.oonirun_link_id, name=res.name, @@ -606,11 +603,7 @@ def get_oonirun_link_engine_descriptor( revision = latest_revision assert isinstance(revision, int) - - nettests, date_created = get_nettests(res, revision) - for nt in nettests: - compute_dynamic_test_lists(clickhouse, nt, meta) - + nettests, date_created = get_nettests(res, revision, meta, clickhouse) return OONIRunLinkEngineDescriptor( nettests=nettests, date_created=date_created, From b5f4308051f23052777dea668462904e4ad14172 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 09:41:18 +0200 Subject: [PATCH 56/72] black reformat --- .../oonirun/src/oonirun/dependencies.py | 6 +- ooniapi/services/oonirun/src/oonirun/main.py | 12 +- .../oonirun/src/oonirun/routers/v2.py | 119 ++++++++----- ooniapi/services/oonirun/tests/conftest.py | 9 +- .../oonirun/tests/integ/test_dynamic_lists.py | 136 +++++++------- .../services/oonirun/tests/test_database.py | 13 +- .../services/oonirun/tests/test_oonirun.py | 167 +++++++++++------- 7 files changed, 275 insertions(+), 187 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/dependencies.py b/ooniapi/services/oonirun/src/oonirun/dependencies.py index 6ff998c0b..2ab98729c 100644 --- a/ooniapi/services/oonirun/src/oonirun/dependencies.py +++ b/ooniapi/services/oonirun/src/oonirun/dependencies.py @@ -13,6 +13,7 @@ DependsSettings = Annotated[Settings, Depends(get_settings)] + def get_postgresql_session(settings: DependsSettings): engine = create_engine(settings.postgresql_url) SessionLocal = sessionmaker(autocommit=False, autoflush=False, bind=engine) @@ -23,8 +24,10 @@ def get_postgresql_session(settings: DependsSettings): finally: db.close() + DependsPostgresSession = Annotated[Session, Depends(get_postgresql_session)] + def get_clickhouse_session(settings: DependsSettings): db = Clickhouse.from_url(settings.clickhouse_url) try: @@ -32,4 +35,5 @@ def get_clickhouse_session(settings: DependsSettings): finally: db.disconnect() -DependsClickhouseClient = Annotated[Clickhouse, Depends(get_clickhouse_session)] \ No newline at end of file + +DependsClickhouseClient = Annotated[Clickhouse, Depends(get_clickhouse_session)] diff --git a/ooniapi/services/oonirun/src/oonirun/main.py b/ooniapi/services/oonirun/src/oonirun/main.py index 5a431eeab..bddaa5737 100644 --- a/ooniapi/services/oonirun/src/oonirun/main.py +++ b/ooniapi/services/oonirun/src/oonirun/main.py @@ -11,7 +11,11 @@ from . import models from .routers import v2 -from .dependencies import DependsPostgresSession, DependsClickhouseClient, DependsSettings +from .dependencies import ( + DependsPostgresSession, + DependsClickhouseClient, + DependsSettings, +) from .common.dependencies import get_settings from .common.version import get_build_label, get_pkg_version from .common.version import get_build_label, get_pkg_version @@ -65,9 +69,9 @@ class HealthStatus(BaseModel): @app.get("/health") async def health( - settings : DependsSettings, - db : DependsPostgresSession, - clickhouse : DependsClickhouseClient, + settings: DependsSettings, + db: DependsPostgresSession, + clickhouse: DependsClickhouseClient, ): errors = [] diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 8539167a1..6958283da 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -37,17 +37,22 @@ def utcnow_seconds(): return datetime.now(timezone.utc).replace(microsecond=0) + class OonirunMeta(BaseModel): - run_type : str = Field(description="Run type", pattern="^(timed|manual)$") - is_charging : bool = Field(description="If the probe is charging") - probe_asn : str = Field(pattern=r"^([a-zA-Z0-9]+)$") - probe_cc : str = Field(description="Country code. Ex: VE") - network_type : str = Field(description="Ex: wifi") - website_category_codes : List[str] = Field(description="List of category codes that user has chosen to test (eg. NEWS,HUMR)", default=[]) + run_type: str = Field(description="Run type", pattern="^(timed|manual)$") + is_charging: bool = Field(description="If the probe is charging") + probe_asn: str = Field(pattern=r"^([a-zA-Z0-9]+)$") + probe_cc: str = Field(description="Country code. Ex: VE") + network_type: str = Field(description="Ex: wifi") + website_category_codes: List[str] = Field( + description="List of category codes that user has chosen to test (eg. NEWS,HUMR)", + default=[], + ) def probe_asn_int(self) -> int: return int(self.probe_asn.replace("AS", "")) + class OONIRunLinkNettest(BaseModel): test_name: str = Field( default="", title="name of the ooni nettest", min_length=2, max_length=100 @@ -78,7 +83,9 @@ class OONIRunLinkNettest(BaseModel): @model_validator(mode="after") def validate_inputs_extra(self) -> Self: - if self.inputs_extra is not None and (self.inputs is None or len(self.inputs) != len(self.inputs_extra)): + if self.inputs_extra is not None and ( + self.inputs is None or len(self.inputs) != len(self.inputs_extra) + ): raise ValueError( "When provided, inputs_extra should be the same length as inputs" ) @@ -88,11 +95,13 @@ def validate_no_inputs_and_targets_name(self): """ Check that you are not providing targets_name and inputs-inputs_extra in the same request """ - if self.targets_name is not None and (self.inputs is not None or self.inputs_extra is not None): + if self.targets_name is not None and ( + self.inputs is not None or self.inputs_extra is not None + ): raise ValueError( "When targets_name is provided, you can't provide inputs or inputs_extra" ) - + return self @@ -209,15 +218,12 @@ def create_oonirun_link( status_code=400, detail="email_address must match the email address of the user who created the oonirun link", ) - + for nt in create_request.nettests: - try: + try: nt.validate_no_inputs_and_targets_name() except ValueError as e: - raise HTTPException( - status_code = 422, - detail = {"error": str(e)} - ) + raise HTTPException(status_code=422, detail={"error": str(e)}) now = utcnow_seconds() @@ -295,13 +301,10 @@ def edit_oonirun_link( account_id = token["account_id"] for nt in edit_request.nettests: - try: + try: nt.validate_no_inputs_and_targets_name() except ValueError as e: - raise HTTPException( - status_code = 422, - detail = {"error": str(e)} - ) + raise HTTPException(status_code=422, detail={"error": str(e)}) now = utcnow_seconds() @@ -397,15 +400,20 @@ def edit_oonirun_link( is_mine=oonirun_link.creator_account_id == account_id, ) -def make_test_lists_from_targets_name(targets_name : str, meta : OonirunMeta, clickhouse : Clickhouse) -> Tuple[List[str], List[Dict[str, Any]]]: + +def make_test_lists_from_targets_name( + targets_name: str, meta: OonirunMeta, clickhouse: Clickhouse +) -> Tuple[List[str], List[Dict[str, Any]]]: if targets_name == "websites_list_prioritized": return make_nettest_websites_list_prioritized(meta, clickhouse) - + raise ValueError("Unknown target name: " + targets_name) -def make_nettest_websites_list_prioritized(meta : OonirunMeta, clickhouse : Clickhouse) -> Tuple[List[str], List[Dict[str, Any]]]: - """Generates an inputs list using prio. +def make_nettest_websites_list_prioritized( + meta: OonirunMeta, clickhouse: Clickhouse +) -> Tuple[List[str], List[Dict[str, Any]]]: + """Generates an inputs list using prio. Returns: Tuple[List[str], List[Dict[str, Any]]]: (Inputs, InputsExtra) """ @@ -416,27 +424,36 @@ def make_nettest_websites_list_prioritized(meta : OonirunMeta, clickhouse : Clic url_limit = 100 else: url_limit = 20 - tests, _1, _2 = generate_test_list(clickhouse, meta.probe_cc, meta.website_category_codes, meta.probe_asn_int(), url_limit, False) + tests, _1, _2 = generate_test_list( + clickhouse, + meta.probe_cc, + meta.website_category_codes, + meta.probe_asn_int(), + url_limit, + False, + ) inputs = [] inputs_extra = [] for test in tests: - url = test['url'] - del test['url'] + url = test["url"] + del test["url"] inputs.append(url) inputs_extra.append(test) - return inputs, inputs_extra def get_nettests( - oonirun_link: models.OONIRunLink, revision: Optional[int], meta : Optional[OonirunMeta] = None, clickhouse : Optional[Clickhouse] = None + oonirun_link: models.OONIRunLink, + revision: Optional[int], + meta: Optional[OonirunMeta] = None, + clickhouse: Optional[Clickhouse] = None, ) -> Tuple[List[OONIRunLinkNettest], datetime]: """Computes a list of nettests related to the given oonirun link - The `meta` parameter is required for the dynamic tests list calculation. If not provided, - it will skip it. + The `meta` parameter is required for the dynamic tests list calculation. If not provided, + it will skip it. """ @@ -448,9 +465,13 @@ def get_nettests( date_created = nt.date_created inputs, inputs_extra = nt.inputs, nt.inputs_extra targets_name = nt.targets_name - if nt.targets_name is not None and meta is not None: - assert clickhouse is not None, "Clickhouse is required to compute the dynamic lists" - inputs, inputs_extra = make_test_lists_from_targets_name(nt.targets_name, meta, clickhouse) + if nt.targets_name is not None and meta is not None: + assert ( + clickhouse is not None + ), "Clickhouse is required to compute the dynamic lists" + inputs, inputs_extra = make_test_lists_from_targets_name( + nt.targets_name, meta, clickhouse + ) nettests.append( OONIRunLinkNettest( @@ -470,7 +491,7 @@ def make_oonirun_link( db: Session, oonirun_link_id: str, account_id: Optional[str], - meta : Optional[OonirunMeta] = None, + meta: Optional[OonirunMeta] = None, revision: Optional[int] = None, ): q = db.query(models.OONIRunLink).filter( @@ -544,7 +565,10 @@ def get_oonirun_link_revisions( revisions.append(str(r)) return OONIRunLinkRevisions(revisions=revisions) + USER_AGENT_PATTERN = r"^([a-zA-Z0-9\-\_]+),([a-zA-Z0-9\-\_\.]+),([a-zA-Z0-9\ ]+),([a-zA-Z0-9\-\_]+),([a-zA-Z0-9\-\_\.]+),([a-zA-Z0-9\-\_\.]+)$" + + @router.post( "/v2/oonirun/links/{oonirun_link_id}/engine-descriptor/{revision_number}", tags=["oonirun"], @@ -562,18 +586,20 @@ def get_oonirun_link_engine_descriptor( ), ], db: DependsPostgresSession, - clickhouse: DependsClickhouseClient, - meta : OonirunMeta, + clickhouse: DependsClickhouseClient, + meta: OonirunMeta, useragent: Annotated[ Optional[str], # TODO Marked as optional to avoid breaking old probes Header( pattern=USER_AGENT_PATTERN, - error_message = "Expected format: ,,,,,", - description="Expected format: ,,,,," + error_message="Expected format: ,,,,,", + description="Expected format: ,,,,,", ), ] = None, - credentials : Annotated[Optional[bytes], Header(description="base64 encoded OONI anonymous credentials")] = None, - ): + credentials: Annotated[ + Optional[bytes], Header(description="base64 encoded OONI anonymous credentials") + ] = None, +): """Fetch an OONI Run link by specifying the revision number""" try: revision = int(revision_number) @@ -581,12 +607,19 @@ def get_oonirun_link_engine_descriptor( # We can assert it, since we are doing validation assert revision_number == "latest" revision = None - + if useragent is not None: result = re.match(USER_AGENT_PATTERN, useragent) # Validated by fastapi assert result is not None - software_name, software_version, platform, engine_name, engine_version, engine_version_full = result.groups() + ( + software_name, + software_version, + platform, + engine_name, + engine_version, + engine_version_full, + ) = result.groups() # TODO Log this metadata q = db.query(models.OONIRunLink).filter( diff --git a/ooniapi/services/oonirun/tests/conftest.py b/ooniapi/services/oonirun/tests/conftest.py index a534bc61b..98a172ea5 100644 --- a/ooniapi/services/oonirun/tests/conftest.py +++ b/ooniapi/services/oonirun/tests/conftest.py @@ -42,7 +42,7 @@ def alembic_migration(postgresql): def client_with_bad_settings(): app.dependency_overrides[get_settings] = make_override_get_settings( postgresql_url="postgresql://bad:bad@localhost/bad", - clickhouse_url="clickhouse://bad:bad@localhost/bad" + clickhouse_url="clickhouse://bad:bad@localhost/bad", ) client = TestClient(app) @@ -55,7 +55,7 @@ def client(alembic_migration, clickhouse_server): postgresql_url=alembic_migration, jwt_encryption_key="super_secure", prometheus_metrics_password="super_secure", - clickhouse_url = clickhouse_server + clickhouse_url=clickhouse_server, ) client = TestClient(app) @@ -96,6 +96,7 @@ def client_with_admin_role(client): client.headers = {"Authorization": f"Bearer {jwt_token}"} yield client + def is_clickhouse_running(url): try: with ClickhouseClient.from_url(url) as client: @@ -104,6 +105,7 @@ def is_clickhouse_running(url): except Exception: return False + @pytest.fixture(scope="session") def clickhouse_server(docker_ip, docker_services): port = docker_services.port_for("clickhouse", 9000) @@ -114,6 +116,7 @@ def clickhouse_server(docker_ip, docker_services): ) yield url + @pytest.fixture(scope="session") def clickhouse_db(clickhouse_server): - yield ClickhouseClient.from_url(clickhouse_server) \ No newline at end of file + yield ClickhouseClient.from_url(clickhouse_server) diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index e03166a89..190b67853 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -1,21 +1,24 @@ from copy import deepcopy import json from pathlib import Path -from oonirun.common.clickhouse_utils import insert_click +from oonirun.common.clickhouse_utils import insert_click import pytest from ..test_oonirun import SAMPLE_OONIRUN, SAMPLE_META from datetime import datetime, timedelta, UTC import random + def postj(client, url, **kw): response = client.post(url, json=kw) assert response.status_code == 200 return response.json() + @pytest.fixture(scope="module") def fixtures_data_dir(): yield Path("tests/fixtures/data") + @pytest.fixture(scope="module") def url_priorities(clickhouse_db, fixtures_data_dir): filename = "url_priorities_us.json" @@ -34,46 +37,49 @@ def url_priorities(clickhouse_db, fixtures_data_dir): yield clickhouse_db.execute("TRUNCATE TABLE url_priorities") + def generate_random_date_last_7_days() -> datetime: start = datetime.now(tz=UTC) - timedelta(days=7) # return a random date between 7 days ago and now - return start + timedelta(seconds=random.randrange(3600 * 24, 3600 * 24 * 7 - 3600 * 24)) + return start + timedelta( + seconds=random.randrange(3600 * 24, 3600 * 24 * 7 - 3600 * 24) + ) + @pytest.fixture(scope="module") def measurements(clickhouse_db, fixtures_data_dir): msmnts_dir = Path(fixtures_data_dir, "measurements.json") - with open(msmnts_dir, "r") as f: + with open(msmnts_dir, "r") as f: measurements = json.load(f) - + for ms in measurements: date = generate_random_date_last_7_days() - ms['measurement_start_time'] = date - ms['test_start_time'] = date - + ms["measurement_start_time"] = date + ms["test_start_time"] = date + query = "INSERT INTO fastpath VALUES" insert_click(clickhouse_db, query, measurements) - yield + yield clickhouse_db.execute("TRUNCATE TABLE url_priorities") def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing simple prioritizationpri" - z['nettests'][0]['targets_name'] = 'websites_list_prioritized' - z['nettests'][0]['inputs'] = None - z['nettests'][0]['inputs_extra'] = None - z['nettests'] = z['nettests'][:1] + z["name"] = "Testing simple prioritizationpri" + z["nettests"][0]["targets_name"] = "websites_list_prioritized" + z["nettests"][0]["inputs"] = None + z["nettests"][0]["inputs_extra"] = None + z["nettests"] = z["nettests"][:1] # Create a link j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) - orlid = j['oonirun_link_id'] + orlid = j["oonirun_link_id"] # Get link r = client.post( - f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", - json=SAMPLE_META + f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", json=SAMPLE_META ) assert r.status_code == 200, r.json() j = r.json() @@ -81,108 +87,116 @@ def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): urls = j["nettests"][0]["inputs"] assert len(urls) > 1, urls + def test_check_in_url_category_news(client, client_with_user_role, url_priorities): """ Test that you can filter by category codes """ z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Categories filtering" - z['nettests'][0]['targets_name'] = 'websites_list_prioritized' - z['nettests'][0]['inputs'] = None - z['nettests'][0]['inputs_extra'] = None - z['nettests'] = z['nettests'][:1] + z["name"] = "Categories filtering" + z["nettests"][0]["targets_name"] = "websites_list_prioritized" + z["nettests"][0]["inputs"] = None + z["nettests"][0]["inputs_extra"] = None + z["nettests"] = z["nettests"][:1] # Create a link j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) - orlid = j['oonirun_link_id'] + orlid = j["oonirun_link_id"] # fetch the link meta = deepcopy(SAMPLE_META) - meta['website_category_codes'] = ["NEWS"] - j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + meta["website_category_codes"] = ["NEWS"] + j = postj(client, f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) inputs = j["nettests"][0]["inputs"] - inputs_extra = j['nettests'][0]["inputs_extra"] + inputs_extra = j["nettests"][0]["inputs_extra"] assert len(inputs), inputs assert len(inputs) == len(inputs_extra) for extra in inputs_extra: assert extra["category_code"] == "NEWS" -def test_prioritization_with_measurements(client, client_with_user_role, url_priorities, measurements): + +def test_prioritization_with_measurements( + client, client_with_user_role, url_priorities, measurements +): """ Test priorization including measurements """ z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing header parsing" - z['nettests'][0]['targets_name'] = 'websites_list_prioritized' - z['nettests'][0]['inputs'] = None - z['nettests'][0]['inputs_extra'] = None - z['nettests'] = z['nettests'][:1] - + z["name"] = "Testing header parsing" + z["nettests"][0]["targets_name"] = "websites_list_prioritized" + z["nettests"][0]["inputs"] = None + z["nettests"][0]["inputs_extra"] = None + z["nettests"] = z["nettests"][:1] + # Create a link j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) - orlid = j['oonirun_link_id'] - + orlid = j["oonirun_link_id"] # fetch the link meta = deepcopy(SAMPLE_META) # In ES we have more measurements for twitter, (see tests/fixtures/data/measurements.json) # so twitter should NOT show up first - meta['probe_cc'] = 'ES' - j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + meta["probe_cc"] = "ES" + j = postj(client, f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) inputs = j["nettests"][0]["inputs"] assert len(inputs), inputs assert "twitter.com" not in inputs[0], "Twitter should not be the first one" - # Twitter with a different asn can be first - meta['probe_cc'] = 'ES' - meta['probe_asn'] = 'AS9999' - j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + # Twitter with a different asn can be first + meta["probe_cc"] = "ES" + meta["probe_asn"] = "AS9999" + j = postj(client, f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) inputs = j["nettests"][0]["inputs"] assert len(inputs), inputs assert "twitter.com" in inputs[0], "Twitter should be the first one" - - # Similarly, in IT twitter should be first, and facebook last - meta['probe_cc'] = 'IT' - meta['probe_asn'] = 'AS1234' - j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + # Similarly, in IT twitter should be first, and facebook last + meta["probe_cc"] = "IT" + meta["probe_asn"] = "AS1234" + j = postj(client, f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) inputs = j["nettests"][0]["inputs"] assert len(inputs), inputs assert "twitter.com" in inputs[0], "Twitter should be the first one" assert "facebook.com" in inputs[-1], "Facebook should be the last one" + @pytest.fixture -def super_prioritized_website(clickhouse_db): +def super_prioritized_website(clickhouse_db): values = { "category_code": "*", "cc": "*", "domain": "ooni.org", "priority": 99999, "url": "*", - "sign" : 1 - } + "sign": 1, + } query = "INSERT INTO url_priorities (sign, category_code, cc, domain, url, priority) VALUES" insert_click(clickhouse_db, query, [values]) - yield + yield clickhouse_db.execute("DELETE FROM url_priorities WHERE domain='www.ooni.com'") - -def test_priorities_basic(client, client_with_user_role, measurements, url_priorities, super_prioritized_website): +def test_priorities_basic( + client, + client_with_user_role, + measurements, + url_priorities, + super_prioritized_website, +): z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing header parsing" - z['nettests'][0]['targets_name'] = 'websites_list_prioritized' - z['nettests'][0]['inputs'] = None - z['nettests'][0]['inputs_extra'] = None - z['nettests'] = z['nettests'][:1] - + z["name"] = "Testing header parsing" + z["nettests"][0]["targets_name"] = "websites_list_prioritized" + z["nettests"][0]["inputs"] = None + z["nettests"][0]["inputs_extra"] = None + z["nettests"] = z["nettests"][:1] + # Create a link j = postj(client_with_user_role, "/api/v2/oonirun/links", **z) - orlid = j['oonirun_link_id'] + orlid = j["oonirun_link_id"] meta = deepcopy(SAMPLE_META) - meta['probe_cc'] = 'ES' - j = postj(client,f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) + meta["probe_cc"] = "ES" + j = postj(client, f"/api/v2/oonirun/links/{orlid}/engine-descriptor/latest", **meta) inputs = j["nettests"][0]["inputs"] assert len(inputs), inputs - assert "ooni.org" in inputs[0], "Ooni should be the first one" \ No newline at end of file + assert "ooni.org" in inputs[0], "Ooni should be the first one" diff --git a/ooniapi/services/oonirun/tests/test_database.py b/ooniapi/services/oonirun/tests/test_database.py index cf181629c..12d14a981 100644 --- a/ooniapi/services/oonirun/tests/test_database.py +++ b/ooniapi/services/oonirun/tests/test_database.py @@ -45,7 +45,7 @@ "test_name": "dnscheck", }, { - "targets_name" : "sample_target", + "targets_name": "sample_target", "options": {}, "backend_options": {}, "is_background_run_enabled_default": False, @@ -53,14 +53,11 @@ "test_name": "dnscheck", }, { - "inputs" : [ + "inputs": [ "https://example.com/", "https://ooni.org/", ], - "inputs_extra" : [ - {"category_code" : "HUMR"}, - {} - ], + "inputs_extra": [{"category_code": "HUMR"}, {}], "options": {}, "backend_options": {}, "is_background_run_enabled_default": False, @@ -167,7 +164,9 @@ def test_upgrade_to_head(postgresql): new_row = db.query(models.OONIRunLink).first() assert new_row - assert new_row.nettests[0].revision == 3, "First one to show up should have the latest revision" + assert ( + new_row.nettests[0].revision == 3 + ), "First one to show up should have the latest revision" db.close() diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index ef4c5cdb3..575ef7371 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -79,14 +79,15 @@ ] SAMPLE_META = { - "run_type" : "timed", - "is_charging" : True, - "probe_asn" : "AS1234", - "probe_cc" : "VE", - "network_type" : "wifi", - "website_category_codes" : [] + "run_type": "timed", + "is_charging": True, + "probe_asn": "AS1234", + "probe_cc": "VE", + "network_type": "wifi", + "website_category_codes": [], } + def test_get_version(client): r = client.get("/version") j = r.json() @@ -577,20 +578,23 @@ def test_oonirun_revisions(client, client_with_user_role): ## Fetch nettests for latest r = client.post( f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/latest", - json=SAMPLE_META + json=SAMPLE_META, ) assert r.status_code == 200, r.json() j_latest = r.json() assert j_latest["revision"] == "3", "revision is 3" - + # The engine-descriptor returns a list along with targets name on reading - lastest_nettests[2]['inputs'] = [] - lastest_nettests[2]['inputs_extra'] = [] + lastest_nettests[2]["inputs"] = [] + lastest_nettests[2]["inputs_extra"] = [] assert j_latest["nettests"] == lastest_nettests, "nettests are the same" assert j_latest["date_created"] == latest_date_created, "date created matches" ## Should match latest - r = client.post(f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/3", json=SAMPLE_META) + r = client.post( + f"/api/v2/oonirun/links/{oonirun_link_id_one}/engine-descriptor/3", + json=SAMPLE_META, + ) assert j_latest == r.json() ## Fetch invalid revision number @@ -606,7 +610,9 @@ def test_oonirun_revisions(client, client_with_user_role): assert r.status_code == 404, r.json() ## Get not-existing engine descriptor - r = client.post(f"/api/v2/oonirun/links/404/engine-descriptor/latest", json=SAMPLE_META) + r = client.post( + f"/api/v2/oonirun/links/404/engine-descriptor/latest", json=SAMPLE_META + ) j = r.json() assert r.status_code == 404, r.json() @@ -621,10 +627,12 @@ def test_inputs_extra_length(client, client_with_user_role): "provider": "riseupvpn", } ] - z['nettests'] = nettests + z["nettests"] = nettests r = client_with_user_role.post("/api/v2/oonirun/links", json=z) - assert r.status_code == 422, "Should fail when inputs_extra != None and len(inputs_extra) != len(inputs)" + assert ( + r.status_code == 422 + ), "Should fail when inputs_extra != None and len(inputs_extra) != len(inputs)" nettests[0]["inputs_extra"] = [ { @@ -632,85 +640,91 @@ def test_inputs_extra_length(client, client_with_user_role): }, { "provider": "riseupvpn", - } + }, ] r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 200, "Appropiate inputs extra size, should pass" nettests[0].pop("inputs_extra") r = client_with_user_role.post("/api/v2/oonirun/links", json=z) - assert r.status_code == 200, "No checks should be performed when inputs_extra is None" + assert ( + r.status_code == 200 + ), "No checks should be performed when inputs_extra is None" + def test_is_latest_list(client, client_with_user_role): """ Test that the only_latest argument in /links filters properly """ - # Create link - z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing list filtering" + # Create link + z = deepcopy(SAMPLE_OONIRUN) + z["name"] = "Testing list filtering" r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 200, r.json() j = r.json() # Now update it - id = j['oonirun_link_id'] - z['nettests'][0]['inputs'].append("https://ooni.io/") + id = j["oonirun_link_id"] + z["nettests"][0]["inputs"].append("https://ooni.io/") r = client_with_user_role.put(f"/api/v2/oonirun/links/{id}", json=z) assert r.status_code == 200, r.json() j = r.json() - # Check filtering # Only last revision by default r = client.get("/api/v2/oonirun/links") assert r.status_code == 200 j = r.json() - nts = j['oonirun_links'][0]['nettests'] + nts = j["oonirun_links"][0]["nettests"] assert len(nts) == 3, "There are only 3 nettests in the last revision" # All revisions - r = client.get("/api/v2/oonirun/links", params = {"only_latest" : False}) + r = client.get("/api/v2/oonirun/links", params={"only_latest": False}) assert r.status_code == 200 j = r.json() - nts = j['oonirun_links'][0]['nettests'] + nts = j["oonirun_links"][0]["nettests"] assert len(nts) == 6, "There are 6 nettests between all revisions" + def test_link_revision_args(client, client_with_user_role): # Check args parsing for oonirun engine-descriptor z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing descriptor revision" + z["name"] = "Testing descriptor revision" r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 200, r.json() j = r.json() - id = j['oonirun_link_id'] + id = j["oonirun_link_id"] # Try with good arguments - gs = ['timed', 'manual'] + gs = ["timed", "manual"] for good in gs: - r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json=SAMPLE_META) + r = client.post( + f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json=SAMPLE_META + ) assert r.status_code == 200, r.json() # Try with bad arguments bm = deepcopy(SAMPLE_META) - bm['run_type'] = "bad" + bm["run_type"] = "bad" r = client.post(f"/api/v2/oonirun/links/{id}/engine-descriptor/1", json=bm) assert r.status_code == 422, r.json() + def test_inputs_and_targets_name(client_with_user_role): """ - Test that you can't specify targets_name and inputs in the same request + Test that you can't specify targets_name and inputs in the same request """ z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing no targets and inputs at the same time" + z["name"] = "Testing no targets and inputs at the same time" - # Only inputs = OK + # Only inputs = OK r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 200, r.json() # Only targets = OK - z['nettests'] = [ + z["nettests"] = [ { "inputs": None, "targets_name": "example_name", @@ -727,7 +741,7 @@ def test_inputs_and_targets_name(client_with_user_role): assert r.status_code == 200, r.json() # Both targets and input = error - z['nettests'] = [ + z["nettests"] = [ { "inputs": [ "https://example.com/", @@ -747,7 +761,7 @@ def test_inputs_and_targets_name(client_with_user_role): assert r.status_code == 422, r.json() # Both targets and inputs_extra = error - z['nettests'] = [ + z["nettests"] = [ { "targets_name": "example_name", "inputs_extra": [{}, {}], @@ -762,12 +776,13 @@ def test_inputs_and_targets_name(client_with_user_role): r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 422, r.json() + def test_creation_with_targets_name(client_with_user_role): z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing dynamic test lists calculation" - z['nettests'][0]['inputs'] = None - z['nettests'][0]['targets_name'] = "websites_list_prioritized" - z['nettests'] = z['nettests'][:1] + z["name"] = "Testing dynamic test lists calculation" + z["nettests"][0]["inputs"] = None + z["nettests"][0]["targets_name"] = "websites_list_prioritized" + z["nettests"] = z["nettests"][:1] # Create r = client_with_user_role.post("/api/v2/oonirun/links", json=z) @@ -780,11 +795,15 @@ def test_creation_with_targets_name(client_with_user_role): j = r.json() # Does it have the targets name? - assert j['nettests'][0]['targets_name'] == "websites_list_prioritized", "Missing targets_name" + assert ( + j["nettests"][0]["targets_name"] == "websites_list_prioritized" + ), "Missing targets_name" # now test that you can edit - z['nettests'][0]['targets_name'] = "new_value" - r = client_with_user_role.put(f"/api/v2/oonirun/links/{j['oonirun_link_id']}", json=z) + z["nettests"][0]["targets_name"] = "new_value" + r = client_with_user_role.put( + f"/api/v2/oonirun/links/{j['oonirun_link_id']}", json=z + ) assert r.status_code == 200, r.json() # Retrieve again @@ -792,32 +811,38 @@ def test_creation_with_targets_name(client_with_user_role): assert r.status_code == 200, r.json() j = r.json() - assert j['nettests'][0]['targets_name'] == 'new_value', "Value of nettest should be changed by now" + assert ( + j["nettests"][0]["targets_name"] == "new_value" + ), "Value of nettest should be changed by now" + def test_dynamic_test_lists_calculation(client_with_user_role): z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing dynamic test lists calculation" - z['nettests'][0]['inputs'] = None - z['nettests'][0]['targets_name'] = "websites_list_prioritized" - z['nettests'] = z['nettests'][:1] + z["name"] = "Testing dynamic test lists calculation" + z["nettests"][0]["inputs"] = None + z["nettests"][0]["targets_name"] = "websites_list_prioritized" + z["nettests"] = z["nettests"][:1] r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 200, r.json() j = r.json() - r = client_with_user_role.post(f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", json=SAMPLE_META) + r = client_with_user_role.post( + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + json=SAMPLE_META, + ) assert r.status_code == 200, r.json() j = r.json() - assert j['nettests'][0]['targets_name'] == "websites_list_prioritized" + assert j["nettests"][0]["targets_name"] == "websites_list_prioritized" # TODO(luis) Finish this test -# TODO(luis) finish this test for checking the parsing of user agent headers +# TODO(luis) finish this test for checking the parsing of user agent headers def test_x_user_agent_header_parsing(client_with_user_role, client): z = deepcopy(SAMPLE_OONIRUN) - z['name'] = "Testing header parsing" + z["name"] = "Testing header parsing" r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 200, r.json() @@ -825,32 +850,38 @@ def test_x_user_agent_header_parsing(client_with_user_role, client): # Test with good headers headers = { - "UserAgent" : "ooniprobe-android-unattended,3.8.2,android,ooniprobe-engine,3.17.2,ooniprobe-engine_1.2.3" + "UserAgent": "ooniprobe-android-unattended,3.8.2,android,ooniprobe-engine,3.17.2,ooniprobe-engine_1.2.3" } - r = client_with_user_role.post(f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", json=SAMPLE_META) + r = client_with_user_role.post( + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + json=SAMPLE_META, + ) r = client.post( - f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", - headers=headers, - json=SAMPLE_META - ) + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + headers=headers, + json=SAMPLE_META, + ) assert r.status_code == 200, r.json() # Should be able to skip the header - r = client_with_user_role.post(f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", json=SAMPLE_META) + r = client_with_user_role.post( + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + json=SAMPLE_META, + ) r = client.post( - f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", - json=SAMPLE_META - ) + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + json=SAMPLE_META, + ) assert r.status_code == 200, r.json() # Bad header headers = { - "UserAgent" : "ooniprobe-android-unattended,3.8.2,android,ooniprobe-engine,3.17.2" + "UserAgent": "ooniprobe-android-unattended,3.8.2,android,ooniprobe-engine,3.17.2" } r = client.post( - f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", - headers=headers, - json=SAMPLE_META - ) - assert r.status_code == 422, r.json() \ No newline at end of file + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + headers=headers, + json=SAMPLE_META, + ) + assert r.status_code == 422, r.json() From 279c0b628dc6ed21a8a8ad6c72a288a0e7125a7d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 09:47:50 +0200 Subject: [PATCH 57/72] fix bad ooni domain --- ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index 190b67853..c2b98d3ce 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -173,7 +173,7 @@ def super_prioritized_website(clickhouse_db): query = "INSERT INTO url_priorities (sign, category_code, cc, domain, url, priority) VALUES" insert_click(clickhouse_db, query, [values]) yield - clickhouse_db.execute("DELETE FROM url_priorities WHERE domain='www.ooni.com'") + clickhouse_db.execute("DELETE FROM url_priorities WHERE domain='ooni.org'") def test_priorities_basic( From 5f41fca5a466752c720fc8644b07a3e15afe51bb Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 10:52:00 +0200 Subject: [PATCH 58/72] Move fixtures to conftest; root fixtures dir to THIS_DIR --- ooniapi/services/oonirun/tests/conftest.py | 74 +++++++++++++++++++ .../oonirun/tests/integ/test_dynamic_lists.py | 71 ------------------ .../services/oonirun/tests/test_oonirun.py | 3 - 3 files changed, 74 insertions(+), 74 deletions(-) diff --git a/ooniapi/services/oonirun/tests/conftest.py b/ooniapi/services/oonirun/tests/conftest.py index 98a172ea5..eaaa17c9e 100644 --- a/ooniapi/services/oonirun/tests/conftest.py +++ b/ooniapi/services/oonirun/tests/conftest.py @@ -1,5 +1,9 @@ import pathlib +from pathlib import Path import pytest +import json +import random +from datetime import datetime, timedelta, UTC import time import jwt @@ -7,10 +11,13 @@ from fastapi.testclient import TestClient from oonirun.common.config import Settings +from oonirun.common.clickhouse_utils import insert_click from oonirun.common.dependencies import get_settings from oonirun.main import app from clickhouse_driver import Client as ClickhouseClient +THIS_DIR = Path(__file__).parent.resolve() + def make_override_get_settings(**kw): def override_get_settings(): @@ -120,3 +127,70 @@ def clickhouse_server(docker_ip, docker_services): @pytest.fixture(scope="session") def clickhouse_db(clickhouse_server): yield ClickhouseClient.from_url(clickhouse_server) + + +@pytest.fixture(scope="module") +def fixtures_data_dir(): + yield Path(THIS_DIR, "fixtures/data") + + +@pytest.fixture(scope="module") +def url_priorities(clickhouse_db, fixtures_data_dir): + filename = "url_priorities_us.json" + file = Path(fixtures_data_dir, filename) + + with file.open("r") as f: + j = json.load(f) + + # 'sign' is created with default value 0, causing a db error. + # use 1 to prevent it + for row in j: + row["sign"] = 1 + + query = "INSERT INTO url_priorities (sign, category_code, cc, domain, url, priority) VALUES" + insert_click(clickhouse_db, query, j) + yield + clickhouse_db.execute("TRUNCATE TABLE url_priorities") + + +def generate_random_date_last_7_days() -> datetime: + start = datetime.now(tz=UTC) - timedelta(days=7) + + # return a random date between 7 days ago and now + return start + timedelta( + seconds=random.randrange(3600 * 24, 3600 * 24 * 7 - 3600 * 24) + ) + + +@pytest.fixture(scope="module") +def measurements(clickhouse_db, fixtures_data_dir): + msmnts_dir = Path(fixtures_data_dir, "measurements.json") + with open(msmnts_dir, "r") as f: + measurements = json.load(f) + + for ms in measurements: + date = generate_random_date_last_7_days() + ms["measurement_start_time"] = date + ms["test_start_time"] = date + + query = "INSERT INTO fastpath VALUES" + insert_click(clickhouse_db, query, measurements) + + yield + clickhouse_db.execute("TRUNCATE TABLE url_priorities") + + +@pytest.fixture +def super_prioritized_website(clickhouse_db): + values = { + "category_code": "*", + "cc": "*", + "domain": "ooni.org", + "priority": 99999, + "url": "*", + "sign": 1, + } + query = "INSERT INTO url_priorities (sign, category_code, cc, domain, url, priority) VALUES" + insert_click(clickhouse_db, query, [values]) + yield + clickhouse_db.execute("DELETE FROM url_priorities WHERE domain='ooni.org'") diff --git a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py index c2b98d3ce..5ffabb8a8 100644 --- a/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py +++ b/ooniapi/services/oonirun/tests/integ/test_dynamic_lists.py @@ -1,11 +1,7 @@ from copy import deepcopy -import json -from pathlib import Path from oonirun.common.clickhouse_utils import insert_click import pytest from ..test_oonirun import SAMPLE_OONIRUN, SAMPLE_META -from datetime import datetime, timedelta, UTC -import random def postj(client, url, **kw): @@ -14,57 +10,6 @@ def postj(client, url, **kw): return response.json() -@pytest.fixture(scope="module") -def fixtures_data_dir(): - yield Path("tests/fixtures/data") - - -@pytest.fixture(scope="module") -def url_priorities(clickhouse_db, fixtures_data_dir): - filename = "url_priorities_us.json" - file = Path(fixtures_data_dir, filename) - - with file.open("r") as f: - j = json.load(f) - - # 'sign' is created with default value 0, causing a db error. - # use 1 to prevent it - for row in j: - row["sign"] = 1 - - query = "INSERT INTO url_priorities (sign, category_code, cc, domain, url, priority) VALUES" - insert_click(clickhouse_db, query, j) - yield - clickhouse_db.execute("TRUNCATE TABLE url_priorities") - - -def generate_random_date_last_7_days() -> datetime: - start = datetime.now(tz=UTC) - timedelta(days=7) - - # return a random date between 7 days ago and now - return start + timedelta( - seconds=random.randrange(3600 * 24, 3600 * 24 * 7 - 3600 * 24) - ) - - -@pytest.fixture(scope="module") -def measurements(clickhouse_db, fixtures_data_dir): - msmnts_dir = Path(fixtures_data_dir, "measurements.json") - with open(msmnts_dir, "r") as f: - measurements = json.load(f) - - for ms in measurements: - date = generate_random_date_last_7_days() - ms["measurement_start_time"] = date - ms["test_start_time"] = date - - query = "INSERT INTO fastpath VALUES" - insert_click(clickhouse_db, query, measurements) - - yield - clickhouse_db.execute("TRUNCATE TABLE url_priorities") - - def test_engine_descriptor_basic(client, client_with_user_role, url_priorities): z = deepcopy(SAMPLE_OONIRUN) z["name"] = "Testing simple prioritizationpri" @@ -160,22 +105,6 @@ def test_prioritization_with_measurements( assert "facebook.com" in inputs[-1], "Facebook should be the last one" -@pytest.fixture -def super_prioritized_website(clickhouse_db): - values = { - "category_code": "*", - "cc": "*", - "domain": "ooni.org", - "priority": 99999, - "url": "*", - "sign": 1, - } - query = "INSERT INTO url_priorities (sign, category_code, cc, domain, url, priority) VALUES" - insert_click(clickhouse_db, query, [values]) - yield - clickhouse_db.execute("DELETE FROM url_priorities WHERE domain='ooni.org'") - - def test_priorities_basic( client, client_with_user_role, diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 575ef7371..4aa61a089 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -836,10 +836,7 @@ def test_dynamic_test_lists_calculation(client_with_user_role): j = r.json() assert j["nettests"][0]["targets_name"] == "websites_list_prioritized" - # TODO(luis) Finish this test - -# TODO(luis) finish this test for checking the parsing of user agent headers def test_x_user_agent_header_parsing(client_with_user_role, client): z = deepcopy(SAMPLE_OONIRUN) z["name"] = "Testing header parsing" From e25e0539e272d722046ef1d372f05d26bd4c6a86 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 11:17:11 +0200 Subject: [PATCH 59/72] Add network type validation and some tests --- .../oonirun/src/oonirun/routers/v2.py | 14 +++++++- .../services/oonirun/tests/test_oonirun.py | 33 ++++++++++++++++++- 2 files changed, 45 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 6958283da..d79d24dee 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -38,12 +38,24 @@ def utcnow_seconds(): return datetime.now(timezone.utc).replace(microsecond=0) +NETWORK_TYPES = [ + "vpn", + "wifi", + "mobile", + "wired_ethernet", + "no_internet", + "unknown", +] + + class OonirunMeta(BaseModel): run_type: str = Field(description="Run type", pattern="^(timed|manual)$") is_charging: bool = Field(description="If the probe is charging") probe_asn: str = Field(pattern=r"^([a-zA-Z0-9]+)$") probe_cc: str = Field(description="Country code. Ex: VE") - network_type: str = Field(description="Ex: wifi") + network_type: str = Field( + description="Ex: wifi", pattern=f"^({'|'.join(NETWORK_TYPES)})$" + ) website_category_codes: List[str] = Field( description="List of category codes that user has chosen to test (eg. NEWS,HUMR)", default=[], diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 4aa61a089..3122ec57d 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -6,7 +6,7 @@ from datetime import datetime, timedelta, timezone import time -from oonirun.routers.v2 import utcnow_seconds +from oonirun.routers.v2 import utcnow_seconds, NETWORK_TYPES SAMPLE_OONIRUN = { @@ -882,3 +882,34 @@ def test_x_user_agent_header_parsing(client_with_user_role, client): json=SAMPLE_META, ) assert r.status_code == 422, r.json() + + +def test_network_type_validation(client_with_user_role, client): + z = deepcopy(SAMPLE_OONIRUN) + z["name"] = "Testing dynamic test lists calculation" + z["nettests"][0]["inputs"] = None + z["nettests"][0]["targets_name"] = "websites_list_prioritized" + z["nettests"] = z["nettests"][:1] + + # Create + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 200, r.json() + j = r.json() + + # try to compute dynamic list with each network type + meta = deepcopy(SAMPLE_META) + for nt in NETWORK_TYPES: + meta["network_type"] = nt + r = client_with_user_role.post( + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + json=meta, + ) + assert r.status_code == 200, r.json() + + # try with a bad network type + meta["network_type"] = "bad" + r = client_with_user_role.post( + f"/api/v2/oonirun/links/{j['oonirun_link_id']}/engine-descriptor/latest", + json=meta, + ) + assert r.status_code == 422, r.json() From f3a914701cc8906e3f874a302e439911537683a7 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 11:19:38 +0200 Subject: [PATCH 60/72] Improve ASN validation --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index d79d24dee..7aa283b36 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -51,7 +51,7 @@ def utcnow_seconds(): class OonirunMeta(BaseModel): run_type: str = Field(description="Run type", pattern="^(timed|manual)$") is_charging: bool = Field(description="If the probe is charging") - probe_asn: str = Field(pattern=r"^([a-zA-Z0-9]+)$") + probe_asn: str = Field(pattern=r"^(AS)?([0-9]{1,10})$") probe_cc: str = Field(description="Country code. Ex: VE") network_type: str = Field( description="Ex: wifi", pattern=f"^({'|'.join(NETWORK_TYPES)})$" From 54f8f0ff5863f16c508d15afef53cbb4599cd91e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 11:22:18 +0200 Subject: [PATCH 61/72] rename header for anonymous credentials --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 7aa283b36..eddc9a989 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -608,7 +608,7 @@ def get_oonirun_link_engine_descriptor( description="Expected format: ,,,,,", ), ] = None, - credentials: Annotated[ + x_ooni_credentials: Annotated[ Optional[bytes], Header(description="base64 encoded OONI anonymous credentials") ] = None, ): From c823315ca304f8cd54f3205c16e9b1534d676e62 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 11:25:18 +0200 Subject: [PATCH 62/72] Remove only_latest parameter --- .../oonirun/src/oonirun/routers/v2.py | 6 +--- .../services/oonirun/tests/test_oonirun.py | 35 ------------------- 2 files changed, 1 insertion(+), 40 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index eddc9a989..b89143a44 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -733,10 +733,6 @@ def list_oonirun_links( Optional[bool], Query(description="List also expired descriptors"), ] = None, - only_latest: Annotated[ - Optional[bool], - Query(description="List only descriptors in the latest revision"), - ] = True, authorization: str = Header("authorization"), settings=Depends(get_settings), ) -> OONIRunLinkList: @@ -758,7 +754,7 @@ def list_oonirun_links( ), "nettests must be sorted by revision" # if revision is None, it will get all the nettests, including from old revisions - nettests, _ = get_nettests(row, revision if only_latest else None) + nettests, _ = get_nettests(row, revision) oonirun_link = OONIRunLink( oonirun_link_id=row.oonirun_link_id, diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 3122ec57d..05974d428 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -652,41 +652,6 @@ def test_inputs_extra_length(client, client_with_user_role): ), "No checks should be performed when inputs_extra is None" -def test_is_latest_list(client, client_with_user_role): - """ - Test that the only_latest argument in /links filters properly - """ - - # Create link - z = deepcopy(SAMPLE_OONIRUN) - z["name"] = "Testing list filtering" - r = client_with_user_role.post("/api/v2/oonirun/links", json=z) - assert r.status_code == 200, r.json() - j = r.json() - - # Now update it - id = j["oonirun_link_id"] - z["nettests"][0]["inputs"].append("https://ooni.io/") - r = client_with_user_role.put(f"/api/v2/oonirun/links/{id}", json=z) - assert r.status_code == 200, r.json() - j = r.json() - - # Check filtering - # Only last revision by default - r = client.get("/api/v2/oonirun/links") - assert r.status_code == 200 - j = r.json() - nts = j["oonirun_links"][0]["nettests"] - assert len(nts) == 3, "There are only 3 nettests in the last revision" - - # All revisions - r = client.get("/api/v2/oonirun/links", params={"only_latest": False}) - assert r.status_code == 200 - j = r.json() - nts = j["oonirun_links"][0]["nettests"] - assert len(nts) == 6, "There are 6 nettests between all revisions" - - def test_link_revision_args(client, client_with_user_role): # Check args parsing for oonirun engine-descriptor z = deepcopy(SAMPLE_OONIRUN) From a3f1a8d9ad10bed635af16a38bf0c9069b21b773 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 11:42:25 +0200 Subject: [PATCH 63/72] Simplify user agent header parsing --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 7 ++----- 1 file changed, 2 insertions(+), 5 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index b89143a44..56d095181 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -601,7 +601,7 @@ def get_oonirun_link_engine_descriptor( clickhouse: DependsClickhouseClient, meta: OonirunMeta, useragent: Annotated[ - Optional[str], # TODO Marked as optional to avoid breaking old probes + Optional[str], Header( pattern=USER_AGENT_PATTERN, error_message="Expected format: ,,,,,", @@ -621,9 +621,6 @@ def get_oonirun_link_engine_descriptor( revision = None if useragent is not None: - result = re.match(USER_AGENT_PATTERN, useragent) - # Validated by fastapi - assert result is not None ( software_name, software_version, @@ -631,7 +628,7 @@ def get_oonirun_link_engine_descriptor( engine_name, engine_version, engine_version_full, - ) = result.groups() + ) = useragent.split(",") # TODO Log this metadata q = db.query(models.OONIRunLink).filter( From 70240f3113f3d8091e648847a25d15215e008243 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 12:00:54 +0200 Subject: [PATCH 64/72] Changed default value of inputs field --- .../services/oonirun/src/oonirun/routers/v2.py | 7 +++---- ooniapi/services/oonirun/tests/test_oonirun.py | 17 +++++++++++++++++ 2 files changed, 20 insertions(+), 4 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 56d095181..b7a5df385 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -8,7 +8,6 @@ from typing import Dict, List, Optional, Tuple, Any from typing_extensions import Annotated, Self import logging -import re import sqlalchemy as sa from sqlalchemy.orm import Session @@ -70,9 +69,9 @@ class OONIRunLinkNettest(BaseModel): default="", title="name of the ooni nettest", min_length=2, max_length=100 ) inputs: Optional[List[str]] = Field( - default=[], title="list of input dictionaries for the nettest" + default=None, title="list of input dictionaries for the nettest" ) - # TODO(luis): Options and backend_options not in the new spec. Should be removed? + # TODO(luis): Options not in the new spec. Should be removed? options: Dict = Field(default={}, title="options for the nettest") is_background_run_enabled_default: bool = Field( default=False, @@ -601,7 +600,7 @@ def get_oonirun_link_engine_descriptor( clickhouse: DependsClickhouseClient, meta: OonirunMeta, useragent: Annotated[ - Optional[str], + Optional[str], Header( pattern=USER_AGENT_PATTERN, error_message="Expected format: ,,,,,", diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 05974d428..e10a0eca3 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -741,6 +741,23 @@ def test_inputs_and_targets_name(client_with_user_role): r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 422, r.json() + # Targets with inputs = [] still an error + z["nettests"] = [ + { + "targets_name": "example_name", + "inputs_extra": [], + "inputs": [], + "options": { + "HTTP3Enabled": True, + }, + "is_background_run_enabled_default": False, + "is_manual_run_enabled_default": False, + "test_name": "web_connectivity", + }, + ] + r = client_with_user_role.post("/api/v2/oonirun/links", json=z) + assert r.status_code == 422, r.json() + def test_creation_with_targets_name(client_with_user_role): z = deepcopy(SAMPLE_OONIRUN) From 59f47dda8fbe71baf1df5624950bdeb98bdd437b Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 14:21:38 +0200 Subject: [PATCH 65/72] Add flag to compute dynamic lists in get_nettest function --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 6 ++++-- 1 file changed, 4 insertions(+), 2 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index b7a5df385..e91d7f3d8 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -458,6 +458,7 @@ def make_nettest_websites_list_prioritized( def get_nettests( oonirun_link: models.OONIRunLink, revision: Optional[int], + compute_dynamic_lists: bool = False, meta: Optional[OonirunMeta] = None, clickhouse: Optional[Clickhouse] = None, ) -> Tuple[List[OONIRunLinkNettest], datetime]: @@ -476,7 +477,8 @@ def get_nettests( date_created = nt.date_created inputs, inputs_extra = nt.inputs, nt.inputs_extra targets_name = nt.targets_name - if nt.targets_name is not None and meta is not None: + if compute_dynamic_lists and nt.targets_name is not None: + assert meta is not None, "OoniMeta is required to compute dynamic lists" assert ( clickhouse is not None ), "Clickhouse is required to compute the dynamic lists" @@ -644,7 +646,7 @@ def get_oonirun_link_engine_descriptor( revision = latest_revision assert isinstance(revision, int) - nettests, date_created = get_nettests(res, revision, meta, clickhouse) + nettests, date_created = get_nettests(res, revision, True, meta, clickhouse) return OONIRunLinkEngineDescriptor( nettests=nettests, date_created=date_created, From f53a30a61a2e1674395c17cdbbd8ea9e8613f46f Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 15:32:30 +0200 Subject: [PATCH 66/72] Remove backend_options and options, even from the DB --- ooniapi/services/oonirun/src/oonirun/models.py | 2 -- .../services/oonirun/src/oonirun/routers/v2.py | 6 ------ ooniapi/services/oonirun/tests/test_database.py | 10 ---------- ooniapi/services/oonirun/tests/test_oonirun.py | 17 ----------------- 4 files changed, 35 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/models.py b/ooniapi/services/oonirun/src/oonirun/models.py index f318bfedd..1f3cc823b 100644 --- a/ooniapi/services/oonirun/src/oonirun/models.py +++ b/ooniapi/services/oonirun/src/oonirun/models.py @@ -56,8 +56,6 @@ class OONIRunLinkNettest(Base): test_name: Mapped[str] = mapped_column() inputs: Mapped[List[str]] = mapped_column(nullable=True) - options: Mapped[Dict[str, Any]] = mapped_column(nullable=True) - backend_options: Mapped[Dict[str, Any]] = mapped_column(nullable=True) is_background_run_enabled_default: Mapped[bool] = mapped_column(default=False) is_manual_run_enabled_default: Mapped[bool] = mapped_column(default=False) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index e91d7f3d8..dd0efa79d 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -71,8 +71,6 @@ class OONIRunLinkNettest(BaseModel): inputs: Optional[List[str]] = Field( default=None, title="list of input dictionaries for the nettest" ) - # TODO(luis): Options not in the new spec. Should be removed? - options: Dict = Field(default={}, title="options for the nettest") is_background_run_enabled_default: bool = Field( default=False, title="if this test should be enabled by default for background runs", @@ -259,7 +257,6 @@ def create_oonirun_link( nettest = OONIRunLinkNettest( test_name=nt.test_name, inputs=nt.inputs, - options=nt.options, targets_name=nt.targets_name, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, @@ -355,7 +352,6 @@ def edit_oonirun_link( targets_name=nt.targets_name, test_name=nt.test_name, inputs=nt.inputs, - options=nt.options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, ) @@ -371,7 +367,6 @@ def edit_oonirun_link( date_created=now, test_name=nt.test_name, inputs=nt.inputs, - options=nt.options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, oonirun_link=oonirun_link, @@ -492,7 +487,6 @@ def get_nettests( test_name=nt.test_name, inputs=inputs, inputs_extra=inputs_extra, - options=nt.options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, ) diff --git a/ooniapi/services/oonirun/tests/test_database.py b/ooniapi/services/oonirun/tests/test_database.py index 12d14a981..1adc5a2e7 100644 --- a/ooniapi/services/oonirun/tests/test_database.py +++ b/ooniapi/services/oonirun/tests/test_database.py @@ -28,26 +28,18 @@ "https://example.com/", "https://ooni.org/", ], - "options": { - "HTTP3Enabled": True, - }, - "backend_options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", }, { "inputs": [], - "options": {}, - "backend_options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", }, { "targets_name": "sample_target", - "options": {}, - "backend_options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", @@ -58,8 +50,6 @@ "https://ooni.org/", ], "inputs_extra": [{"category_code": "HUMR"}, {}], - "options": {}, - "backend_options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index e10a0eca3..cff27953b 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -30,9 +30,6 @@ ], "targets_name": None, "inputs_extra": None, - "options": { - "HTTP3Enabled": True, - }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -41,7 +38,6 @@ "inputs": [], "targets_name": None, "inputs_extra": None, - "options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", @@ -50,7 +46,6 @@ "inputs": None, "targets_name": "websites_list_prioritized", "inputs_extra": None, - "options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", @@ -694,9 +689,6 @@ def test_inputs_and_targets_name(client_with_user_role): "inputs": None, "targets_name": "example_name", "inputs_extra": None, - "options": { - "HTTP3Enabled": True, - }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -714,9 +706,6 @@ def test_inputs_and_targets_name(client_with_user_role): ], "targets_name": "example_name", "inputs_extra": None, - "options": { - "HTTP3Enabled": True, - }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -730,9 +719,6 @@ def test_inputs_and_targets_name(client_with_user_role): { "targets_name": "example_name", "inputs_extra": [{}, {}], - "options": { - "HTTP3Enabled": True, - }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -747,9 +733,6 @@ def test_inputs_and_targets_name(client_with_user_role): "targets_name": "example_name", "inputs_extra": [], "inputs": [], - "options": { - "HTTP3Enabled": True, - }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", From 0d28ee76efd712680f830ea6eaf3a6068e6f330e Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 4 Jun 2025 15:42:07 +0200 Subject: [PATCH 67/72] Split dynamic test list calculation from nettest db fetch --- .../oonirun/src/oonirun/routers/v2.py | 30 ++++++++++--------- 1 file changed, 16 insertions(+), 14 deletions(-) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index dd0efa79d..7fe1df2ce 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -453,9 +453,6 @@ def make_nettest_websites_list_prioritized( def get_nettests( oonirun_link: models.OONIRunLink, revision: Optional[int], - compute_dynamic_lists: bool = False, - meta: Optional[OonirunMeta] = None, - clickhouse: Optional[Clickhouse] = None, ) -> Tuple[List[OONIRunLinkNettest], datetime]: """Computes a list of nettests related to the given oonirun link @@ -472,14 +469,6 @@ def get_nettests( date_created = nt.date_created inputs, inputs_extra = nt.inputs, nt.inputs_extra targets_name = nt.targets_name - if compute_dynamic_lists and nt.targets_name is not None: - assert meta is not None, "OoniMeta is required to compute dynamic lists" - assert ( - clickhouse is not None - ), "Clickhouse is required to compute the dynamic lists" - inputs, inputs_extra = make_test_lists_from_targets_name( - nt.targets_name, meta, clickhouse - ) nettests.append( OONIRunLinkNettest( @@ -493,12 +482,21 @@ def get_nettests( ) return nettests, date_created +def populate_dynamic_lists(nettest : OONIRunLinkNettest, meta : OonirunMeta, clickhouse : Clickhouse): + + if nettest.targets_name is None: + return + + inputs, inputs_extra = make_test_lists_from_targets_name( + nettest.targets_name, meta, clickhouse + ) + nettest.inputs = inputs + nettest.inputs_extra = inputs_extra def make_oonirun_link( db: Session, oonirun_link_id: str, account_id: Optional[str], - meta: Optional[OonirunMeta] = None, revision: Optional[int] = None, ): q = db.query(models.OONIRunLink).filter( @@ -517,7 +515,7 @@ def make_oonirun_link( assert isinstance(revision, int) - nettests, date_created = get_nettests(res, revision, meta) + nettests, date_created = get_nettests(res, revision) return OONIRunLink( oonirun_link_id=res.oonirun_link_id, name=res.name, @@ -640,7 +638,11 @@ def get_oonirun_link_engine_descriptor( revision = latest_revision assert isinstance(revision, int) - nettests, date_created = get_nettests(res, revision, True, meta, clickhouse) + + nettests, date_created = get_nettests(res, revision) + for nt in nettests: + populate_dynamic_lists(nt, meta, clickhouse) + return OONIRunLinkEngineDescriptor( nettests=nettests, date_created=date_created, From eeb97433f1b15b4dcd3315de923f831ddd555d2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 5 Jun 2025 10:27:07 +0200 Subject: [PATCH 68/72] drop backend_options and options column --- ...eb79750f_add_targets_name_and_inputs_extra_.py | 15 +++++++++------ 1 file changed, 9 insertions(+), 6 deletions(-) diff --git a/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py index 8163a78bf..df8465d27 100644 --- a/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py +++ b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py @@ -5,6 +5,7 @@ Create Date: 2025-05-21 15:44:32.959349 """ + from typing import Sequence, Union from alembic import op @@ -12,19 +13,21 @@ # revision identifiers, used by Alembic. -revision: str = 'b860eb79750f' -down_revision: Union[str, None] = '8e7ecea5c2f5' +revision: str = "b860eb79750f" +down_revision: Union[str, None] = "8e7ecea5c2f5" branch_labels: Union[str, Sequence[str], None] = None depends_on: Union[str, Sequence[str], None] = None def upgrade() -> None: - op.add_column("oonirun_nettest", - sa.Column("targets_name", sa.String(), nullable=True) + op.add_column( + "oonirun_nettest", sa.Column("targets_name", sa.String(), nullable=True) ) - op.add_column("oonirun_nettest", - sa.Column("inputs_extra", sa.ARRAY(sa.JSON()), nullable=True) + op.add_column( + "oonirun_nettest", sa.Column("inputs_extra", sa.ARRAY(sa.JSON()), nullable=True) ) + op.drop_column("oonirun_nettest", "backend_options") + op.drop_column("oonirun_nettest", "options") def downgrade() -> None: From 3d099899100fe649a03d1840461003a118117a2d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Thu, 5 Jun 2025 10:35:49 +0200 Subject: [PATCH 69/72] Add backend_options and options on downgrade --- .../b860eb79750f_add_targets_name_and_inputs_extra_.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py index df8465d27..7bb2047aa 100644 --- a/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py +++ b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py @@ -33,3 +33,6 @@ def upgrade() -> None: def downgrade() -> None: op.drop_column("oonirun_nettest", "targets_name") op.drop_column("oonirun_nettest", "inputs_extra") + + op.add_column("oonirun_nettest", sa.Column("backend_options", sa.ARRAY(sa.JSON()), nullable=True)) + op.add_column("oonirun_nettest", sa.Column("options", sa.ARRAY(sa.JSON()), nullable=True)) \ No newline at end of file From a7bf78f330a68c7a7349a8278bd511b8c6a4777d Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 10 Jun 2025 10:19:30 +0200 Subject: [PATCH 70/72] Restor options field in nettest --- ...79750f_add_targets_name_and_inputs_extra_.py | 4 +--- ooniapi/services/oonirun/src/oonirun/models.py | 1 + .../services/oonirun/src/oonirun/routers/v2.py | 5 +++++ ooniapi/services/oonirun/tests/test_database.py | 6 ++++++ ooniapi/services/oonirun/tests/test_oonirun.py | 17 +++++++++++++++++ 5 files changed, 30 insertions(+), 3 deletions(-) diff --git a/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py index 7bb2047aa..831ff0640 100644 --- a/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py +++ b/ooniapi/common/src/common/alembic/versions/b860eb79750f_add_targets_name_and_inputs_extra_.py @@ -27,12 +27,10 @@ def upgrade() -> None: "oonirun_nettest", sa.Column("inputs_extra", sa.ARRAY(sa.JSON()), nullable=True) ) op.drop_column("oonirun_nettest", "backend_options") - op.drop_column("oonirun_nettest", "options") def downgrade() -> None: op.drop_column("oonirun_nettest", "targets_name") op.drop_column("oonirun_nettest", "inputs_extra") - op.add_column("oonirun_nettest", sa.Column("backend_options", sa.ARRAY(sa.JSON()), nullable=True)) - op.add_column("oonirun_nettest", sa.Column("options", sa.ARRAY(sa.JSON()), nullable=True)) \ No newline at end of file + op.add_column("oonirun_nettest", sa.Column("backend_options", sa.ARRAY(sa.JSON()), nullable=True)) \ No newline at end of file diff --git a/ooniapi/services/oonirun/src/oonirun/models.py b/ooniapi/services/oonirun/src/oonirun/models.py index 1f3cc823b..abad33909 100644 --- a/ooniapi/services/oonirun/src/oonirun/models.py +++ b/ooniapi/services/oonirun/src/oonirun/models.py @@ -56,6 +56,7 @@ class OONIRunLinkNettest(Base): test_name: Mapped[str] = mapped_column() inputs: Mapped[List[str]] = mapped_column(nullable=True) + options: Mapped[Dict[str, Any]] = mapped_column(nullable=True) is_background_run_enabled_default: Mapped[bool] = mapped_column(default=False) is_manual_run_enabled_default: Mapped[bool] = mapped_column(default=False) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index 7fe1df2ce..e7a0c4a2b 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -71,6 +71,7 @@ class OONIRunLinkNettest(BaseModel): inputs: Optional[List[str]] = Field( default=None, title="list of input dictionaries for the nettest" ) + options: Dict = Field(default={}, title="options for the nettest") is_background_run_enabled_default: bool = Field( default=False, title="if this test should be enabled by default for background runs", @@ -257,6 +258,7 @@ def create_oonirun_link( nettest = OONIRunLinkNettest( test_name=nt.test_name, inputs=nt.inputs, + options=nt.options, targets_name=nt.targets_name, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, @@ -352,6 +354,7 @@ def edit_oonirun_link( targets_name=nt.targets_name, test_name=nt.test_name, inputs=nt.inputs, + options=nt.options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, ) @@ -367,6 +370,7 @@ def edit_oonirun_link( date_created=now, test_name=nt.test_name, inputs=nt.inputs, + options=nt.options, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, oonirun_link=oonirun_link, @@ -475,6 +479,7 @@ def get_nettests( targets_name=targets_name, test_name=nt.test_name, inputs=inputs, + options=nt.options, inputs_extra=inputs_extra, is_background_run_enabled_default=nt.is_background_run_enabled_default, is_manual_run_enabled_default=nt.is_manual_run_enabled_default, diff --git a/ooniapi/services/oonirun/tests/test_database.py b/ooniapi/services/oonirun/tests/test_database.py index 1adc5a2e7..d9a62786a 100644 --- a/ooniapi/services/oonirun/tests/test_database.py +++ b/ooniapi/services/oonirun/tests/test_database.py @@ -28,18 +28,23 @@ "https://example.com/", "https://ooni.org/", ], + "options": { + "HTTP3Enabled": True, + }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", }, { "inputs": [], + "options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", }, { "targets_name": "sample_target", + "options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", @@ -50,6 +55,7 @@ "https://ooni.org/", ], "inputs_extra": [{"category_code": "HUMR"}, {}], + "options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index cff27953b..9df6fa08c 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -30,6 +30,9 @@ ], "targets_name": None, "inputs_extra": None, + "options": { + "HTTP3Enabled": True, + }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -38,6 +41,7 @@ "inputs": [], "targets_name": None, "inputs_extra": None, + "options": {}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", @@ -46,6 +50,7 @@ "inputs": None, "targets_name": "websites_list_prioritized", "inputs_extra": None, + "options":{}, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "dnscheck", @@ -689,6 +694,9 @@ def test_inputs_and_targets_name(client_with_user_role): "inputs": None, "targets_name": "example_name", "inputs_extra": None, + "options": { + "HTTP3Enabled": True, + }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -706,6 +714,9 @@ def test_inputs_and_targets_name(client_with_user_role): ], "targets_name": "example_name", "inputs_extra": None, + "options": { + "HTTP3Enabled": True, + }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -719,6 +730,9 @@ def test_inputs_and_targets_name(client_with_user_role): { "targets_name": "example_name", "inputs_extra": [{}, {}], + "options": { + "HTTP3Enabled": True, + }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", @@ -733,6 +747,9 @@ def test_inputs_and_targets_name(client_with_user_role): "targets_name": "example_name", "inputs_extra": [], "inputs": [], + "options": { + "HTTP3Enabled": True, + }, "is_background_run_enabled_default": False, "is_manual_run_enabled_default": False, "test_name": "web_connectivity", From 6868ab9af9e418d5efe0fdc4c3f660cb5ce39db5 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Tue, 10 Jun 2025 15:10:31 +0200 Subject: [PATCH 71/72] trigger CI --- ooniapi/services/oonirun/tests/test_oonirun.py | 2 +- 1 file changed, 1 insertion(+), 1 deletion(-) diff --git a/ooniapi/services/oonirun/tests/test_oonirun.py b/ooniapi/services/oonirun/tests/test_oonirun.py index 9df6fa08c..74c018503 100644 --- a/ooniapi/services/oonirun/tests/test_oonirun.py +++ b/ooniapi/services/oonirun/tests/test_oonirun.py @@ -725,7 +725,7 @@ def test_inputs_and_targets_name(client_with_user_role): r = client_with_user_role.post("/api/v2/oonirun/links", json=z) assert r.status_code == 422, r.json() - # Both targets and inputs_extra = error + # Both targets and inputs_extra = error z["nettests"] = [ { "targets_name": "example_name", From 48e60d97391f31bf4c0727a8093da60d127189e6 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?Luis=20D=C3=ADaz?= Date: Wed, 18 Jun 2025 13:22:09 +0200 Subject: [PATCH 72/72] Add bluetooth and usb as valid network types --- ooniapi/services/oonirun/src/oonirun/routers/v2.py | 3 +++ 1 file changed, 3 insertions(+) diff --git a/ooniapi/services/oonirun/src/oonirun/routers/v2.py b/ooniapi/services/oonirun/src/oonirun/routers/v2.py index e7a0c4a2b..26438fa3e 100644 --- a/ooniapi/services/oonirun/src/oonirun/routers/v2.py +++ b/ooniapi/services/oonirun/src/oonirun/routers/v2.py @@ -26,6 +26,7 @@ ) from ..common.prio import generate_test_list from ..dependencies import DependsPostgresSession, DependsClickhouseClient +from uuid import uuid4 log = logging.getLogger(__name__) @@ -43,6 +44,8 @@ def utcnow_seconds(): "mobile", "wired_ethernet", "no_internet", + "bluetooth", + "usb", "unknown", ]