getTableByPhpName() has incorrect fallback order #385

Closed
porcelijn opened this Issue Jun 19, 2012 · 5 comments

Projects

None yet

5 participants

@porcelijn

FIle propel/runtime/lib/map/DatabaseMap.php (167-)

 public function getTableByPhpName($phpName)
  {
    if (array_key_exists($phpName, $this->tablesByPhpName)) {
      return $this->tablesByPhpName[$phpName];
    } else if (class_exists($tmClass = $phpName . 'TableMap')) {
      $this->addTableFromMapClass($tmClass);
      return $this->tablesByPhpName[$phpName];
    } else if (class_exists($tmClass = substr_replace($phpName, '\\map\\', strrpos($phpName, '\\'), 1) . 'TableMap')) {
      $this->addTableFromMapClass($tmClass);
      return $this->tablesByPhpName[$phpName];
    } else {
      throw new PropelException("Cannot fetch TableMap for undefined table phpName: " . $phpName);
    }
  }

Here 2nd and 3rd conditional branches appear to be the wrong way around, as the more specific namespace-variant is hidden by the more general phpNameTableMap-variant

@fzaninotto
Member

Do you have an example of bug caused by this code?

@porcelijn

Hi Francois,

Thanks for your response, and sorry for the lack of unit tests to
accompany suggested change.
The problem I ran into was a regression in our software that spread out
over large parts of our system code base, spec. involving some hacks we
made in the Zend autoloader. I intend to strip the irrelevant parts from
the problem and create a minimal test subject. But this will take me
some time as I don't yet have a fully functional Propel GIT test setup
and live is full of other responsibilities. ;-)

tijn

On 2012-06-19 17:21, Francois Zaninotto wrote:

Do you have an example of bug caused by this code?


Reply to this email directly or view it on GitHub:
#385 (comment)

@havvg havvg added a commit to Ormigo/Propel that referenced this issue Jan 2, 2013
@havvg havvg Merge branch 'master' into ormigo
* master:
  Fixes #345
  Allow the QuickGenerator to use another Platform
  Change fallback order in getTableByPhpName(). Fixes #385
  Fix initialization of internal iterator for getRelCol. fix #460
  Add a test for initialization of internal iterator for related object collection getter fix #460
  Fix PR #544
  teaching propel to clear UP and DOWN when calling clearAllReferences(true) we have infinite recursion prevention already covered also we add an optional param to clearInstancePool. if passes as true it will clearAllReferences(true) on every instance before clearing it.
  Fix CS
  Fix cast in setters. Should fix #283
  Fix inconsistency for BIGINT. Fixes #459
  Use model prefix in QueryInheritanceBuilder, fixes #542
  fix variable name on boolean filter methods
  Tests for improved ENUM handling
  Improved getValueSet() method, added ENUM getters for SQL value, set Query filter to use SQL getters
  Add PHP 5.5 to travis config
  Update generator/lib/builder/om/PHP5PeerBuilder.php
  sqlType="enum(..)" now set the valueSet attribute it is not required to use Propel::ENUM as type
7d94f67
@ciromattia
Contributor

using class_exists() this earlier issues a problem if you have a later spl_autoload function registered that throws errors or exceptions. Using Prado framework, for example, stops at all the evaluation when a table named "AppProfile" is checked as "map\ppProfile" (with the backslash - winstyle?), Prado's autoloader tries to include the class issuing an exception - I think most autoloaders would behave the same.
ATM I solved setting class_exists() second parameter to false thus preventing autoload, but I'm wondering if this is good or not.

@staabm
Member
staabm commented Feb 26, 2013

@ciromattia could you explain what the problem is, I don't got it ...

PS: I deleted your double posted comment..

@ciromattia
Contributor

In my app's workflow I have three autoloaders registered: Propel, PropelAutoloader, and Prado.
Prado's one is simple an include_once(), thus issuing a PhpException if class does not exists.
I have a table named app_profile, php name AppProfile, and debugging I noticed that for some reasons that chunk of code calls class_exists() over map/ppProfile, trimming the trailing A; class_exists() has the autoload argument true by default, so PHP goes over the autoload stack eventually reaching the last one with the include_once() that issues the Exception.

PS: good, I didn't know if this issue was still followed so I commented in the code too :)

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