Skip to content

Commit

Permalink
🔧 Make direct access to env attributes private
Browse files Browse the repository at this point in the history
These attributes should always be accessed via the API interface,
adding `_` to them makes this contract more explicit.
  • Loading branch information
chrisjsewell committed Oct 2, 2024
1 parent ec2869f commit 1fa5bb8
Show file tree
Hide file tree
Showing 7 changed files with 36 additions and 37 deletions.
46 changes: 23 additions & 23 deletions sphinx_needs/data.py
Original file line number Diff line number Diff line change
Expand Up @@ -764,23 +764,23 @@ def get_or_create_docs(self) -> dict[str, list[str]]:
This is lazily created and cached in the environment.
"""
try:
return self.env.needs_all_docs
return self.env._needs_all_docs
except AttributeError:
self.env.needs_all_docs = {"all": []}
return self.env.needs_all_docs
self.env._needs_all_docs = {"all": []}
return self.env._needs_all_docs

@property
def needs_is_post_processed(self) -> bool:
"""Whether needs have been post-processed."""
try:
return self.env.needs_is_post_processed
return self.env._needs_is_post_processed
except AttributeError:
self.env.needs_is_post_processed = False
return self.env.needs_is_post_processed
self.env._needs_is_post_processed = False
return self.env._needs_is_post_processed

@needs_is_post_processed.setter
def needs_is_post_processed(self, value: bool) -> None:
self.env.needs_is_post_processed = value
self.env._needs_is_post_processed = value

def get_or_create_services(self) -> ServiceManager:
"""Get information about services.
Expand All @@ -790,40 +790,40 @@ def get_or_create_services(self) -> ServiceManager:
from sphinx_needs.services.manager import ServiceManager

try:
return self.env.app.needs_services
return self.env.app._needs_services
except AttributeError:
self.env.app.needs_services = ServiceManager(self.env.app)
return self.env.app.needs_services
self.env.app._needs_services = ServiceManager(self.env.app)
return self.env.app._needs_services

def get_or_create_extends(self) -> dict[str, NeedsExtendType]:
"""Get all need modifications, mapped by ID.
This is lazily created and cached in the environment.
"""
try:
return self.env.need_all_needextend
return self.env._need_all_needextend
except AttributeError:
self.env.need_all_needextend = {}
return self.env.need_all_needextend
self.env._need_all_needextend = {}
return self.env._need_all_needextend

def get_or_create_umls(self) -> dict[str, NeedsUmlType]:
"""Get all need uml diagrams, mapped by ID.
This is lazily created and cached in the environment.
"""
try:
return self.env.needs_all_needumls
return self.env._needs_all_needumls
except AttributeError:
self.env.needs_all_needumls = {}
return self.env.needs_all_needumls
self.env._needs_all_needumls = {}
return self.env._needs_all_needumls

@property
def _needs_all_nodes(self) -> dict[str, Need]:
try:
return self.env.needs_all_nodes
return self.env._needs_all_nodes
except AttributeError:
self.env.needs_all_nodes = {}
return self.env.needs_all_nodes
self.env._needs_all_nodes = {}
return self.env._needs_all_nodes

def set_need_node(self, need_id: str, node: Need) -> None:
"""Set a need node in the cache."""
Expand Down Expand Up @@ -906,7 +906,7 @@ def _merge(name: str, is_complex_dict: bool = False) -> None:
f"not {type(other_objects)} and {type(objects)}"
)

_merge("needs_all_docs", is_complex_dict=True)
_merge("needs_all_nodes")
_merge("need_all_needextend")
_merge("needs_all_needumls")
_merge("_needs_all_docs", is_complex_dict=True)
_merge("_needs_all_nodes")
_merge("_need_all_needextend")
_merge("_needs_all_needumls")
3 changes: 1 addition & 2 deletions sphinx_needs/needs.py
Original file line number Diff line number Diff line change
Expand Up @@ -499,10 +499,9 @@ def prepare_env(app: Sphinx, env: BuildEnvironment, _docname: str) -> None:
"""
needs_config = NeedsSphinxConfig(app.config)
data = SphinxNeedsData(env)
data.get_or_create_docs()
services = data.get_or_create_services()

# Register embedded services
services = data.get_or_create_services()
services.register("github-issues", GithubService, gh_type="issue")
services.register("github-prs", GithubService, gh_type="pr")
services.register("github-commits", GithubService, gh_type="commit")
Expand Down
4 changes: 2 additions & 2 deletions sphinx_needs/warnings.py
Original file line number Diff line number Diff line change
Expand Up @@ -45,10 +45,10 @@ def process_warnings(app: Sphinx, exception: Exception | None) -> None:
# Check if warnings already got executed.
# Needed because the used event gets executed multiple times, but warnings need to be checked only
# on first execution
if hasattr(env, "needs_warnings_executed") and env.needs_warnings_executed:
if hasattr(env, "_needs_warnings_executed") and env._needs_warnings_executed:
return

env.needs_warnings_executed = True # type: ignore[attr-defined]
env._needs_warnings_executed = True # type: ignore[attr-defined]

# Exclude external needs for warnings check
needs_view = needs_view.filter_is_external(False)
Expand Down
4 changes: 2 additions & 2 deletions tests/test_needarch.py
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,7 @@ def test_doc_needarch_jinja_import(test_app, snapshot):
assert html

# check needarch
all_needumls = app.env.needs_all_needumls
all_needumls = app.env._needs_all_needumls
assert all_needumls == snapshot


Expand All @@ -64,7 +64,7 @@ def test_needarch_jinja_func_need(test_app, snapshot):
app = test_app
app.build()

all_needumls = app.env.needs_all_needumls
all_needumls = app.env._needs_all_needumls
assert all_needumls == snapshot

html = Path(app.outdir, "index.html").read_text(encoding="utf8")
Expand Down
8 changes: 4 additions & 4 deletions tests/test_needuml.py
Original file line number Diff line number Diff line change
Expand Up @@ -20,7 +20,7 @@ def test_doc_build_html(test_app, snapshot):
all_needs = dict(SphinxNeedsData(app.env).get_needs_view())
assert all_needs == snapshot()

all_needumls = app.env.needs_all_needumls
all_needumls = app.env._needs_all_needumls
assert all_needumls == snapshot


Expand Down Expand Up @@ -169,7 +169,7 @@ def test_needuml_filter(test_app, snapshot):
app = test_app
app.build()

all_needumls = app.env.needs_all_needumls
all_needumls = app.env._needs_all_needumls
assert all_needumls == snapshot

html = Path(app.outdir, "index.html").read_text(encoding="utf8")
Expand All @@ -193,7 +193,7 @@ def test_needuml_jinja_func_flow(test_app, snapshot):
app = test_app
app.build()

all_needumls = app.env.needs_all_needumls
all_needumls = app.env._needs_all_needumls
assert all_needumls == snapshot

html = Path(app.outdir, "index.html").read_text(encoding="utf8")
Expand Down Expand Up @@ -267,7 +267,7 @@ def test_needuml_jinja_func_ref(test_app, snapshot):
app = test_app
app.build()

all_needumls = app.env.needs_all_needumls
all_needumls = app.env._needs_all_needumls
assert all_needumls == snapshot

html = Path(app.outdir, "index.html").read_text(encoding="utf8")
Expand Down
4 changes: 2 additions & 2 deletions tests/test_open_needs_service.py
Original file line number Diff line number Diff line change
Expand Up @@ -91,9 +91,9 @@ def test_ons_service(test_app):
)

test_app.build()
assert isinstance(test_app.needs_services, ServiceManager)
assert isinstance(test_app._needs_services, ServiceManager)

manager = test_app.needs_services
manager = test_app._needs_services
service = manager.get("open-needs")
assert hasattr(service, "content")

Expand Down
4 changes: 2 additions & 2 deletions tests/test_services/test_service_basics.py
Original file line number Diff line number Diff line change
Expand Up @@ -61,9 +61,9 @@ def request(self, _options):
def test_service_creation(test_app):
app = test_app
app.build()
assert isinstance(app.needs_services, ServiceManager)
assert isinstance(app._needs_services, ServiceManager)

manager = app.needs_services
manager = app._needs_services
service = manager.get("testservice")
assert hasattr(service, "custom_option")

Expand Down

0 comments on commit 1fa5bb8

Please sign in to comment.