Skip to content

Commit

Permalink
pkp/pkp-lib#7111 Replace homebrew caching with Laravel tools
Browse files Browse the repository at this point in the history
  • Loading branch information
asmecher committed Jun 26, 2024
1 parent 6d9dc8d commit c0066ff
Show file tree
Hide file tree
Showing 2 changed files with 4 additions and 84 deletions.
60 changes: 0 additions & 60 deletions classes/issue/DAO.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,8 +22,6 @@
use Illuminate\Support\Collection;
use Illuminate\Support\Facades\DB;
use Illuminate\Support\LazyCollection;
use PKP\cache\CacheManager;
use PKP\cache\GenericCache;
use PKP\core\EntityDAO;
use PKP\core\traits\EntityWithParent;
use PKP\db\DAOResultFactory;
Expand All @@ -39,9 +37,6 @@ class DAO extends EntityDAO implements \PKP\plugins\PKPPubIdPluginDAO
*/
use EntityWithParent;

// TODO: Needs to be addressed with refactor of caching.
public $caches;

/** @copydoc EntityDAO::$schema */
public $schema = PKPSchemaService::SCHEMA_ISSUE;

Expand Down Expand Up @@ -85,42 +80,6 @@ public function getParentColumn(): string
return 'journal_id';
}

/**
* Handle a cache miss.
*
* TODO: Caching not currently working as expected
*
*/
public function _cacheMiss(GenericCache $cache, int $id): ?Issue
{
if ($cache->getCacheId() === 'current') {
$issue = Repo::issue()->getCurrent($id);
} else {
$issue = Repo::issue()->getByBestId($id, null, false);
}
$cache->setCache($id, $issue);
return $issue;
}

/**
* Get an issue cache by cache ID
*
* TODO: Not currently working as expected. Not used throughout current class
*
* @return mixed|object|\PKP\cache\APCCache|\PKP\cache\FileCache|GenericCache|\PKP\cache\MemcacheCache|\PKP\cache\XCacheCache
*/
public function _getCache(string $cacheId)
{
if (!isset($this->caches)) {
$this->caches = [];
}
if (!isset($this->caches[$cacheId])) {
$cacheManager = CacheManager::getManager();
$this->caches[$cacheId] = $cacheManager->getObjectCache('issues', $cacheId, $this->_cacheMiss(...));
}
return $this->caches[$cacheId];
}

/**
* Instantiate a new DataObject
*/
Expand Down Expand Up @@ -205,15 +164,13 @@ public function update(Issue $issue)
$issue->stampModified();
parent::_update($issue);
$this->resequenceCustomIssueOrders($issue->getData('journalId'));
// TODO: Flush cache
}

/** @copydoc EntityDAO::_delete() */
public function delete(Issue $issue)
{
parent::_delete($issue);
$this->resequenceCustomIssueOrders($issue->getData('journalId'));
// TODO: Flush cache
}

/**
Expand Down Expand Up @@ -375,8 +332,6 @@ public function changePubId($pubObjectId, $pubIdType, $pubId)
['issue_id' => (int) $pubObjectId, 'locale' => '', 'setting_name' => 'pub-id::' . $pubIdType],
['setting_value' => (string) $pubId]
);
// TODO: Cache not implemented
// $this->flushCache();
}

/**
Expand All @@ -393,8 +348,6 @@ public function deletePubId($pubObjectId, $pubIdType)
(int)$pubObjectId
]
);
// TODO: Cache not implemented
// $this->flushCache();
}

/**
Expand All @@ -414,8 +367,6 @@ public function deleteAllPubIds($contextId, $pubIdType)
]
);
}
// TODO: Cache not implemented
// $this->flushCache();
}

/**
Expand Down Expand Up @@ -460,17 +411,6 @@ public function getExportable($contextId, $pubIdType = null, $pubIdSettingName =
return new DAOResultFactory($result, $this, 'fromRow', [], $q, [], $rangeInfo);
}

/**
* Flush the issue cache.
*
* TODO: Not currently in use. _getCache always results in cache miss.
*/
public function flushCache()
{
$this->_getCache('issues')->flush();
$this->_getCache('current')->flush();
}

/**
* Set the DOI object
*
Expand Down
28 changes: 4 additions & 24 deletions classes/issue/Repository.php
Original file line number Diff line number Diff line change
Expand Up @@ -64,21 +64,11 @@ public function exists(int $id, ?int $journalId = null): bool
return $this->dao->exists($id, $journalId);
}

/** @copydoc DAO::get()
* TODO: Function signature should stick with ID, but previous DAO expected $useCache = false as default
/**
* @copydoc DAO::get()
*/
public function get(int $id, ?int $journalId = null): ?Issue
{
// TODO: Caching as currently setup never properly caches objects and always fires a _cacheMiss()
// if ($useCache) {
// $cache = $this->dao->_getCache('issues');
// $returner = $cache->get($id);
// if ($returner && $contextId != null && $contextId != $returner->getJournalId()) {
// $returner = null;
// }
// return $returner;
// }

return $this->dao->get($id, $journalId);
}

Expand Down Expand Up @@ -201,17 +191,9 @@ public function deleteMany(Collector $collector)

/**
* Retrieve current issue
*
* @param bool $useCache TODO: Not currently implemented. Adding to preserved desired cache usage in future
*/
public function getCurrent(int $contextId, bool $useCache = false): ?Issue
public function getCurrent(int $contextId): ?Issue
{
// TODO: Caching as currently setup never properly caches objects and always fires a _cacheMiss()
// if ($useCache) {
// $cache = $this->dao->_getCache('current');
// return $cache->get($contextId);
// }

/** @var JournalDAO $journalDao */
$journalDao = DAORegistry::getDAO('JournalDAO');

Expand Down Expand Up @@ -256,10 +238,8 @@ public function getBySubmissionId(int $submissionId): ?Issue
/**
* Retrieve Issue by "best" issue id -- url path if it exists,
* falling back on the internal issue ID otherwise.
*
* @param bool $useCache TODO: Carryover from IssueDAO—was not in use
*/
public function getByBestId(string $idOrUrlPath, ?int $contextId = null, bool $useCache = false): ?Issue
public function getByBestId(string $idOrUrlPath, ?int $contextId = null): ?Issue
{
// Get the issue that matches the requested urlPath (if $idOrUrlPath is one)
return ctype_digit((string) $idOrUrlPath)
Expand Down

0 comments on commit c0066ff

Please sign in to comment.