Skip to content

Commit

Permalink
Make .freeze() return self, for easier exporting/rebinding (#2)
Browse files Browse the repository at this point in the history
* .freeze() returns self.

* Test failing type checks

* Github Actions: Cache all the things!
  • Loading branch information
flipbit03 authored Dec 31, 2024
1 parent d241599 commit fa89b5d
Show file tree
Hide file tree
Showing 9 changed files with 106 additions and 30 deletions.
39 changes: 39 additions & 0 deletions .github/actions/poetry_cached/action.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,39 @@
name: Get (possibly cached) Project/Pre-commit virtualenvs

runs:
using: "composite"
steps:
- name: Set up Python
id: setup-python
uses: actions/setup-python@v5

- name: Install Poetry
shell: bash
run: |
python -m pip install -qqq poetry
- name: Load cached Poetry virtualenv, if available
id: cached-poetry-dependencies
uses: actions/cache@v4
with:
path: ~/.cache/pypoetry/virtualenvs
key: venv-${{ runner.os }}-${{ hashFiles('poetry.lock') }}-${{ hashFiles('.github/actions/poetry_cached/action.yml') }}-${{ steps.setup-python.outputs.python-version }}

- name: Load cached pre-commit virtualenv
id: cached-pre-commit
uses: actions/cache@v4
with:
path: ~/.cache/pre-commit
key: pre-commit-${{ runner.os }}-${{ hashFiles('.pre-commit-config.yaml') }}-${{ hashFiles('.github/actions/poetry_cached/action.yml') }}-${{ steps.setup-python.outputs.python-version }}

- name: Install poetry dependencies, only if cache wasn't found
if: steps.cached-poetry-dependencies.outputs.cache-hit != 'true'
shell: bash
run: |
poetry install --no-interaction
- name: Install pre-commit hooks, only if cache wasn't found
if: steps.cached-pre-commit.outputs.cache-hit != 'true'
shell: bash
run: |
poetry run pre-commit install-hooks
27 changes: 27 additions & 0 deletions .github/workflows/lints_and_checks.yml
Original file line number Diff line number Diff line change
@@ -0,0 +1,27 @@
name: Lints and Checks

on:
push:
branches: [ main ]
pull_request:
branches: [ main, "*" ]
workflow_dispatch:

jobs:
lints_and_checks:
runs-on: ubuntu-20.04

steps:
- name: Check out repository
uses: actions/checkout@v4

- name: Get (possibly cached) Project/Pre-commit virtualenvs
uses: ./.github/actions/poetry_cached

- name: Static type checking with `mypy`
run: |
poetry run mypy
- name: Linting with `pre-commit` and `ruff`
run: |
poetry run pre-commit run --all-files --show-diff-on-failure
10 changes: 4 additions & 6 deletions .github/workflows/publish.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,13 +14,11 @@ jobs:

# Steps represent a sequence of tasks that will be executed as part of the job
steps:
- uses: actions/checkout@v3
- uses: actions/setup-python@v2
- name: Check out repository
uses: actions/checkout@v4

- name: Install dependencies
run: |
python -m pip install --upgrade pip
pip install poetry
- name: Get (possibly cached) Project/Pre-commit virtualenvs
uses: ./.github/actions/poetry_cached

- name: Set version from GITHUB_REF
run: |
Expand Down
14 changes: 4 additions & 10 deletions .github/workflows/tests.yml
Original file line number Diff line number Diff line change
Expand Up @@ -14,17 +14,11 @@ jobs:
matrix:
python-version: [ "3.11", "3.12", "3.13" ]
steps:
- uses: actions/checkout@v3
- name: Check out repository
uses: actions/checkout@v4

- name: Set up Python ${{ matrix.python-version }}
uses: actions/setup-python@v4
with:
python-version: ${{ matrix.python-version }}

- name: Install dependencies
run: |
python -m pip install poetry
poetry install
- name: Get (possibly cached) Project/Pre-commit virtualenvs
uses: ./.github/actions/poetry_cached

- name: Run Tests
run: |
Expand Down
2 changes: 1 addition & 1 deletion .pre-commit-config.yaml
Original file line number Diff line number Diff line change
@@ -1,4 +1,4 @@
# We specifically don't include the `src/**` folders, as each project (frontend, backend, etc) has their own
# We specifically don't include the `src/**` folders, as each project (frontend, backend, etc.) has their own
# linting and formatting rules.
# Everything else is linted and formatted using this top level pre-commit config
files: ^(?!src/).*$
Expand Down
8 changes: 6 additions & 2 deletions cadurso/system.py
Original file line number Diff line number Diff line change
@@ -1,7 +1,7 @@
import asyncio
import inspect
import logging
from typing import Callable, Hashable, cast
from typing import Callable, Hashable, Self, cast

from cadurso.errors import (
ERROR_AUTH_FUNC_ACTION_NOT_HASHABLE,
Expand Down Expand Up @@ -208,9 +208,11 @@ def can(self, actor: Actor) -> CanQueryBuilder:
"""
return CanQueryBuilder(cadurso=self, actor=actor)

def freeze(self) -> None:
def freeze(self) -> Self:
"""
Freeze the Cadurso instance, preventing new rules from being added.
Returns the instance itself, for convenience and easier rebinding.
"""

if self.__frozen:
Expand All @@ -220,3 +222,5 @@ def freeze(self) -> None:

# Log the number of rules added
logger.debug(f"Frozen Cadurso instance with {self.rule_count} rules")

return self
2 changes: 1 addition & 1 deletion tests/akira/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -358,7 +358,7 @@ async def tetsuo_can_destroy_neotokyo_if_melting_down(

@akira_universe.add_rule(LocationPermission.DESTROY)
def colonel_refuses_to_destroy_any_location(
actor: Character, location: Location
actor: Character, _location: Location
) -> bool:
"""The Colonel won't personally obliterate major locations on a whim."""
return actor == colonel and False
Expand Down
6 changes: 3 additions & 3 deletions tests/brazil/conftest.py
Original file line number Diff line number Diff line change
Expand Up @@ -263,7 +263,7 @@ def only_minister_archives(actor: Character, _form: OfficialForm) -> bool:
# DUCT RULES
@brazil_universe.add_rule(DuctPermission.INSPECT)
def engineer_inspects_duct(actor: Character, _duct: DuctSystem) -> bool:
"""ENGINEERs are allowed to INSPECT duct systems."""
"""ENGINEERS are allowed to INSPECT duct systems."""
return Role.ENGINEER in actor.roles

@brazil_universe.add_rule(DuctPermission.INSPECT)
Expand All @@ -273,13 +273,13 @@ def rebel_inspects_duct(actor: Character, _duct: DuctSystem) -> bool:

@brazil_universe.add_rule(DuctPermission.INSPECT)
def bureaucrat_inspects_duct(actor: Character, _duct: DuctSystem) -> bool:
"""BUREAUCRATs want to keep an eye on everything, so they can INSPECT ducts as well."""
"""BUREAUCRATS want to keep an eye on everything, so they can INSPECT ducts as well."""
return Role.BUREAUCRAT in actor.roles

@brazil_universe.add_rule(DuctPermission.REPAIR)
def engineer_with_form_27b6(actor: Character, _duct: DuctSystem) -> bool:
"""
ENGINEERs may REPAIR a duct, but only if they are carrying the form 27B/6
ENGINEERS may REPAIR a duct, but only if they are carrying the form 27B/6
in their pocket.
"""
if Role.ENGINEER not in actor.roles:
Expand Down
28 changes: 21 additions & 7 deletions tests/test_rule_definition_errors.py
Original file line number Diff line number Diff line change
Expand Up @@ -21,12 +21,26 @@ def test_fail_to_add_rule_to_frozen_cadurso_instance() -> None:
You cannot add a rule to a frozen Cadurso instance.
"""
cadurso = Cadurso()
cadurso.freeze()

# Freeze the instance
cadurso_frozen = cadurso.freeze()

# .freeze() returns itself.
# This is useful for re-exporting the authz system *after* defining all the rules. e.g:
#
# cadurso_base = Cadurso()
# @cadurso_base.add_rule(...) (several times)
#
# Then, somewhere else in the code, probably in a top-level module:
# import cadurso_base from some_module
# cadurso = cadurso_base.freeze()
# __all__ = ["cadurso"]
assert cadurso_frozen is cadurso

with pytest.raises(CadursoRuleDefinitionError) as exc_info:

@cadurso.add_rule("test")
def this_rule_will_blow_up(actor: str, resource: str) -> bool:
def this_rule_will_blow_up(_actor: str, _resource: str) -> bool:
return True

assert cast(str, exc_info.value.args[0]) == ERROR_CANNOT_ADD_RULE_FROZEN
Expand Down Expand Up @@ -54,7 +68,7 @@ def test_fail_cannot_add_auth_function_with_more_than_two_parameters() -> None:
with pytest.raises(CadursoRuleDefinitionError) as exc_info:

@cadurso.add_rule("test") # type: ignore
def this_rule_will_blow_up(actor: str, resource: str, extra: str) -> bool:
def this_rule_will_blow_up(_actor: str, _resource: str, _extra: str) -> bool:
# Mypy will complain about this, but we're testing the runtime behavior
return True

Expand Down Expand Up @@ -104,7 +118,7 @@ def test_fail_cannot_add_rule_with_nonpositional_params() -> None:
with pytest.raises(CadursoRuleDefinitionError) as exc_info:

@cadurso.add_rule("test") # type: ignore
def this_rule_will_blow_up(actor: str, *, resource: str) -> bool:
def this_rule_will_blow_up(_actor: str, *, _resource: str) -> bool:
# This function has 2 parameters, so the parameter count check will pass.
# But the second parameter, `resource`, is a keyword-only argument
# MyPy will complain about this, but we're testing the runtime behavior
Expand All @@ -120,7 +134,7 @@ def test_can_add_rule_with_only_positional_params() -> None:
cadurso = Cadurso()

@cadurso.add_rule("test")
def this_rule_will_not_blow_up(actor: str, resource: str, /) -> bool:
def this_rule_will_not_blow_up(_actor: str, _resource: str, /) -> bool:
# Both of the parameters are positional *ONLY*, because of the `/` in the signature
# This will work just fine
return True
Expand All @@ -138,7 +152,7 @@ def test_fail_cannot_add_rule_that_does_not_return_bool() -> None:
with pytest.raises(CadursoRuleDefinitionError) as exc_info:
# MyPy will complain about this, but we're testing the runtime behavior, so we type-ignore
@cadurso.add_rule("test") # type: ignore
def this_rule_will_blow_up(actor: str, resource: str) -> str:
def this_rule_will_blow_up(_actor: str, _resource: str) -> str:
# We don't care if non-zero length strings are truthy
# We favor explicitness, as preconized in the Zen of Python *wink*

Expand Down Expand Up @@ -166,7 +180,7 @@ class User:

# MyPy will complain about this, but we're testing the runtime behavior, so we type-ignore
@cadurso.add_rule("test")
def this_rule_will_blow_up(actor: john, resource: int) -> bool: # type: ignore
def this_rule_will_blow_up(_actor: john, _resource: int) -> bool: # type: ignore
# `john` is an instance, not a type
return True

Expand Down

0 comments on commit fa89b5d

Please sign in to comment.