load_entity: disable_cache=True enables the cache when it was disabled #186

OSW.load_entity accepts LoadEntityParam.disable_cache, documented at src/osw/core.py:1159-1160 as "If true, disable the cache for the loading process". With disable_cache=True the cache is not disabled for the duration of the load. It is set to the opposite of the caller's starting state.

Code

src/osw/core.py:1222-1228:

# store original cache state
cache_state = self.site.get_cache_enabled()
if param.disable_cache:
    self.site.disable_cache()
if not cache_state and param.disable_cache:
    # enable cache to speed up loading
    self.site.enable_cache()

The second condition re-enables the cache exactly when the caller had it disabled. The two statements cancel each other.

Measured

Against a fake site that records the state when get_page is called, with disable_cache=True:

The state after the call is correct in both rows, so the restore works. The state during the load is inverted in the first row.

Consequence

A caller that already has the cache disabled and passes disable_cache=True to force a fresh read gets a cached read instead. That is the failure mode of #176: a get_page served from the cache can return a revision from before a write.

Open question

The comment "enable cache to speed up loading" suggests the second block was written on purpose. If that is the intended behaviour, then the field name and its docstring are wrong rather than the code, and the fix is to rename or document the field instead of removing the block. A maintainer decision is needed on which of the two is intended.

Related

#183 and #185 move the cache restore into a finally block. That PR deliberately leaves the behaviour described here unchanged.