Skip to content
New issue

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

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

STI dump seems incorrect #11

Open
asgrim opened this issue Sep 14, 2015 · 4 comments
Open

STI dump seems incorrect #11

asgrim opened this issue Sep 14, 2015 · 4 comments

Comments

@asgrim
Copy link

asgrim commented Sep 14, 2015

We have two separate STI relationships:
abstract class Sanction, with various children, one of which is class Roadworthiness extends Sanction
and
abstract class ChecksiteLocation, with two children Temporary and Permanent

The data looks correct in the DB, but when dumping the fixtures I encounter an odd issue with each of them:

  • the Sanction entity is written directly, and the Roadworthiness entity is not generated at all in the fixtures
  • the Temporary and Permanent entities are written fine, but an additional ChecksiteLocation entity (with duplicated data that already exists in one of Temporary or `Permanent) is generated

I have a feeling these issues are related, but I can't quite put my finger on it. It seems to me that:

  • the Roadworthiness entity is not actually being written properly (not sure why)
  • the ChecksiteLocation entity is making it past the check in ORMDumper on Line 25 if (!$class->getReflectionClass()->isInstantiable() || $class->isMappedSuperclass) {

Do you have any ideas what might be causing this strange behaviour?

Thanks!

@asgrim
Copy link
Author

asgrim commented Sep 14, 2015

Note, I think a potential fix for this is to change that check in ORMDumper to simply:

if ($class->getReflectionClass()->isAbstract() || $class->isMappedSuperclass) {

The issue was Roadworthiness class is not instantiable (private function __construct, it is created using static constructors), but is not actually abstract, so should be included in the dump.

The ChecksiteLocation being included looks normal enough, so we simply ignore these in our import script (we simply now check if the class is abstract or not)

@Spea
Copy link
Owner

Spea commented Oct 3, 2015

When i change the check to ->isAbstract it might lead to fixtures which can not be executed since the class can not be instantiated (private constructor as you already mentioned). So yeah, it might fix your problem with fixtures not be generated, but these fixtures would be useless since they can not be executed.
If you could create a gist with the classes (including properties/methods) you are using it might be easier for me the get the whole problem.

@asgrim
Copy link
Author

asgrim commented Oct 3, 2015

The classes look a little like:

class Foo {
  private function __construct($a, $b, $c /* ... */) {
    /* ... */
  }

  public static function createBlah(/* ... */) {
    return new self(/* .. */);
  }
}

The point is this class is not directly instantiable (private constructor), it is not abstract - but we still need to include it in the dumped fixtures.

@Spea
Copy link
Owner

Spea commented Oct 3, 2015

But in order to create a fixture from this class, the library needs to know how to initialize it and currently I see no way how this might be possible.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

No branches or pull requests

2 participants