From 25684535e85a8bbb05b65b0da2304273d1d90ca0 Mon Sep 17 00:00:00 2001 From: =?UTF-8?q?=C3=81rni=20Dagur?= Date: Mon, 27 Jul 2020 20:35:50 +0000 Subject: Update upstream library to 0.3.0, respond to API changes (#10) * Update upstream library to 0.3.0, respond to API changes * Change pypi publishing criteria in CI * Create test to make sure the version numbers are the same everywhere --- .github/workflows/ci.yml | 7 +- Cargo.toml | 4 +- adblock/adblock.pyi | 28 ++++---- pyproject.toml | 3 +- src/lib.rs | 150 ++++++++++++++++++++++++++++++------------ tests/test_engine.py | 48 ++++++++++---- tests/test_typestubs.py | 53 +++++++++++++++ tests/test_version_numbers.py | 28 ++++++++ 8 files changed, 249 insertions(+), 72 deletions(-) create mode 100644 tests/test_typestubs.py create mode 100644 tests/test_version_numbers.py diff --git a/.github/workflows/ci.yml b/.github/workflows/ci.yml index 18cdc06..4071231 100644 --- a/.github/workflows/ci.yml +++ b/.github/workflows/ci.yml @@ -73,8 +73,13 @@ jobs: run: poetry run maturin develop --release - name: Run Python tests + if: matrix.python-version != 3.5 run: poetry run pytest -vv --color=yes + - name: Run Python tests (skip typestub tests) + if: matrix.python-version == 3.5 + run: poetry run pytest -vv --color=yes --ignore=tests/test_typestubs.py + python-publish: needs: build runs-on: ${{ matrix.os }} @@ -138,7 +143,7 @@ jobs: GITHUB_TOKEN: ${{ secrets.GITHUB_TOKEN }} - name: PyPi publish - if: startsWith(github.ref, 'refs/tags/') + if: github.event_name == 'release' && github.event.action == 'created' env: MATURIN_PASSWORD: ${{ secrets.PYPI }} run: poetry run maturin publish --interpreter python${{matrix.python_version}} --username __token__ diff --git a/Cargo.toml b/Cargo.toml index acba2c9..4af9670 100644 --- a/Cargo.toml +++ b/Cargo.toml @@ -1,7 +1,7 @@ [package] name = "adblock" publish = false -version = "0.2.3" +version = "0.3.0" authors = ["Árni Dagur str: pass -class HostnameSpecificResources: +class UrlSpecificResources: hide_selectors: Set[str] style_selectors: Dict[str, List[str]] exceptions: Set[str] @@ -19,14 +19,16 @@ class HostnameSpecificResources: def __repr__(self) -> str: pass +class FilterSet: + def __init__(self, debug: bool = False) -> None: + pass + def add_filter_list(self, filter_list: str, format: str) -> None: + pass + def add_filters(self, filters: List[str], format: str) -> None: + pass + class Engine: - def __init__( - self, - network_filters: Optional[List[str]] = None, - load_network: bool = True, - load_cosmetic: bool = False, - debug: bool = False, - ) -> None: + def __init__(self, filter_set: FilterSet, optimize: bool = True) -> None: pass def check_network_urls( self, url: str, source_url: str, request_type: str @@ -60,17 +62,17 @@ class Engine: pass def deserialize_from_file(self, file: str) -> None: pass - def add_filter_list(self, filter_list: str) -> None: - pass def filter_exists(self, filter: str) -> bool: pass - def tags_enable(self, tags: List[str]) -> None: + def use_tags(self, tags: List[str]) -> None: + pass + def enable_tags(self, tags: List[str]) -> None: pass - def tags_disable(self, tags: List[str]) -> None: + def disable_tags(self, tags: List[str]) -> None: pass def tag_exists(self, tag: str) -> bool: pass - def hostname_cosmetic_resources(self, hostname: str) -> HostnameSpecificResources: + def url_cosmetic_resources(self, url: str) -> UrlSpecificResources: pass def hidden_class_id_selectors( self, classes: List[str], ids: List[str], exceptions: Set[str] diff --git a/pyproject.toml b/pyproject.toml index d92a78f..099e239 100644 --- a/pyproject.toml +++ b/pyproject.toml @@ -1,6 +1,6 @@ [tool.poetry] name = "adblock" -version = "0.2.3" +version = "0.3.0" description = "Brave's adblocking in Python" authors = ["Árni Dagur "] license = "MIT OR Apache-2.0" @@ -14,3 +14,4 @@ python = "^3.5" [tool.poetry.dev-dependencies] maturin = "*" pytest = "*" +toml = "*" diff --git a/src/lib.rs b/src/lib.rs index 588f4e9..7839a42 100644 --- a/src/lib.rs +++ b/src/lib.rs @@ -12,8 +12,10 @@ use adblock::blocker::BlockerError as RustBlockerError; use adblock::blocker::BlockerResult as RustBlockerResult; -use adblock::cosmetic_filter_cache::HostnameSpecificResources as RustHostnameSpecificResources; +use adblock::cosmetic_filter_cache::UrlSpecificResources as RustUrlSpecificResources; use adblock::engine::Engine as RustEngine; +use adblock::lists::FilterFormat; +use adblock::lists::FilterSet as RustFilterSet; use pyo3::class::PyObjectProtocol; use pyo3::exceptions::ValueError as PyValueError; use pyo3::prelude::*; @@ -32,8 +34,9 @@ use std::io::{Read, Write}; fn adblock(_py: Python<'_>, m: &PyModule) -> PyResult<()> { m.add("__version__", env!("CARGO_PKG_VERSION"))?; m.add_class::()?; + m.add_class::()?; m.add_class::()?; - m.add_class::()?; + m.add_class::()?; Ok(()) } @@ -163,10 +166,71 @@ impl Into for RustBlockerError { } } +fn filter_format_from_string(filter_format: &str) -> PyResult { + match filter_format { + "standard" => Ok(FilterFormat::Standard), + "hosts" => Ok(FilterFormat::Hosts), + _ => Err(PyErr::new::("Invalid format value")), + } +} + +/// Manages a set of rules to be added to an Engine. +/// +/// To be able to efficiently handle special options like $badfilter, and to +/// allow optimizations, all rules must be available when the Engine is first +/// created. FilterSet allows assembling a compound list from multiple +/// different sources before compiling the rules into an Engine. +#[pyclass] +#[text_signature = "($self, debug)"] +#[derive(Clone)] +pub struct FilterSet { + filter_set: RustFilterSet, +} + +#[pymethods] +impl FilterSet { + /// Creates a new `FilterSet`. The `debug` argument specifies whether or + /// not to save information about the original raw filter rules alongside + /// the more compact internal representation. If enabled, this information + /// will be passed to the corresponding Engine. + #[new] + #[args(debug = false)] + pub fn new(debug: bool) -> Self { + Self { + filter_set: RustFilterSet::new(debug), + } + } + + /// Adds the contents of an entire filter list to this FilterSet. Filters + /// that cannot be parsed successfully are ignored. + /// + /// The format is a string containing either "standard" (ABP/uBO-style) + /// or "hosts". + #[text_signature = "($self, filter_list, format)"] + #[args(filter_list, format = "\"standard\"")] + pub fn add_filter_list(&mut self, filter_list: &str, format: &str) -> PyResult<()> { + let filter_format = filter_format_from_string(format)?; + self.filter_set.add_filter_list(filter_list, filter_format); + Ok(()) + } + + /// Adds a collection of filter rules to this FilterSet. Filters that + /// cannot be parsed successfully are ignored. + /// + /// The format is a string containing either "standard" (ABP/uBO-style) + /// or "hosts". + #[text_signature = "($self, filters, format)"] + #[args(filters, format = "\"standard\"")] + pub fn add_filters(&mut self, filters: Vec, format: &str) -> PyResult<()> { + let filter_format = filter_format_from_string(format)?; + self.filter_set.add_filters(&filters, filter_format); + Ok(()) + } +} /// Contains cosmetic filter information intended to be injected into a /// particular hostname. #[pyclass] -pub struct HostnameSpecificResources { +pub struct UrlSpecificResources { /// A set of any CSS selector on the page that should be hidden, i.e. /// styled as `{ display: none !important; }`. #[pyo3(get)] @@ -187,9 +251,9 @@ pub struct HostnameSpecificResources { pub injected_script: String, } -impl Into for RustHostnameSpecificResources { - fn into(self) -> HostnameSpecificResources { - HostnameSpecificResources { +impl Into for RustUrlSpecificResources { + fn into(self) -> UrlSpecificResources { + UrlSpecificResources { hide_selectors: self.hide_selectors, style_selectors: self.style_selectors, exceptions: self.exceptions, @@ -199,10 +263,10 @@ impl Into for RustHostnameSpecificResources { } #[pyproto] -impl PyObjectProtocol for HostnameSpecificResources { +impl PyObjectProtocol for UrlSpecificResources { fn __repr__(&self) -> PyResult { Ok(format!( - "HostnameSpecificResources<{} hide selectors, {} style selectors, {} exceptions, injected_javascript={:?}>", + "UrlSpecificResources<{} hide selectors, {} style selectors, {} exceptions, injected_javascript={:?}>", self.hide_selectors.len(), self.style_selectors.len(), self.exceptions.len(), @@ -231,7 +295,7 @@ impl PyObjectProtocol for HostnameSpecificResources { /// /// [1]: https://developer.mozilla.org/en-US/docs/Mozilla/Add-ons/WebExtensions/API/webRequest/ResourceType #[pyclass] -#[text_signature = "($self, network_filters=None, load_network=True, load_cosmetic=False, debug=False)"] +#[text_signature = "($self, filter_set, optimize)"] pub struct Engine { engine: RustEngine, } @@ -240,21 +304,9 @@ pub struct Engine { impl Engine { /// Create a new adblocking engine #[new] - #[args(network_filters="None", load_network=true, load_cosmetic=false, debug=false)] - pub fn new( - network_filters: Option>, - load_network: bool, - load_cosmetic: bool, - debug: bool, - ) -> Self { - let filters = network_filters.unwrap_or(Vec::new()); - let engine = RustEngine::from_rules_parametrised( - &filters, - load_network, - load_cosmetic, - debug, - true, - ); + #[args(filter_set, optimize = true)] + pub fn new(filter_set: FilterSet, optimize: bool) -> Self { + let engine = RustEngine::from_filter_set(filter_set.filter_set, optimize); Self { engine } } @@ -404,43 +456,57 @@ impl Engine { self.deserialize(&data) } - /// Add the contents of a block list file to the blocking engine. - #[text_signature = "($self, filter_list)"] - pub fn add_filter_list(&mut self, filter_list: &str) { - self.engine.add_filter_list(filter_list); - } - /// Checks if the given filter exists in the blocking engine. #[text_signature = "($self, filter)"] pub fn filter_exists(&self, filter: &str) -> bool { self.engine.filter_exists(filter) } - /// Enable the given tags + /// Sets this engine's tags to be _only_ the ones provided in tags. + /// + /// Tags can be used to cheaply enable or disable network rules with a + /// corresponding $tag option. #[text_signature = "($self, tags)"] - pub fn tags_enable(&mut self, tags: Vec<&str>) { - self.engine.tags_enable(&tags); + pub fn use_tags(&mut self, tags: Vec<&str>) { + self.engine.use_tags(&tags); } - /// Disable the given tags + /// Sets this engine's tags to additionally include the ones provided in + /// tags. + /// + /// Tags can be used to cheaply enable or disable network rules with a + /// corresponding $tag option. #[text_signature = "($self, tags)"] - pub fn tags_disable(&mut self, tags: Vec<&str>) { - self.engine.tags_disable(&tags); + pub fn enable_tags(&mut self, tags: Vec<&str>) { + self.engine.enable_tags(&tags); } - /// Check if the given tag exists + /// Sets this engine's tags to no longer include the ones provided in + /// tags. + /// + /// Tags can be used to cheaply enable or disable network rules with a + /// corresponding $tag option. + #[text_signature = "($self, tags)"] + pub fn disable_tags(&mut self, tags: Vec<&str>) { + self.engine.disable_tags(&tags); + } + + /// Checks if a given tag exists in this engine. + /// + /// Tags can be used to cheaply enable or disable network rules with a + /// corresponding $tag option. #[text_signature = "($self, tag)"] pub fn tag_exists(&self, tag: &str) -> bool { self.engine.tag_exists(tag) } /// Returns a set of cosmetic filter resources required for a particular - /// hostname. Once this has been called, all CSS ids and classes on a + /// url. Once this has been called, all CSS ids and classes on a /// page should be passed to hidden_class_id_selectors to obtain any /// stylesheets consisting of generic rules. - #[text_signature = "($self, hostname)"] - pub fn hostname_cosmetic_resources(&self, hostname: &str) -> HostnameSpecificResources { - self.engine.hostname_cosmetic_resources(hostname).into() + #[text_signature = "($self, url)"] + pub fn url_cosmetic_resources(&self, url: &str) -> UrlSpecificResources { + self.engine.url_cosmetic_resources(url).into() } /// If any of the provided CSS classes or ids could cause a certain generic @@ -449,7 +515,7 @@ impl Engine { /// referencing those classes or ids, provided that the corresponding rules /// are not excepted. /// - /// Exceptions should be passed directly from HostnameSpecificResources. + /// Exceptions should be passed directly from UrlSpecificResources. #[text_signature = "($self, classes, ids, exceptions)"] pub fn hidden_class_id_selectors( &self, diff --git a/tests/test_engine.py b/tests/test_engine.py index 28e25e6..a3d2898 100644 --- a/tests/test_engine.py +++ b/tests/test_engine.py @@ -1,40 +1,62 @@ import adblock import pytest +SMALL_FILTER_LIST = """ +||wikipedia.org^ +||old.reddit.com^ +||lobste.rs^ +""" -def test_engine_arguments(): - # None of these should panic - adblock.Engine() - adblock.Engine([]) - adblock.Engine(network_filters=None) - adblock.Engine(network_filters=[]) - adblock.Engine(load_network=False, load_cosmetic=True, debug=False) - adblock.Engine(debug=True) + +def empty_engine(): + return adblock.Engine(adblock.FilterSet()) + + +def test_engine_creation_and_blocking(): + filter_set = adblock.FilterSet(debug=True) + filter_set.add_filter_list(SMALL_FILTER_LIST) + engine = adblock.Engine(filter_set=filter_set) + + blocker_result_wikipedia = engine.check_network_urls( + url="https://wikipedia.org/img.png", + source_url="https://google.com/", + request_type="image", + ) + assert isinstance(blocker_result_wikipedia, adblock.BlockerResult) + assert blocker_result_wikipedia.matched + + blocker_result_facebook = engine.check_network_urls( + "https://facebook.com/directory/img.png", + "https://old.reddit.com/r/all", + "image", + ) + assert isinstance(blocker_result_facebook, adblock.BlockerResult) + assert not blocker_result_facebook.matched def test_serde_file(tmpdir): path = str(tmpdir / "cache.dat") - engine0 = adblock.Engine() + engine0 = empty_engine() with pytest.raises(FileNotFoundError): # We haven't created the cache.dat file, so we should get an exception # when attempting to deserialize. engine0.deserialize_from_file(path) - engine1 = adblock.Engine() + engine1 = empty_engine() serialization_result = engine1.serialize_to_file(path) assert serialization_result is None - engine2 = adblock.Engine() + engine2 = empty_engine() deserialization_result = engine2.deserialize_from_file(path) assert deserialization_result is None def test_serde(): - engine = adblock.Engine() + engine = empty_engine() serialization_result = engine.serialize() assert isinstance(serialization_result, bytes) - engine2 = adblock.Engine() + engine2 = empty_engine() deserialization_result = engine2.deserialize(serialization_result) assert deserialization_result is None diff --git a/tests/test_typestubs.py b/tests/test_typestubs.py new file mode 100644 index 0000000..6a8d75f --- /dev/null +++ b/tests/test_typestubs.py @@ -0,0 +1,53 @@ +import ast +import re + + +def read_stubfile(): + with open("adblock/adblock.pyi", encoding="utf-8") as file: + node = ast.parse(file.read()) + return node + + +def get_functions_and_methods(node): + functions = [n for n in node.body if isinstance(n, ast.FunctionDef)] + classes = [n for n in node.body if isinstance(n, ast.ClassDef)] + + methods = {} + for c in classes: + methods[c.name] = [n for n in c.body if isinstance(n, ast.FunctionDef)] + + return functions, methods + + +def pattern_exists_in_file(filename, regex): + """ + Checks if the given regex is present in the given file + """ + with open(filename, "r", encoding="utf-8") as f: + for line in f: + if re.search(regex, line): + return True + return False + + +def test_functions_and_methods_exist_in_rust(): + """ + Check that for each of the functions and methods present in the Python + typestub file, there is a line in `src/lib.rs` containing a matching + definition. Since we're doing a naive grep search, without access to the + Rust AST, there may be false negatives. + """ + stubfile_node = read_stubfile() + functions, methods = get_functions_and_methods(stubfile_node) + + methods_flattened = [] + for class_methods in methods.values(): + methods_flattened += class_methods + + for f in functions + methods_flattened: + if f.name.startswith("__"): + # Skip dunder methods since their names are the same for every + # class, making the test not particularly useful. They are also not + # marked `pub` in Rust. + continue + assert pattern_exists_in_file("src/lib.rs", r"pub fn {}".format(f.name)) diff --git a/tests/test_version_numbers.py b/tests/test_version_numbers.py new file mode 100644 index 0000000..6519ffa --- /dev/null +++ b/tests/test_version_numbers.py @@ -0,0 +1,28 @@ +import toml +import adblock + + +def get_version_value_poetry(): + with open("pyproject.toml", encoding="utf-8") as f: + pyproject_toml = toml.loads(f.read()) + return pyproject_toml["tool"]["poetry"]["version"] + + +def get_version_value_cargo(): + with open("Cargo.toml", encoding="utf-8") as f: + cargo_toml = toml.loads(f.read()) + return cargo_toml["package"]["version"] + + +def test_version_numbers_all_same(): + """ + Makes sure that `pyproject.toml` and `Cargo.toml` contain the same version + number as the one attached to the `adblock` module. + """ + cargo_version = get_version_value_cargo() + poetry_version = get_version_value_poetry() + module_version = adblock.__version__ + + assert cargo_version == poetry_version + assert poetry_version == module_version + assert cargo_version == module_version -- cgit v1.2.3