Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

Already on GitHub? Sign in to your account

Fixed #613. Proves the issue described in #613 and added a kind of fix for it. #620

Closed
wants to merge 4 commits into
from

Conversation

Projects
None yet
3 participants
Owner

marcj commented Feb 25, 2013

That added method shouldn't be used in other internal code, but for the scenario mentioned in #613 it should be one of the most elegant ways.

Owner

marcj commented Feb 25, 2013

@staabm, @jaugustin, after a review it should be ready to merge.

@staabm staabm commented on an outdated diff Feb 25, 2013

runtime/lib/Propel.php
@@ -381,6 +381,33 @@ public static function getConfiguration($type = PropelConfiguration::TYPE_ARRAY)
return self::$configuration->getParameters($type);
}
+
+ /**
+ * This makes sure all model peers have registered their table to Propel's core.
+ * E.g. through a `unserialization` of a standalone Criteria it's not automatically
+ * known which tables belongs to which model classes.
+ *
+ * This call can be quite slow and can have a big memory consumption, since
+ * it autoLoads/parses all model class that belongs to the current database
@staabm

staabm Feb 25, 2013

Member

typo auto_L_oad

@staabm staabm commented on an outdated diff Feb 25, 2013

runtime/lib/Propel.php
@@ -381,6 +381,33 @@ public static function getConfiguration($type = PropelConfiguration::TYPE_ARRAY)
return self::$configuration->getParameters($type);
}
+
+ /**
+ * This makes sure all model peers have registered their table to Propel's core.
+ * E.g. through a `unserialization` of a standalone Criteria it's not automatically
+ * known which tables belongs to which model classes.
+ *
+ * This call can be quite slow and can have a big memory consumption, since
+ * it autoLoads/parses all model class that belongs to the current database
+ * (defined through `Propel::setConfiguration` or `Propel::init`).
+ *
+ * If you know which tables (and their class name) are used in the Criteria,
+ * then you should better trigger only the appropriate `*Peer::buildTableMap()`
+ * to make the `table => model class` known. If not, this method is the only
+ * wait to make all `tables to model classes` relation available to Propel.
+ *
@staabm

staabm Feb 25, 2013

Member

unnecessary newline

@staabm staabm commented on the diff Feb 25, 2013

test/testsuite/misc/Issue613/Issue613Test.php
+
+/**
+ * Proves the issue described in #613 and the fix for it.
+ *
+ * Basically it describes that a standalone Criteria does not
+ * contain the information which table belongs to which model class.
+ * We've added with this issue a new method `Propel::buildAllTableMaps` which
+ * can help. See the description of this method for more information.
+ *
+ * @see https://github.com/propelorm/Propel/issues/613
+ */
+class Issue613Test extends BookstoreTestBase
+{
+
+ /**
+ * Creates a temp file with a serialized Criteria object.
@staabm

staabm Feb 25, 2013

Member

missing doc

@marcj

marcj Feb 25, 2013

Owner

I see docs?

@staabm

staabm Feb 25, 2013

Member

Oh... this line was cropped at first.. but now it looks good...

Member

staabm commented Feb 25, 2013

despite the few nits, LGTM

Owner

marcj commented Feb 25, 2013

@jaugustin, your turn.

@marcj marcj closed this Mar 4, 2013

Owner

willdurand commented Mar 4, 2013

Why did you close this PR?

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment