Skip to content

[DDC-1766] Initial implementation of hydration cache. #329

Merged
merged 9 commits into from Apr 6, 2012

4 participants

@beberlei
Doctrine member
beberlei commented Apr 4, 2012

No description provided.

@guilhermeblanco guilhermeblanco commented on the diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
@@ -21,7 +21,8 @@
use Doctrine\DBAL\Types\Type,
@guilhermeblanco
Doctrine member

Is it possible for us to split the "use"s in logical blocks? Like Doctrine\DBAL and Doctrine\ORM ones?

@Ocramius
Doctrine member
Ocramius added a note Apr 5, 2012

Also (not very important right now) PSR-1 says one use statement per row.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+ * @example
+ * $lifetime = 100;
+ * $resultKey = "abc";
+ * $query->setHydrationCacheProfile(new QueryCacheProfile());
+ * $query->setHydrationCacheProfile(new QueryCacheProfile($lifetime, $resultKey));
+ *
+ * @param \Doctrine\DBAL\Cache\QueryCacheProfile $profile
+ * @return \Doctrine\ORM\AbstractQuery
+ */
+ public function setHydrationCacheProfile(QueryCacheProfile $profile = null)
+ {
+ if ( ! $profile->getResultCacheDriver()) {
+ $profile = $profile->setResultCacheDriver($this->_em->getConfiguration()->getResultCacheImpl());
+ }
+
+ $this->_hydrationCacheProfile = $profile;
@guilhermeblanco
Doctrine member

Add a line space between assignments and control structures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+ * some form of caching with UnitOfWork registration you should use
+ * {@see AbstractQuery::setResultCacheProfile()}.
+ *
+ * @example
+ * $lifetime = 100;
+ * $resultKey = "abc";
+ * $query->setHydrationCacheProfile(new QueryCacheProfile());
+ * $query->setHydrationCacheProfile(new QueryCacheProfile($lifetime, $resultKey));
+ *
+ * @param \Doctrine\DBAL\Cache\QueryCacheProfile $profile
+ * @return \Doctrine\ORM\AbstractQuery
+ */
+ public function setHydrationCacheProfile(QueryCacheProfile $profile = null)
+ {
+ if ( ! $profile->getResultCacheDriver()) {
+ $profile = $profile->setResultCacheDriver($this->_em->getConfiguration()->getResultCacheImpl());
@guilhermeblanco
Doctrine member

Split the operation in 2 lines:

$resultCache = $this->_em->getConfiguration()->getResultCacheImpl();
$profile     = $profile->setResultCacheDriver($resultCache);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+ /**
+ * Set a cache profile for the result cache.
+ *
+ * If no result cache driver is set in the QueryCacheProfile, the default
+ * result cache driver is used from the configuration.
+ *
+ * @param \Doctrine\DBAL\Cache\QueryCacheProfile $profile
+ * @return \Doctrine\ORM\AbstractQuery
+ */
+ public function setResultCacheProfile(QueryCacheProfile $profile = null)
+ {
+ if ( ! $profile->getResultCacheDriver()) {
+ $profile = $profile->setResultCacheDriver($this->_em->getConfiguration()->getResultCacheImpl());
+ }
+
+ $this->_queryCacheProfile = $profile;
@guilhermeblanco
Doctrine member

Line space between assignments and control structures.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+ return $this->_hydrationCacheProfile;
+ }
+
+ /**
+ * Set a cache profile for the result cache.
+ *
+ * If no result cache driver is set in the QueryCacheProfile, the default
+ * result cache driver is used from the configuration.
+ *
+ * @param \Doctrine\DBAL\Cache\QueryCacheProfile $profile
+ * @return \Doctrine\ORM\AbstractQuery
+ */
+ public function setResultCacheProfile(QueryCacheProfile $profile = null)
+ {
+ if ( ! $profile->getResultCacheDriver()) {
+ $profile = $profile->setResultCacheDriver($this->_em->getConfiguration()->getResultCacheImpl());
@guilhermeblanco
Doctrine member

Split the operation in 2 lines:

$resultCache = $this->_em->getConfiguration()->getResultCacheImpl();
$profile     = $profile->setResultCacheDriver($resultCache);
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on the diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
@@ -644,15 +708,73 @@ public function execute($params = array(), $hydrationMode = null)
$this->setParameters($params);
}
+ $setCacheEntry = function() {};
@guilhermeblanco
Doctrine member

Line space between assignment and control structures.

@guilhermeblanco
Doctrine member

WTH is that?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
@@ -644,15 +708,73 @@ public function execute($params = array(), $hydrationMode = null)
$this->setParameters($params);
}
+ $setCacheEntry = function() {};
+ if ($this->_hydrationCacheProfile !== null) {
+ list($cacheKey, $realCacheKey) = $this->getHydrationCacheId();
+
+ $qcp = $this->getHydrationCacheProfile();
@guilhermeblanco
Doctrine member

Align the assignment blocks "=" signs for better code readability.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
@@ -644,15 +708,73 @@ public function execute($params = array(), $hydrationMode = null)
$this->setParameters($params);
}
+ $setCacheEntry = function() {};
+ if ($this->_hydrationCacheProfile !== null) {
+ list($cacheKey, $realCacheKey) = $this->getHydrationCacheId();
+
+ $qcp = $this->getHydrationCacheProfile();
+ $cache = $qcp->getResultCacheDriver();
+
+ $result = $cache->fetch($cacheKey);
@guilhermeblanco
Doctrine member

This LOC is locally part of the previous block. Remove the line space previous to this line and add one after.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on the diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
$stmt = $this->_doExecute();
if (is_numeric($stmt)) {
+ $setCacheEntry($stmt);
@guilhermeblanco
Doctrine member

Line space.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on the diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+
+ $setCacheEntry($data);
+
+ return $data;
+ }
+
+ /**
+ * Get the result cache id to use to store the result set cache entry.
+ * Will return the configured id if it exists otherwise a hash will be
+ * automatically generated for you.
+ *
+ * @return array ($key, $hash)
+ */
+ protected function getHydrationCacheId()
+ {
+ $params = $this->getParameters();
@guilhermeblanco
Doctrine member

Line space between assignment and control structure.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+
+ return $data;
+ }
+
+ /**
+ * Get the result cache id to use to store the result set cache entry.
+ * Will return the configured id if it exists otherwise a hash will be
+ * automatically generated for you.
+ *
+ * @return array ($key, $hash)
+ */
+ protected function getHydrationCacheId()
+ {
+ $params = $this->getParameters();
+ foreach ($params AS $key => $value) {
+ if (is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(get_class($value))) {
@guilhermeblanco
Doctrine member

Early return can be applied here to reduce one indentation level.

foreach (...) {
    if ( ! (is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(get_class($value)))) {
        continue;
    }

    // ...
}
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on the diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
$stmt, $this->_resultSetMapping, $this->_hints
);
+
+ $setCacheEntry($data);
+
+ return $data;
+ }
+
+ /**
+ * Get the result cache id to use to store the result set cache entry.
+ * Will return the configured id if it exists otherwise a hash will be
+ * automatically generated for you.
+ *
+ * @return array ($key, $hash)
+ */
@guilhermeblanco
Doctrine member

I'd rather encapsulate this block in a separate function.

protected function getEntityIdentifierValueList($entity)
{
    $unitOfWork = $this->_em->getUnitOfWork();

    if ($unitOfWork->getEntityState($entity) === UnitOfWork::STATE_MANAGED) {
        return $unitOfWork->getEntityIdentifier($entity);
    }

    $classMetadata = $this->_em->getClassMetadata(get_class($entity));

    return $classMetadata->getIdentifierValues($entity);
}

Then just reuse it here.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+ * Get the result cache id to use to store the result set cache entry.
+ * Will return the configured id if it exists otherwise a hash will be
+ * automatically generated for you.
+ *
+ * @return array ($key, $hash)
+ */
+ protected function getHydrationCacheId()
+ {
+ $params = $this->getParameters();
+
+ foreach ($params AS $key => $value) {
+ if (is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(get_class($value))) {
+ if ($this->_em->getUnitOfWork()->getEntityState($value) == UnitOfWork::STATE_MANAGED) {
+ $idValues = $this->_em->getUnitOfWork()->getEntityIdentifier($value);
+ } else {
+ $class = $this->_em->getClassMetadata(get_class($value));
@guilhermeblanco
Doctrine member

Align "=" signs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+ *
+ * @return array ($key, $hash)
+ */
+ protected function getHydrationCacheId()
+ {
+ $params = $this->getParameters();
+
+ foreach ($params AS $key => $value) {
+ if (is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(get_class($value))) {
+ if ($this->_em->getUnitOfWork()->getEntityState($value) == UnitOfWork::STATE_MANAGED) {
+ $idValues = $this->_em->getUnitOfWork()->getEntityIdentifier($value);
+ } else {
+ $class = $this->_em->getClassMetadata(get_class($value));
+ $idValues = $class->getIdentifierValues($value);
+ }
+ $params[$key] = $idValues;
@guilhermeblanco
Doctrine member

Line space before

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@guilhermeblanco guilhermeblanco and 1 other commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+ */
+ protected function getHydrationCacheId()
+ {
+ $params = $this->getParameters();
+
+ foreach ($params AS $key => $value) {
+ if (is_object($value) && $this->_em->getMetadataFactory()->hasMetadataFor(get_class($value))) {
+ if ($this->_em->getUnitOfWork()->getEntityState($value) == UnitOfWork::STATE_MANAGED) {
+ $idValues = $this->_em->getUnitOfWork()->getEntityIdentifier($value);
+ } else {
+ $class = $this->_em->getClassMetadata(get_class($value));
+ $idValues = $class->getIdentifierValues($value);
+ }
+ $params[$key] = $idValues;
+ }
+ }
@guilhermeblanco
Doctrine member

Do not abbreviate. What's qcp? Quebec Cops Party?

@Ocramius
Doctrine member
Ocramius added a note Apr 5, 2012

I lol'd hard X°D

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@stof stof and 1 other commented on an outdated diff Apr 4, 2012
lib/Doctrine/ORM/AbstractQuery.php
+ * some form of caching with UnitOfWork registration you should use
+ * {@see AbstractQuery::setResultCacheProfile()}.
+ *
+ * @example
+ * $lifetime = 100;
+ * $resultKey = "abc";
+ * $query->setHydrationCacheProfile(new QueryCacheProfile());
+ * $query->setHydrationCacheProfile(new QueryCacheProfile($lifetime, $resultKey));
+ *
+ * @param \Doctrine\DBAL\Cache\QueryCacheProfile $profile
+ * @return \Doctrine\ORM\AbstractQuery
+ */
+ public function setHydrationCacheProfile(QueryCacheProfile $profile = null)
+ {
+ if ( ! $profile->getResultCacheDriver()) {
+ $resultCacheDriver = $this->_em->getConfiguration()->getResultCacheImpl();
@stof
Doctrine member
stof added a note Apr 4, 2012

is it expected to use the ResultCache implementation for the hydration cache too ? Currently the place using caching in the ORM are using separate cache implementations

@Ocramius
Doctrine member
Ocramius added a note Apr 5, 2012

Indeed, a $hydrationCacheDriver could be a nice addition to Doctrine\ORM\Configuration.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ocramius Ocramius commented on the diff Apr 5, 2012
lib/Doctrine/ORM/AbstractQuery.php
@@ -102,6 +103,11 @@
protected $_expireResultCache = false;
/**
+ * @param \Doctrine\DBAL\Cache\QueryCacheProfile
+ */
+ protected $_hydrationCacheProfile;
@Ocramius
Doctrine member
Ocramius added a note Apr 5, 2012

Just wondering about the _. When will this be dropped?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@beberlei
Doctrine member
beberlei commented Apr 5, 2012

@guilhermeblanco please see last commit 0b3577f and merge if everything ok now. From me this is finished, and then i rebase this into 2.2

@guilhermeblanco
Doctrine member

@beberlei seems perfect! Feel free to merge! =D

@guilhermeblanco guilhermeblanco merged commit a5c13a5 into master Apr 6, 2012
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Something went wrong with that request. Please try again.