Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with
or
.
Download ZIP

Loading…

Add a filter to the import and convert mapping #603

Closed
wants to merge 2 commits into from

5 participants

@nicolasThal

Hi,

I needed to import the mapping of an existing database in order to create doctrine2 entities, but when you run the command, it throw an exception on every table without a primary key.

I tried to use the filter option, but the error still occurs because the exception was throw in the method which get all the metadata informations.

I added a method which get the metadata of some tables according to a filter.

What are your thought bout my issue?

@doctrinebot
Collaborator

Hello,

thank you for positing this Pull Request. I have automatically opened an issue on our Jira Bug Tracker for you with the details of this Pull-Request. See the Link:

http://doctrine-project.org/jira/browse/DDC-2335

lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php
((13 lines not shown))
- foreach ($this->_sm->listTableNames() as $tableName) {
+ if (empty($filters)) {
+ $listTableNamesToUse = $listTableNames;
+ } else {
+ foreach ($listTableNames as $tableName) {
+ foreach ($filters as $regex) {
+ if (strpos($regex, $tableName) !== false){
@stof
stof added a note

A regex checked with strpos ? Please don't call it a regex then

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...e/ORM/Tools/Console/Command/ConvertMappingCommand.php
@@ -125,8 +125,8 @@ protected function execute(InputInterface $input, OutputInterface $output)
$cmf = new DisconnectedClassMetadataFactory();
$cmf->setEntityManager($em);
- $metadata = $cmf->getAllMetadata();
- $metadata = MetadataFilter::filter($metadata, $input->getOption('filter'));
+ $metadata = $cmf->getAllFilteredMetadata($input->getOption('filter'));
+// $metadata = MetadataFilter::filter($metadata, $input->getOption('filter'));
@stof
stof added a note

Old code should be removed, not commented

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php
@@ -347,6 +360,20 @@ public function getAllClassNames()
}
/**
+ * Gets the names of all mapped classes known to this driver and filter them by the pattern in filter.
+ *
+ * @param array $filter
+ * @return array The names of all mapped classes known to this driver.
+ */
+ function getFilteredClassNames(array $filter = array())
@stof
stof added a note

missing visibility

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

Hi guys,

Can I have your feedback on my pull-request?

Thank you very much!

@Ocramius Ocramius commented on the diff
lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php
@@ -109,15 +109,28 @@ public function setTables($entityTables, $manyToManyTables)
*
* @throws \Doctrine\ORM\Mapping\MappingException
*/
- private function reverseEngineerMappingFromDatabase()
+ private function reverseEngineerMappingFromDatabase(array $filters = array())
{
if ($this->tables !== null) {
return;
}
$tables = array();
@Ocramius Owner

Align = signs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ocramius Ocramius commented on the diff
lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php
((13 lines not shown))
- foreach ($this->_sm->listTableNames() as $tableName) {
+ if (empty($filters)) {
+ $listTableNamesToUse = $listTableNames;
@Ocramius Owner

You can avoid swapping the variables and simply skip over

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ocramius Ocramius commented on the diff
lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php
((13 lines not shown))
- foreach ($this->_sm->listTableNames() as $tableName) {
+ if (empty($filters)) {
+ $listTableNamesToUse = $listTableNames;
+ } else {
+ foreach ($listTableNames as $tableName) {
+ foreach ($filters as $haystack) {
+ if (strpos($haystack, $tableName) !== false){
+ $listTableNamesToUse[] = $tableName;
+ break;
@Ocramius Owner

Un-needed break (performance, ok, but not really necessary)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@Ocramius Ocramius commented on the diff
lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php
((13 lines not shown))
- foreach ($this->_sm->listTableNames() as $tableName) {
+ if (empty($filters)) {
+ $listTableNamesToUse = $listTableNames;
+ } else {
+ foreach ($listTableNames as $tableName) {
+ foreach ($filters as $haystack) {
+ if (strpos($haystack, $tableName) !== false){
+ $listTableNamesToUse[] = $tableName;
+ break;
+ }
+ }
+ }
+ }
+ foreach ( $listTableNamesToUse as $tableName) {
@Ocramius Owner

Missing empty newline before this line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@beberlei beberlei commented on the diff
lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php
@@ -347,6 +360,20 @@ public function getAllClassNames()
}
/**
+ * Gets the names of all mapped classes known to this driver and filter them by the pattern in filter.
+ *
+ * @param array $filter
+ * @return array The names of all mapped classes known to this driver.
+ */
+ public function getFilteredClassNames(array $filter = array())
@beberlei Owner

where is this used?

@Ocramius Owner

I think this was meant to be getAllFilteredMetadata

I haven't seen that I haven't made a pull request for a portion of my modification.
In the doctrine-common, I added a method which uses this method.
doctrine/common#266

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

This is the wrong approach, you should use the $config object to filter DBAL assets out of the SchemaManager. It has a function setFilterSchemaAssetsExpression that applies globally.

@beberlei beberlei closed this
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Commits on Mar 6, 2013
  1. Add a filter to the import and convert mapping

    nicolasTheodo authored
  2. Code style modification after @stof comments

    nicolasTheodo authored
This page is out of date. Refresh to see the latest.
View
31 lib/Doctrine/ORM/Mapping/Driver/DatabaseDriver.php
@@ -109,15 +109,28 @@ public function setTables($entityTables, $manyToManyTables)
*
* @throws \Doctrine\ORM\Mapping\MappingException
*/
- private function reverseEngineerMappingFromDatabase()
+ private function reverseEngineerMappingFromDatabase(array $filters = array())
{
if ($this->tables !== null) {
return;
}
$tables = array();
@Ocramius Owner

Align = signs

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ $listTableNames = $this->_sm->listTableNames();
- foreach ($this->_sm->listTableNames() as $tableName) {
+ if (empty($filters)) {
+ $listTableNamesToUse = $listTableNames;
@Ocramius Owner

You can avoid swapping the variables and simply skip over

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ } else {
+ foreach ($listTableNames as $tableName) {
+ foreach ($filters as $haystack) {
+ if (strpos($haystack, $tableName) !== false){
+ $listTableNamesToUse[] = $tableName;
+ break;
@Ocramius Owner

Un-needed break (performance, ok, but not really necessary)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ }
+ }
+ }
+ }
+ foreach ( $listTableNamesToUse as $tableName) {
@Ocramius Owner

Missing empty newline before this line

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
$tables[$tableName] = $this->_sm->listTableDetails($tableName);
}
@@ -347,6 +360,20 @@ public function getAllClassNames()
}
/**
+ * Gets the names of all mapped classes known to this driver and filter them by the pattern in filter.
+ *
+ * @param array $filter
+ * @return array The names of all mapped classes known to this driver.
+ */
+ public function getFilteredClassNames(array $filter = array())
@beberlei Owner

where is this used?

@Ocramius Owner

I think this was meant to be getAllFilteredMetadata

I haven't seen that I haven't made a pull request for a portion of my modification.
In the doctrine-common, I added a method which uses this method.
doctrine/common#266

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
+ {
+ $this->reverseEngineerMappingFromDatabase($filter);
+
+ return array_keys($this->classToTableNames);
+ }
+
+
+ /**
* Sets class name for a table.
*
* @param string $tableName
View
3  lib/Doctrine/ORM/Tools/Console/Command/ConvertMappingCommand.php
@@ -125,8 +125,7 @@ protected function execute(InputInterface $input, OutputInterface $output)
$cmf = new DisconnectedClassMetadataFactory();
$cmf->setEntityManager($em);
- $metadata = $cmf->getAllMetadata();
- $metadata = MetadataFilter::filter($metadata, $input->getOption('filter'));
+ $metadata = $cmf->getAllFilteredMetadata($input->getOption('filter'));
// Process destination directory
if ( ! is_dir($destPath = $input->getArgument('dest-path'))) {
Something went wrong with that request. Please try again.