propelorm/Propel#205

/cc @themouette

ping @fzaninotto

It seems unused but it as a dependency to Phing...

Hi all,

I would like to know how to configure and run the tests suite of Propel. I have specified all my db identifiers in build.properties and runtime-conf.xml files in tests/Fixtures.

But when I prepare the tests suite with the reset_tests exectable file, I got the following error:

Execution of target "check-buildprops-for-propel-gen" failed for the following reason: /Users/Hugo/Development/Propel2/tools/generator/build.xml:95:15: No project directory specified.

BUILD FAILED
/Users/Hugo/Development/Propel2/tools/generator/build.xml:95:15: No project directory specified.
Total time: 0.1338 seconds

I suppose I need to change something in tools/generator directory but what???

Any clue to help me configure the tests suite please?

Thanks.

After finding out where to put my database credentials I ran into a blocking issue with the reset_tests.sh script:

propel > sql-template:                                                                                                                                                                                      

[propel-sql] Loading XML schema files...
[propel-sql] 8 tables found in 6 schema files.
PHP Fatal error:  Call to a member function addTable() on a non-object in Propel2/src/Propel/Generator/Task/PropelSQLTask.php on line 255
PHP Stack trace:
PHP   1. {main}() /usr/share/php5/PEAR/phing.php:0
PHP   2. Phing::fire() /usr/share/php5/PEAR/phing.php:37
PHP   3. Phing::start() /usr/share/php5/PEAR/phing/Phing.php:270
PHP   4. Phing->runBuild() /usr/share/php5/PEAR/phing/Phing.php:170
PHP   5. Project->executeTargets() /usr/share/php5/PEAR/phing/Phing.php:564
PHP   6. Project->executeTarget() /usr/share/php5/PEAR/phing/Project.php:706
PHP   7. Target->performTasks() /usr/share/php5/PEAR/phing/Project.php:733
PHP   8. Target->main() /usr/share/php5/PEAR/phing/Target.php:300
PHP   9. Task->perform() /usr/share/php5/PEAR/phing/Target.php:277
PHP  10. PhingTask->main() /usr/share/php5/PEAR/phing/Task.php:253
PHP  11. PhingTask->processFile() /usr/share/php5/PEAR/phing/tasks/system/PhingTask.php:150
PHP  12. Project->executeTarget() /usr/share/php5/PEAR/phing/tasks/system/PhingTask.php:275
PHP  13. Target->performTasks() /usr/share/php5/PEAR/phing/Project.php:733
PHP  14. Target->main() /usr/share/php5/PEAR/phing/Target.php:300
PHP  15. Task->perform() /usr/share/php5/PEAR/phing/Target.php:277
PHP  16. PhingCallTask->main() /usr/share/php5/PEAR/phing/Task.php:253
PHP  17. PhingTask->main() /usr/share/php5/PEAR/phing/tasks/system/PhingCallTask.php:158
PHP  18. PhingTask->processFile() /usr/share/php5/PEAR/phing/tasks/system/PhingTask.php:150
PHP  19. Project->executeTarget() /usr/share/php5/PEAR/phing/tasks/system/PhingTask.php:275
PHP  20. Target->performTasks() /usr/share/php5/PEAR/phing/Project.php:733
PHP  21. Target->main() /usr/share/php5/PEAR/phing/Target.php:300
PHP  22. Task->perform() /usr/share/php5/PEAR/phing/Target.php:277
PHP  23. PhingCallTask->main() /usr/share/php5/PEAR/phing/Task.php:253
PHP  24. PhingTask->main() /usr/share/php5/PEAR/phing/tasks/system/PhingCallTask.php:158
PHP  25. PhingTask->processFile() /usr/share/php5/PEAR/phing/tasks/system/PhingTask.php:150
PHP  26. Project->executeTarget() /usr/share/php5/PEAR/phing/tasks/system/PhingTask.php:275
PHP  27. Target->performTasks() /usr/share/php5/PEAR/phing/Project.php:733
PHP  28. Target->main() /usr/share/php5/PEAR/phing/Target.php:300
PHP  29. Task->perform() /usr/share/php5/PEAR/phing/Target.php:277
PHP  30. Propel\Generator\Task\PropelSQLTask->main() /usr/share/php5/PEAR/phing/Task.php:253
PHP  31. Propel\Generator\Task\PropelSQLTask->packageDataModels() Propel2/src/Propel/Generator/Task/PropelSQLTask.php:140

For those not wanting to search: credentials have to be set in:

• tests/Fixtures/bookstore/build.properties (uncomment lines 21 and 22 for mysql)
• tests/Fixtures/bookstore/runtime-conf.xml (uncomment user and password blocks for mysql)

The same goes for the other test configurations, but at the moment it seems I'm stuck with the bookstore.

The classes under the Generator/Model/ directory help to model the relational schema. The problem is that at buildtime, there is no model of the object-oriented domain. The code necessary to generate ActiveRecord, TableMap, RelationMap, is all inside Relational classdes - or worse, builder classes.

I suggest that we introduce an object-oriented view of the model at buildtime - something like the runtime Map classes, but more intelligent - and to make these classes the primary data for PHP builders. SQL builders can keep using relational models, but PHP builders need a central place where they can look for a reference of the Object model.

propelorm/Propel#207

/cc @havvg

propelorm/Propel#234

/cc @yitznewton

Regression was introduced by commit bec8a58 (thanks, git bisect).

$ phpunit tests/Propel/Tests/Runtime/Collection/ArrayCollectionTest.php 
PHPUnit 3.5.14 by Sebastian Bergmann.

......F..

Time: 0 seconds, Memory: 25.25Mb

There was 1 failure:

1) Propel\Tests\Runtime\Collection\ArrayCollectionTest::testToArray
Failed asserting that two arrays are equal.
--- Expected
+++ Actual
@@ @@
 Array
 (
     [AuthorId] => 29
     [ISBN] => 043935806X
     [Id] => 27
-    [Price] => 10.99
+    [Price] => 10.989999771118
     [PublisherId] => 28
     [Title] => Harry Potter and the Order of the Phoenix
 )

/path/to/Propel/Tests/Runtime/Collection/ArrayCollectionTest.php:150

FAILURES!
Tests: 9, Assertions: 20, Failures: 1.

Note that some systems don't have the bug.

Propel 1.x branch stores date and time values as strings internally in the generated objects.

The historical reason for that was the lack of DateTime serialization in PHP < 5.3. Now when we are 5.3+ only, we could store the datetimes internaly as DateTimes.

Why is this so important?

• Make setters / getters more simple, no need to handle the string representation serialization
• Intuition. #48 makes the getters always return DateTime instances. Then you will end up doing stuff like

  $foo->getDate()->modify('+1 days'); $foo->save(); 

  and you would expect it to end up to the database modified. Currently it doesn't modify anything.

Ported from: propelorm/Propel#152

I get error

PHP Fatal error:  Class Propel\Runtime\Adapter\Pdo\PdoConnection contains 1 abstract method and must therefore be declared abstract or implement the remaining methods (Propel\Runtime\Connection\ConnectionInterface::inTransaction) in /vagrant/src/Propel/Runtime/Adapter/Pdo/PdoConnection.php on line 52

When trying to run the test suite.

@francois it seems to have something to do with your PR #64 maybe

Related to commit: 7986e50

(Oh, and PR it next time :))

Hi,

I looked at the Connection namespace and I have some suggestions on how to improve these classes. First of all, I'm a bit suprised by the number of classes to handle connection. Maybe we could remove some of them, but anyway this is not the topic of this RFC.

The ConnectionWrapper class does too much thing and I suggest to remove all calls related to debug. It will simplify the code a lot and it will certainly add a small speed boost in production as they won't have any conditions on the isDebug property.

What I suggest is to keep the ConnectionWrapper as simple as possible by removing all debugging stuff and overriding methods in an AdvancedConnectionWrapper or DebugConnectionWrapper or ProfiledConnectionWrapper class (which actually corresponds to the DebugPDO class).

We have two ways of doing this: inheritance or decoration with composition.

Inheritance way:

class ConnectionWrapper
{
    public function exec($sql)
    {
        return $this->connection->exec($sql);
    }
}

class ProfiledConnectionWrapper extends ConnectionWrapper
{
    public function exec($sql)
    {
        $debug = $this->getDebugSnapshot();

        $return = parent::exec($sql);

        $this->log($sql, null, 'exec', $debug);
        $this->setLastExecutedQuery($sql);
        $this->incrementQueryCount();

        return $return;
    }
}

So in production we just need to instanciate the ConnectionWrapper class or use the other in development mode:

// Production
$conn = new ConnectionWrapper(new \PDO());

// Development
$conn = new ProfiledConnectionWrapper(new \PDO(), array(
    'foo' => 'bar'  // Options to configure what we want to profile
));

This ProfiledConnectionWrapper connection will also be able to receive an injected Monolog dependency, so that the main ConnectionWrapper keeps remain free of any debug things.

Or we can do it by composition:

class ProfiledConnectionWrapper implements ConnectionWrapperInterface
{
    private $conn;

    public function __construct(ConnectionWrapperInterface $conn)
    {
        $this->conn = $conn;
    }

    public function exec($sql)
    {
        $debug = $this->getDebugSnapshot();

        $return = $this->conn->exec($sql);

        $this->log($sql, null, 'exec', $debug);
        $this->setLastExecutedQuery($sql);
        $this->incrementQueryCount();

        return $return;
    }
}

So, finally we would have the following code:

// Production
$conn = new ConnectionWrapper(new \PDO());

// Development
$conn = new ProfiledConnectionWrapper(new ConnectionWrapper(new \PDO()), array(
    'foo' => 'bar'  // Options to configure what we want to profile
));

What do you think about this RFC?

Cheers.

We have to remove the DefaultEnglishPluralizer in order to use the StandardEnglishPluralizer only but the Runtime has to be checked to track all $string.'s' and so on.

The Propel2 Roadmap

Please find the roadmap as discussed during the IRC Meeting held on Wednesday, 5th October 2011:

• Removing all require() and adding namespaces, that means the directory structure will be modified to follow the PSR-0 specification. OK
• Adding an autoloader component, probably the Symfony2 ClassLoader Component. OK
• Fixing CS. OK
• Fixing naming. OK
• Removing Phing and related stuffs.
• Removing late-static-binding hacks;
• Adding a new component to handle the "console logic" : Symfony2 Console Component is suitable for that part.
• Introducing a "commons" logic (useful for shared information between the Platform (buildtime) and Adapters (runtime).
• Refactoring Adapters to be more generic (Proxy connection or something else).
• Adding new named exceptions.
• Adding a new (or real) logging part: probably Monolog.
• Refactoring builders by using Twig instead of plain old PHP to generate PHP.
• Removing PEER classes.

Hi,

This is an open discution about when we need to use Short Classname and when we need to use FQCN.

We got lot's of issues with tests when we start Propel2, we fixed tests and not the lib but sometimes it's not the good thing to do.

Short Classename :

• all lib class in generated classes (Object, Query, Peer, Base, Map)
• keys in toArray
• keys in export yml, xml, (based on toArray())

Alias :

• Base in Child classes ( use Model\Base\Book as BaseBook in Model\Book)
• Base, and child classes in other Base classes (use Model\Book as ChildBook in Model\Base\Book or in Model\Base\Author )

FQCN :

• PHPDoc (with leading \) ( @return \Model\Book)
• in string used for calling classes dynamicaly (constant($class . '::MY_CONSTANT'), new $object()),

Methods

• getUnqualifiedClassName() => Book
• getQualifiedClassName() => Model\Base\Book
• getFullyQualifiedClassName() => \Model\Base\Book
• getClassname() alias of getFullyQualifiedClassName()

TBD :

We should add all usecase here to be consitant.

Others linked question:

What should return a getClassname() ?

• A short Classname / Alias
• A FQCN (with or without leading \)

we use those methods in runtime, builder, tableMap, and the behavior of the method is never consitant.

PropelPDO extending PDO is quite an important limitation. It's not (really) possible to create an oci8 adapter, not to mention NoSQL drivers.

An adapter approach, where the actual PDO connection would be injected and not extended, would be preferable and more extensible.

Note for myself:
Maybe port this patch: propelorm/Propel@5ce2674

William

The reset_tests.sh script fails (but continues) on the sql task of the bookstore-packaged fixture directory:

Fatal error: Call to a member function addTable() on a non-object in ~/projects/git/Propel2/src/Propel/Generator/Task/PropelSQLTask.php on line 255

Turns out one of the schemas has a different database and the PropelSQLTask::packageDataModels() method fails on this case.

Turns out the same build works flawlessly in Propel 1.6...

Hey Guys,

i'm currently have to face with a product database. As everyone knows, different kinds of products can have different attributes and also a different count of attriutes. for example a TV has screenwidth(inch=>integer), or hdmi (1/0=>smallint) and a tshirt has size(X/M/L=>varchar) etc. At buildtime it is not possible to decided which fields an which type of products will be used later.
the magento shop system solves this problem with the entity-attribute-value princple. For me as i am using propel thats kind of a problem. For sure it would be possible to add the attributes as a new Entity and therefore a new table. I would then have to add magic to __call(), but i think this problem is sth, that would be useful for very many propel users.
In Short: It would be great if you could make propel even greater and implement it directly into the core behaviors as this is a bigger feature i think and it should be implemented with all the background knowledge of propel which is possible to gain the best out of this feature. Discussion opened :)

greets,
DaHaiz

Hi there,

As we are almost refactored the whole code for Coding Standards, Namespaces, and Adapters, we can also remove useless code. I suggest:

• The NestedSet treemode: #25 ; done
• The addIncludes methods in builders + addIncludes parameter ; done
• The code related to non namespaces ;
• …

Actually, the most part to remove is located in the Generator. That way we could start refactoring builders, at least to slim them.

Any suggestions? I'm waiting for your comments and/or advices/code to remove.

As mentioned on the IRC Propel1 won't add (completely new) features, so my question directly on Propel2.

What do you think about this:
https://github.com/Ormigo/Propel/commit/9eb7922666784cefbbf0af3ba8098896f530f0fe

It allows to setup a list of database tables to be ignored from migration.
This enables the database to contain tables other than those generated and maintained using Propel.

For example: We list log and session tables, because there is no model for those.

Hi,

We have our own Jenkins instance for both Propel & Propel2 at http://ci.propelorm.org/.
It's fine but we could add Propel2 on Travis-CI as well (http://travis-ci.org/).

Why? Because we can test Propel2 on different database systems (MySQL, SQLite, PostgreSQL, …) and different PHP version (5.3, 5.4) without any effort!.

It may need some adjustments to get the whole working around fixtures configuration, which is a nice idea IMO.

Agreed ?

William

Hi,

I looked at the Propel main class and saw there is a logger attached to Propel. What about injecting a Propel\Logger\LoggerInterface object for using Monolog as the underlying logging system?

Any idea @fzaninotto and @willdurand?

I want to share some ideas about using ORM's in big projects. I know Propel has basic support for "master" "multiple slaves". But in big projects it would be grate to have more control. I still do not have any briliant idea, how to implement it, so I'll start from tasks that are importat form my point of view.

1. There shoudl be easy way to define master and slaves connections ( ok, we can do it now )

• master can be READ-WRITE on WRITE-only
• slaves are READ only

2. Why we should be able to define master in READ-WRITE mode ?
   • In may cases master is located on more powerfull machine. So it can handle some part of READ queries.
   • Sometimes you have important informations that you must access just after you put them into database. In that case you cant trust that slave will get up-to-date informations.
3. According to point 2 there should be some way to define probability of selecting master in case of READ query.
4. There should be some way to force Propel to user master for upcoming READ query.
5. There should be some way to handle connection problems. For example, if we have master and 2 slaves:
   • if there is WRITE query and master is not working - we're dead no way to handle it.
   • if there is READ query and selected db is not connected another one should be selected. Also dead connection shoud be placed on 'black-list'. This point can be in fact quite complex in case we would like to keep black-list for some time ( for example 20 seconds ), because there should be some cache system involved.
   • if there is READ query and all databases are not woring - we're dead.

Some cases ( Asume that probability of selecting master for READ query is 30% and we have 2 slaves):

1. Simple example
   • 1st query ( READ ) - slave_1 is selected
   • 2nd query ( READ ) - slave_1 ( we're using same connection as long as we can )
   • 3rd query ( WRITE ) - master is selected
   • 4th query ( READ ) - lets back to slave_1
2. Force master selection ( in case master is READ-WRITE )
   • 1st query ( READ ) - slave_2 is selected
   • 2nd query ( WRITE ) - master is selected
   • 3th query ( READ but we're asking about some VERY IMPORTANT DATA) - use_master=true master is selected
   • 4th query ( READ ) -lest back to slave_2
3. Master also can be picked for READ queries
   • 1st query ( READ ) - wow rand < 30 so master is selected
   • 2nd query ( READ ) still master ( we're keep same connection just like with slave connection )
   • 3rd, 4th ... ( some READ or WRITE ) we are still usign master connection

In my opinion those points CAN'T be placed directly inside Propel, Connection or any other "core" class. What I was thinking is something like "ConnectionManager" class. Maybe this name is not the best idea but developers could keep all those logic and functionality in this class.

Those points are not my "I want it or I'll get angry". In our company we're working on applications that must handle objectively high traffic. We know about all cache stuff and so on. Last project is using Doctrine2 and it was pain because of lack of expansibility. Propel has many grate ideas and in my opinion this is the best moment to think about that. I know Propel can handle small and normal traffic, but 'hey' big projects also want to use it :)

As you can see on our CI server, we are experimenting issues : http://ci.propelorm.org/job/Propel2/.

Actually, two tests are failing on the CI server but not on my laptop. It's probably due to iconv or a bug in the configuration but I ever double checked the PHP config and it seems ok.

Any ideas ?

Thanks,
William

See: https://github.com/cedriclombardot/TwigGenerator

Any suggestions ? Agreed, not agreed ?

/ping @cedriclombardot

As we can know the modified column for an object we can also know the old value, and of course the new, for this column.

Hi there,

I'm currently looking for removing Phing stuffs by introducing the Console component. And here are some ideas to discuss.

build.properties

First, I wonder how to deal with build properties.

• Do we need to parse build.properties file ?
• Will we keep this file ?

IMO we cannot convert all build properties into console parameters, I just don't imagine what we'll need to type to build model classes… The runtime as an XML file for the configuration.

So, we may use a generator-conf.xml to replace the build.properties existing file. That way, a command will load configurations from these XML files.

Here is the build.properties of the bookstore fixtures for instance:

<?xml version="1.0" encoding="UTF-8"?>
<generator>
    <project>
        <name>bookstore</name>
        <dir>.</dir>
    </project>
    <namespace>
        <autoPackage>false</autoPackage>
    </namespace>
    <schema>
        <autoPackage>false</autoPackage>
        <autoNamespace>false</autoNamespace>
        <autoPrefix>false</autoPrefix>
        <validate>true</validate>
        <transform>false</transform>
    </schema>
    <database>
        <platform>mysql</platform>
        <dsn>mysql:dbname=test</dsn>
        <user></user>
        <password></password>
        <schema></schema>
        <encoding></encoding>
        <tablePrefix></tablePrefix>
        <!--
            buildUrl ?
            createUrl ?
        -->
    </database>
    <mysql>
        <tableType>InnoDB</tableType>
    </mysql>
    <reverse>
        <samePhpName>false</samePhpName>
        <addVendorInfo>false</addVendorInfo>
        <addValidators></addValidators>
    </reverse>
    <model>
        <addGenericAccessors>true</addGenericAccessors>
        <addGenericMutators>true</addGenericMutators>
        <addTimeStamp>false</addTimeStamp>
        <addValidateMethod>true</addValidateMethod>
        <addHooks>true</addHooks>
        <basePrefix>Base</basePrefix>
        <classPrefix></classPrefix>
        <disableIdentifierQuoting>true</disableIdentifierQuoting>
        <targetPackage>bookstore</targetPackage>
        <packageObjectModel>false</packageObjectModel>
    </model>
    <datetime>
        <dateTimeClass>\DateTime</dateTimeClass>
        <defaultTimeStampFormat>Y-m-d H:i:s</defaultTimeStampFormat>
        <defaultTimeFormat>%X</defaultTimeFormat>
        <defaultDateFormat>%X</defaultDateFormat>
    </datetime>
    <behaviors>
        <test_all_hooks>\Propel\Tests\Helpers\Bookstore\Behavior\Testallhooksbehavior</test_all_hooks>
        <do_nothing>\Propel\Tests\Helpers\Bookstore\Behavior\DonothingBehavior</do_nothing>
        <add_class>\Propel\Tests\Helpers\Bookstore\Behavior\AddClassBehavior</add_class>
    </behaviors>
</generator>

Update: but we can also be smart by loading configuration (GeneratorConfig) from an array, and this array can be created from XML, YAML, INI, … That means, we can guess the generator-config file type by looking at its extension, and then to load the right loader.

Update2: Here is an extract of my work in progress: https://gist.github.com/f1e02b7411d016abaf33.
Should we rename some build properties ? At the moment, it's BC.

How to name the script ?

Another thing I would like to discuss is the name of the script. I suggest we call the console propel, that way we'll call it by running ./bin/propel or php bin/propel. Anyway, it's not important.
Doctrine puts it in the PATH to get a working doctrine command available everywhere (configured through PEAR).

Naming commands

Build stuffs:

• model:build
• sql:build

Migrations:

• migration:up
• migration:down
• migration:status
• migration:migrate [--up] [--down] // kind of alias

SQL:

• sql:insert
• sql:diff

Data:

• data:dump
• data:sql

Reverse:

• schema:reverse

Tools:

• tool:convert-conf
• tool:graphviz

Usage

// will build the model for the current directory
./bin/propel build:model

// will build the model into /path/to/project
./bin/propel build:model --dir=/path/to/project

As usual, suggestions? Comments? Ideas?

William

We have to fix the following tests:

 Generator

• tests/Propel/Tests/Generator/Behavior/NestedSet/NestedSetBehaviorObjectBuilderModifierTest.php;
• tests/Propel/Tests/Generator/Behavior/NestedSet/NestedSetBehaviorObjectBuilderModifierWithScopeTest.php;

The MS SQL Adapter simulates Limit/Offset by creating a query like :

SELECT TOP 20 * FROM (SELECT TOP 40 * FROM (SELECT ... ORDER BY column ASC) ORDER BY column DESC) ORDER BY column ASC;

https://github.com/propelorm/Propel/blob/9f91c2011ba783bc5ad1753cd1e840116c300b5f/runtime/lib/adapter/DBMSSQL.php#L126
In addition to be a very complex and durty query, this technic fails when Limit+Offset is out of bounds.

Someone found a better way to paginage over a query in MS SQL by using Row_Number() available since MS SQL 2005.
http://www.toosweettobesour.com/2010/09/16/doctrine-1-2-mssql-alternative-limitpaging/

One of the goals expressed in #1 is to remove the Peer classeS. But where do we put the current content of the Peer classes then?

As a reminder, here is what the Peer classes store:

• Database mapping info, sometimes overlapping that of TableMap classes (constants like TABLE_NAME, column name constants), sometimes not (constants like DATABASE_NAME, number of columns and of lazy columns).
• References to the ActiveRecord and TableMap classes
• Instance pool data and methods
• Field names conversion data and methods
• doXXX queries, (mostly) overlapping methods generated in the ActiveQuery class
• Hydration utilities (populateObject(), getPrimaryKeyFromRow(), etc.)

Here are a few considerations:

Mapping Info: The generated ActiveRecord and ActiveQuery classes are both first-class citizens in Propel, and none is more legitimate than the other to store mapping data. The TableMap classes are, but they can't be overridden by the user, contrary to the current constants. I suggest to put all the mapping info into the TableMap classes, add a constant TABLE_MAP_CLASS to both the generated AR and AQ classes, that the user can override in stub classes to get different mapping than what was done at buildtime. I also suggest that all TableMap classes become singletons, since there is no "primary" way of getting/storing them - so we would need something like BookTableMap::getinstance()

Hydration utilities: This is related to data mapping to my opinion, so I'd move these to the TableMap classes as well.

Instance Pool: This could all go into one single InstancePool service class. But the question is: how could the AR and AQ classes access this service? Not to mention that the Instance Pool service needs access to methods like getPrimaryKeyFromRow(). I suggest to initialize an instance pool in the TableMap's __construct() methods, and store the instances in a static attribute of the TableMap class.

doXXX Queries: Remove them altogether. This will require some work (all the queries are not migrated to AQ classes, see for instance AQ::delete()), but it makes sense.

All in all, I suggest to "boost" the generated TableMap classes instead of putting reusable services (like Hydrators, Instance Pool, name converters, etc) in a central "EntityManager" object that would be required for every operation. I think it's better to make all models relatively independent, and let them only rely on a global registry for configuration (and, for instance, to get the default connection for queries). Also, generated methods are more powerful and require less overhead at runtime - that's the Propel way.

Thoughts?

Hi

I think we should remove the whole validation part. It's not the Propel's job IMO.
It's not really up to date and it causes some issues (See Propel 1.6 issues).

Propel2 has to focus on what it's designed for. The (Symfony2) Validator Component is built to perform validation, if someone wants to validate things, he could use this component.

Thoughts ?

William

Migrated from: propelorm/Propel#149

Right now the schema.xml is ordered by when a table/column/index/etc is added to the schema, not alpha order. Well, actually table entries are alpha, but I think that's coincidence.

The problem is that if multiple people work on the DB at the same time, their changes may cause the "reverse" task to generate "changes" to the schema file that don't actually exist. This makes changesets larger than they have to be and adds risk to committing re-built code.

I propose that a new build option be added that allows you to have the reverse task sort all entries alphabetically so that the order of the schema can be counted on to be the same for an "equivalent" database.

I will write this code and submit a patch shortly.

Propel2 Coding Standards

We use the Symfony2 convention, here is a short example (from http://symfony.com/doc/current/contributing/code/standards.html

<?php

/**
 * This file is part of the Propel package.
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 *
 * @license    MIT License
 */

namespace Propel;

use A\Baz;
use B\Bar;

class Foo
{
    const SOME_CONST = 42;

    private $foo;

    /**
     * @param string $dummy Some argument description
     */
    public function __construct($dummy)
    {
        $this->foo = $this->transform($dummy);
    }

    /**
     * @param string $dummy Some argument description
     * @return string|null Transformed input
     */
    private function transform($dummy)
    {
        if (true === $dummy) {
            return;
        }
        if ('string' === $dummy) {
            $dummy = substr($dummy, 0, 5);
        }

        return $dummy;
    }
}

Structure

• Never use short tags (<?);
• Don't end class files with the usual ?> closing tag;
• Indentation is done by 4 spaces, don't use tabs;
• Use the linefeed character (0x0A) to end lines;
• Add a single space after each comma delimiter;
• Don't put spaces after an opening parenthesis and before a closing one;
• Add a single space around operators (==, &&, ...);
• Add a single space before the opening parenthesis of a control keyword (if, else, for, while, ...);
• Add a blank line before return statements, unless the return is alone inside a statement-group (like an if statement);
• Don't add trailing spaces at the end of lines;
• Use braces to indicate control structure body regardless of the number of statements it contains;
• Put braces on their own line for classes, methods, and functions declaration;
• Separate the conditional statements (if, else, ...) and the opening brace with a single space and no blank line;
• Declare visibility explicitly for class, methods, and properties (usage of var is prohibited);
• Use lowercase PHP native typed constants: false, true, and null. The same goes for array();
• Use uppercase strings for constants with words separated with underscores;
• Define one class per file;
• Declare class properties before methods;
• Declare class visibility after static or final keywords (e.g. final public, static public, final static, ...);
• Organize use alphabetically and group them by Runtime, Generator, and others. Each group will be separated by a new blank line.

Naming Conventions

• Use camelCase, not underscores, for variable, function and method names;
• Use namespaces for all classes;
• Use Propel as the first namespace level;
• Suffix interfaces with Interface;
• Use alphanumeric characters and underscores for file names;
• Don't forget to look at the more verbose Conventions document for more subjective naming considerations.

Documentation

• Add PHPDoc blocks for all classes, methods, and functions;
• Omit the @return tag if the method does not return anything;
• The @package and @subpackage annotations are no more used.

License

The following license has to be included in all files:

/**
 * This file is part of the Propel package.
 * For the full copyright and license information, please view the LICENSE
 * file that was distributed with this source code.
 *
 * @license    MIT License
 */

The generated Model classes are currently stored in files under the 'Om' namespace (and subdirectory). 'om' used to mean 'Object Model' (although I'm not really sure), but the stub files are also part of the Object Model, so ot doesn't really make sense.

I suggest to rename the 'Om' folder to 'Base', and rename the generated classes from 'BaseBook' to 'Base\Book', which would make more sense.

The Propel class is not a true singleton. It's a class full of static methods, not easily replaceable. I suggest to refactor it to a true singleton class, the only one in Runtime, holding a reference to all the necessary configuration:

• connections
• runtime model (Map classes)
• adapters
• logger

I have started refactoring the Propel class to make the domain more apparent (see fzaninotto@2f1cbfb). This must go further.

So in the runtime code, any call to

$con = Propel::getConnection(BookPeer::DATABASE_NAME, Propel::CONNECTION_READ);

should be rewritten to something like:

$con = Propel::getInstance()->getSlaveConnection(BookPeer::DATABASE_NAME);

The naming is important. Propelmight not be the best name. Registry also sounds too vague for me. It's a registry for runtime configuration - there are other types of registry in Propel. Manager is not good either, as it doesn't manage much more than configuration. Any suggestion? I'd vote for Propel\Runtime\Configuration:

$con = Configuration::getInstance()->getSlaveConnection(BookPeer::DATABASE_NAME);

which, incidentally, is a little shorter than the Propel::getConnection() version.

But there is also a Configuration namespace in Runtime. However, it contains code that can be removed if the convert-conf class creates a Configuration singleton. Something like:

<?php
// in generated $bookstore-conf.php
$conf = \Propel\Runtime\Configuration::getInstance();
$conf->addDatasource('bookstore', array(
  'dsn' => 'mysql:dbname=test',
  'user' => 'foo',
  'password' => 'bar', 
  'options' => array('ATTR_PERSISTENT' => false)
));
$conf->addAdapter('bookstore', 'Propel\Runtime\Adapter\MysqlAdapter');
$conf->setDefaultDatasource('bookstore');

Advantage: Configuration can easily be done by hand, using this class. The convert-conf task would just be a utility, taking XML as input but also YAML, or whatever you want. And in the Symfony2 integration, no need for that - the DI would populate the conf at will, maybe by subclassing it and injecting the DIC. That means we can probably get rid of the classes currently under Propel\Runtime\Configuration.

I know it's a BC break, but I think it's necessary.

Comments?

Rules

We need to be consistent between the Runtime and the Generator.
We have to be as explicit as we can.
We have to avoid single classes in a folder (src/Propel/Generator/Reverse/ for instance).
We can avoid Propel in classnames as the namespace contains Propel (except for the singleton).

Generator

src/Propel/Generator/Platform

• PropelPlatformInterface => PlatformInterface Done

src/Propel/Generator/Model

• IDMethod => IdMethod Done
• XMLElement => XmlElement Done

src/Propel/Generator/Model/Diff

Remove Propel in classnames. Done

src/Propel/Generator/Reverse

Move single classes in src/Propel/Generator/Reverse. Done

• BaseSchemaParser => AbstractSchemaParser Done
• SchemaParser => SchemaParserInterface Done

src/Propel/Generator/Util

• PropelPHPParser => PhpParser Done
• PropelSQLParser => SqlParser Done

Remove Propel in classnames. Done

Runtime

src/Propel/Runtime/Adapter

• DBAdapter => AbstractAdapter Done
• DBMySQL => MysqlAdapter (to be consistent with Platforms) Done

The `MSSQL` directory should be moved in another location.

 src/Propel/Runtime/Collection

Remove Propel in classnames. Done

/!\ How to name the `PropelCollection` class ?

src/Propel/Runtime/Config

Remove Propel in classnames. Done

src/Propel/Runtime/Formatter

• PropelFormatter => AbstractFormatter Done
• … Done

src/Propel/Runtime/Parser

• PropelParser => AbstractParser Done
• PropelCSVParser => CsvParser Done
• … Done

Hi guys,

As we are moving to version 2, I think we should remove the Propel singleton and probably rename it to PropelRegistry like for the Doctrine ORM. That's way we can use an instance of PropelRegistry (or Propel\Registry) to store DatabaseConnection objects, configuration and so on.

If we have such a Registry class without any static methods, we will be able to inject it in any dependency injection containers (Pimple, Symfony DIC, ZF2 DIC...)

Are you in favor of that?

Hi,

I started a new behavior for Propel 1.6 named Typehintable and I thought that should be nice to provide a native solution for the following use case: to allow users to typehint generated code in order to be compliant with interfaces.

Why ?

• Because it's possible to type hint methods, and to implement interfaces using proxy classes is almost always a pain;
• Because it's cool to implement interfaces;
• Because it's cool to type hint methods.

Feel free to add more use cases :)

What ?

We should be able to typehint the following methods:

• setXXX()
• addXXX()
• removeXXX()

We should allow PHP interfaces and primitive types (actually array).

How ?

By adding new attributes to the column and table tags:

<column name="roles" type="array" typehint="array" />

Will generate:

<?php

public function setRoles(array $roles)
{
    // ...
}

Or:

<table name="group" typehint="\GroupInterface">

A typehint on a table tag will allow to typehint addGroup(), removeGroup() methods on related tables (in a N-N relation for instance). That way, you'll deal with relations, which is the most common use case.

<?php

// in Model/User.php for instance

public function addGroup(\GroupInterface $group)
{
    // ...
}

Oh, and it will also modify the phpdoc.

The typehint has to be an interface or array, if it's not the case, you'll throw an exception at build time.

Thoughts?

William

The Citerion and ModelCriterion code is a mess. The dispatchPsHandling() cries for subclasses. I suggest that the code from this method moves up to Criteria, at the Criterion creation, so as to instanciate various Criterion subclasses (e.g. CriterionLike, CriterionIn, CriterionRaw, etc.), each having a dedicated and clean appendPsTo() method.

Is there anything like we can start writing the code like propel-gen and all ?

Or still this has some dependencies with phing ?

I / Someone may be interested to help , let me know what I can do for you if I get sometime .

The treemode=NestedSet feature is deprecated since Propel 1.5 (and the awesome nested set behavior), time to remove it.

Propel2 does not follow CS defined here: #2 at the moment, as we use 1.6.3 to setup Propel2. To fix CS is a priority before to start working on new features.

propelorm/Propel#199

/cc @martinj

• propelorm/Propel#161 done
• propelorm/Propel#163 done

The PropelQuickBuilder doesn't work at the moment, we get the following error:

phpunit -c phpunit.xml.dist tests/Propel/Tests/Generator/Builder/Om/GeneratedObjectArrayColumnTypeTest.php

PHPUnit 3.5.14 by Sebastian Bergmann.

Fatal error: Cannot use Propel\Runtime\Propel as Propel because the name is already in use in /Users/william/projects/Propel/Propel2/src/Propel/Generator/Util/PropelQuickBuilder.php(141) : eval()'d code on line 816

Due to use statements, we cannot call eval().

William

/ping @fzaninotto

See: propelorm/Propel#236

/cc @abrobston