Skip to content

Commit

Permalink
[#889] Removed excessive requests,fixed tests
Browse files Browse the repository at this point in the history
  • Loading branch information
vaszig committed Nov 21, 2022
1 parent 56b2b2b commit 302efb8
Show file tree
Hide file tree
Showing 4 changed files with 84 additions and 47 deletions.
32 changes: 16 additions & 16 deletions src/open_inwoner/accounts/views/cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -83,33 +83,26 @@ class CaseListView(BaseBreadcrumbMixin, CaseAccessMixin, TemplateView):
def crumbs(self):
return [(_("Mijn aanvragen"), reverse("accounts:my_cases"))]

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)

def get_cases(self):
cases = fetch_cases(self.request.user.bsn)

case_types = {}
status_types = {}
temp_status_types = []
current_statuses = {}

for case in cases:
# Fetch new case types
if case.zaaktype not in case_types.keys():
case_type = fetch_single_case_type(case.zaaktype)
case_types.update({case_type.url: case_type})
case_types[case_type.url] = case_type

# Fetch list of case's status types and each one (unique) separately
temp_status_types += [st.url for st in fetch_status_types(case.zaaktype)]

for status_type_url in set(temp_status_types):
if status_type_url not in status_types:
status_type = fetch_single_status_type(status_type_url)
status_types.update({status_type.url: status_type})
# Fetch new status types
for st in fetch_status_types(case.zaaktype):
status_types[st.url] = st

# Fetch case's current status
current_status = fetch_specific_status(case.status)
current_statuses.update({current_status.zaak: current_status})
current_statuses[current_status.zaak] = current_status

# Prepare data for frontend
updated_cases = []
Expand All @@ -119,7 +112,7 @@ def get_context_data(self, **kwargs):
# If the status type does not exist in the status types, retrieve it manually
if current_status and not current_status.statustype in status_types:
status_type = fetch_single_status_type(current_status.statustype)
status_types.update({status_type.url: status_type})
status_types[status_type.url] = status_type

updated_cases.append(
{
Expand All @@ -138,20 +131,27 @@ def get_context_data(self, **kwargs):
}
)

return updated_cases

def get_context_data(self, **kwargs):
context = super().get_context_data(**kwargs)

cases = self.get_cases()

context["anchors"] = [
("#pending_apps", _("Lopende aanvragen")),
("#completed_apps", _("Afgeronde aanvragen")),
]

context["open_cases"] = [
case
for case in updated_cases
for case in cases
if not case["end_date"] and not case["current_status"] == "Afgerond"
]
context["open_cases"].sort(key=lambda case: case["start_date"])
context["closed_cases"] = [
case
for case in updated_cases
for case in cases
if case["end_date"] or case["current_status"] == "Afgerond"
]
context["closed_cases"].sort(key=lambda case: case["end_date"])
Expand Down
4 changes: 2 additions & 2 deletions src/open_inwoner/openzaak/catalog.py
Original file line number Diff line number Diff line change
Expand Up @@ -14,6 +14,7 @@
logger = logging.getLogger(__name__)


@cache_result("status_types_for_case_type:{case_type}", timeout=60 * 60 * 24)
def fetch_status_types(case_type=None) -> List[StatusType]:
client = build_client("catalogi")

Expand Down Expand Up @@ -42,7 +43,6 @@ def fetch_status_types(case_type=None) -> List[StatusType]:
return status_types


@cache_result("statustype:{status_type_url}", timeout=60 * 60 * 24)
def fetch_single_status_type(status_type_url: str) -> Optional[StatusType]:
client = build_client("catalogi")

Expand All @@ -63,7 +63,7 @@ def fetch_single_status_type(status_type_url: str) -> Optional[StatusType]:
return status_type


@cache_result("zaaktype:{case_type_url}", timeout=60 * 60)
@cache_result("case_type:{case_type_url}", timeout=60 * 60 * 24)
def fetch_single_case_type(case_type_url: str) -> Optional[ZaakType]:
client = build_client("catalogi")

Expand Down
82 changes: 53 additions & 29 deletions src/open_inwoner/openzaak/tests/test_cases.py
Original file line number Diff line number Diff line change
Expand Up @@ -225,32 +225,32 @@ def test_case_types_are_cached(self, m):
self._setUpMocks(m)

# Cache is empty before the request
self.assertIsNone(cache.get(f"zaaktype:{self.zaaktype['url']}"))
self.assertIsNone(cache.get(f"case_type:{self.zaaktype['url']}"))

self.app.get(reverse("accounts:my_cases"), user=self.user)

# Case type is cached after the request
self.assertIsNotNone(cache.get(f"zaaktype:{self.zaaktype['url']}"))
self.assertIsNotNone(cache.get(f"case_type:{self.zaaktype['url']}"))

def test_cached_case_types_are_deleted_after_one_hour(self, m):
def test_cached_case_types_are_deleted_after_one_day(self, m):
self._setUpMocks(m)

with freeze_time("2022-01-01 12:00") as frozen_time:
self.app.get(reverse("accounts:my_cases"), user=self.user)

# After one hour the results should be deleted
frozen_time.tick(delta=datetime.timedelta(hours=1))
self.assertIsNone(cache.get(f"zaaktype:{self.zaaktype['url']}"))
# After one day the results should be deleted
frozen_time.tick(delta=datetime.timedelta(days=1))
self.assertIsNone(cache.get(f"case_type:{self.zaaktype['url']}"))

def test_cached_case_types_in_combination_with_new_ones(self, m):
self._setUpMocks(m)

# First attempt
self.assertIsNone(cache.get(f"zaaktype:{self.zaaktype['url']}"))
self.assertIsNone(cache.get(f"case_type:{self.zaaktype['url']}"))

self.app.get(reverse("accounts:my_cases"), user=self.user)

self.assertIsNotNone(cache.get(f"zaaktype:{self.zaaktype['url']}"))
self.assertIsNotNone(cache.get(f"case_type:{self.zaaktype['url']}"))

# Second attempt with new case and case type
new_zaak = generate_oas_component(
Expand Down Expand Up @@ -327,25 +327,33 @@ def test_cached_case_types_in_combination_with_new_ones(self, m):
json=new_status,
)

self.assertIsNone(cache.get(f"zaaktype:{new_zaaktype['url']}"))
self.assertIsNone(cache.get(f"case_type:{new_zaaktype['url']}"))

self.app.get(reverse("accounts:my_cases"), user=self.user)

self.assertIsNotNone(cache.get(f"zaaktype:{self.zaaktype['url']}"))
self.assertIsNotNone(cache.get(f"zaaktype:{new_zaaktype['url']}"))
self.assertIsNotNone(cache.get(f"case_type:{self.zaaktype['url']}"))
self.assertIsNotNone(cache.get(f"case_type:{new_zaaktype['url']}"))

def test_status_types_are_cached(self, m):
self._setUpMocks(m)

# Cache is empty before the request
self.assertIsNone(cache.get(f"statustype:{self.status_type1['url']}"))
self.assertIsNone(cache.get(f"statustype:{self.status_type2['url']}"))
self.assertIsNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)
self.assertIsNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)

self.app.get(reverse("accounts:my_cases"), user=self.user)

# Case type is cached after the request
self.assertIsNotNone(cache.get(f"statustype:{self.status_type1['url']}"))
self.assertIsNotNone(cache.get(f"statustype:{self.status_type2['url']}"))
self.assertIsNotNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)
self.assertIsNotNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)

def test_cached_status_types_are_deleted_after_one_day(self, m):
self._setUpMocks(m)
Expand All @@ -355,20 +363,32 @@ def test_cached_status_types_are_deleted_after_one_day(self, m):

# After one day the results should be deleted
frozen_time.tick(delta=datetime.timedelta(hours=24))
self.assertIsNone(cache.get(f"statustype:{self.status_type1['url']}"))
self.assertIsNone(cache.get(f"statustype:{self.status_type2['url']}"))
self.assertIsNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)
self.assertIsNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)

def test_cached_status_types_in_combination_with_new_ones(self, m):
self._setUpMocks(m)

# First attempt
self.assertIsNone(cache.get(f"statustype:{self.status_type1['url']}"))
self.assertIsNone(cache.get(f"statustype:{self.status_type2['url']}"))
self.assertIsNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)
self.assertIsNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)

self.app.get(reverse("accounts:my_cases"), user=self.user)

self.assertIsNotNone(cache.get(f"statustype:{self.status_type1['url']}"))
self.assertIsNotNone(cache.get(f"statustype:{self.status_type2['url']}"))
self.assertIsNotNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)
self.assertIsNotNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)

# Second attempt with new case and status type
new_zaak = generate_oas_component(
Expand Down Expand Up @@ -440,18 +460,22 @@ def test_cached_status_types_in_combination_with_new_ones(self, m):
f"{ZAKEN_ROOT}statussen/3da81560-c7fc-476a-ad13-oie8u899923g",
json=new_status,
)
m.get(
f"{CATALOGI_ROOT}statustypen/e3798107-ab27-4c3c-977d-984yr8887rhe",
json=new_status_type,
)

self.assertIsNone(cache.get(f"statustype:{new_status_type['url']}"))
self.assertIsNone(
cache.get(f"status_types_for_case_type:{new_zaaktype['url']}")
)

self.app.get(reverse("accounts:my_cases"), user=self.user)

self.assertIsNotNone(cache.get(f"statustype:{new_status_type['url']}"))
self.assertIsNotNone(cache.get(f"statustype:{self.status_type1['url']}"))
self.assertIsNotNone(cache.get(f"statustype:{self.status_type2['url']}"))
self.assertIsNotNone(
cache.get(f"status_types_for_case_type:{new_zaaktype['url']}")
)
self.assertIsNotNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)
self.assertIsNotNone(
cache.get(f"status_types_for_case_type:{self.zaaktype['url']}")
)

def test_statuses_are_cached(self, m):
self._setUpMocks(m)
Expand Down
13 changes: 13 additions & 0 deletions src/open_inwoner/openzaak/utils.py
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,16 @@ def filter_info_object_visibility(


def cache(key: str, alias: str = "default", **set_options):
"""
Function-decorator for updating the django low-level cache.
It determines if the key exists in cache and skips it by calling the decorated function
or creates it if doesn't exist.
We can pass a keyword argument for the time we want the cache the data in
seconds (timeout=60).
"""

def decorator(func: callable):
argspec = inspect.getfullargspec(func)

Expand Down Expand Up @@ -67,10 +77,13 @@ def wrapped(*args, **kwargs):

_cache = caches[alias]
result = _cache.get(cache_key)

# The key exists in cache so we return the already cached data
if result is not None:
logger.debug("Cache key '%s' hit", cache_key)
return result

# The key does not exist so we call the decorated function and set the cache
result = func(*args, **kwargs)
_cache.set(cache_key, result, **set_options)

Expand Down

0 comments on commit 302efb8

Please sign in to comment.