Skip to content

Commit

Permalink
Fix mapping of ambiguous columns (#122)
Browse files Browse the repository at this point in the history
  • Loading branch information
nilmerg authored Oct 23, 2023
2 parents ed6d937 + 22fd973 commit 3a6f687
Show file tree
Hide file tree
Showing 8 changed files with 114 additions and 7 deletions.
4 changes: 2 additions & 2 deletions phpstan-baseline.neon
Original file line number Diff line number Diff line change
Expand Up @@ -602,12 +602,12 @@ parameters:

-
message: "#^Cannot access offset non\\-falsy\\-string on mixed\\.$#"
count: 3
count: 2
path: src/Resolver.php

-
message: "#^Cannot access offset string on mixed\\.$#"
count: 6
count: 5
path: src/Resolver.php

-
Expand Down
5 changes: 4 additions & 1 deletion src/Behavior/Binary.php
Original file line number Diff line number Diff line change
Expand Up @@ -29,7 +29,10 @@ public function fromDb($value, $key, $_)

if ($value !== null) {
if (is_resource($value)) {
return stream_get_contents($value);
$content = stream_get_contents($value);
rewind($value);

return $content;
}

return $value;
Expand Down
37 changes: 34 additions & 3 deletions src/Hydrator.php
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,9 @@ class Hydrator
/** @var array Additional hydration rules for the model's relations */
protected $hydrators = [];

/** @var array<string, array<string, bool>> Map of columns to referencing paths */
protected $columnToTargetMap = [];

/** @var Query The query the hydration rules are for */
protected $query;

Expand Down Expand Up @@ -70,11 +73,31 @@ public function add($path)
}
}

$this->updateColumnToTargetMap($path, $columnToPropertyMap);
$this->hydrators[$path] = [$target, $relation, $columnToPropertyMap, $defaults];

return $this;
}

/**
* Update which columns the given path is referencing
*
* @param string $path
* @param array<string, string> $columnToPropertyMap
*
* @return void
*/
protected function updateColumnToTargetMap(string $path, array $columnToPropertyMap): void
{
foreach ($columnToPropertyMap as $qualifiedColumnPath => $_) {
if (isset($this->columnToTargetMap[$qualifiedColumnPath])) {
$this->columnToTargetMap[$qualifiedColumnPath][$path] = true;
} else {
$this->columnToTargetMap[$qualifiedColumnPath] = [$path => true];
}
}
}

/**
* Hydrate the given raw database rows into the specified model
*
Expand Down Expand Up @@ -120,7 +143,7 @@ public function hydrate(array $data, Model $model)
}
}

$subject->setProperties($this->extractAndMap($data, $columnToPropertyMap));
$subject->setProperties($this->extractAndMap($data, $columnToPropertyMap, $path));
$this->query->getResolver()->getBehaviors($target)->retrieve($subject);
$defaultsToApply[] = [$subject, $defaults];
}
Expand Down Expand Up @@ -181,15 +204,23 @@ public function hydrate(array $data, Model $model)
*
* @param array $data
* @param array $columnToPropertyMap
* @param string $path
*
* @return array
*/
protected function extractAndMap(array &$data, array $columnToPropertyMap)
protected function extractAndMap(array &$data, array $columnToPropertyMap, string $path)
{
$extracted = [];
foreach (array_intersect_key($columnToPropertyMap, $data) as $column => $property) {
$extracted[$property] = $data[$column];
unset($data[$column]);

if (isset($this->columnToTargetMap[$column][$path])) {
unset($this->columnToTargetMap[$column][$path]);
if (empty($this->columnToTargetMap[$column])) {
// Only unset a column once it's really not required anymore
unset($data[$column], $this->columnToTargetMap[$column]);
}
}
}

return $extracted;
Expand Down
2 changes: 2 additions & 0 deletions tests/Audit.php
Original file line number Diff line number Diff line change
Expand Up @@ -22,11 +22,13 @@ public function getColumns()
return [
'user_id',
'activity',
'subsystem_id'
];
}

public function createRelations(Relations $relations)
{
$relations->hasOne('user', User::class);
$relations->belongsToOne('subsystem', Subsystem::class);
}
}
16 changes: 16 additions & 0 deletions tests/Behavior/BinaryTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -37,6 +37,22 @@ public function testRetrievePropertyReturnsStreamContentsIfAdapterIsPostgreSQL()
);
}

public function testRetrievePropertyRewindsAStreamIfAdapterIsPostgreSQL(): void
{
$stream = fopen('php://temp', 'r+');
fputs($stream, static::TEST_BINARY_VALUE);
rewind($stream);

$this->assertSame(
static::TEST_BINARY_VALUE,
$this->behavior(true)->retrieveProperty($stream, static::TEST_COLUMN)
);
$this->assertSame(
static::TEST_BINARY_VALUE,
$this->behavior(true)->retrieveProperty($stream, static::TEST_COLUMN)
);
}

public function testPersistPropertyReturnsVanillaValueIfAdapterIsNotPostgreSQL(): void
{
$this->assertSame(
Expand Down
25 changes: 25 additions & 0 deletions tests/HydratorTest.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,25 @@
<?php

namespace ipl\Tests\Orm;

use ipl\Tests\Sql\TestCase;

class HydratorTest extends TestCase
{
public function testWhetherAmbiguousColumnsAreCorrectlyMapped(): void
{
$query = Subsystem::on(new TestConnection())
->with(['audit', 'audit.user']);

$hydrator = $query->createHydrator();

$subject = new Subsystem();
$hydrator->hydrate(['subsystem_audit_user_id' => 2], $subject);

$this->assertTrue(isset($subject->audit->user_id), 'Ambiguous columns are not mapped correctly');
$this->assertSame($subject->audit->user_id, 2, 'Ambiguous columns are not mapped correctly');

$this->assertTrue(isset($subject->audit->user->id), 'Ambiguous columns are not mapped correctly');
$this->assertSame($subject->audit->user->id, 2, 'Ambiguous columns are not mapped correctly');
}
}
3 changes: 2 additions & 1 deletion tests/SqlTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -354,7 +354,8 @@ public function testSelectFromModelWithNestedWith()
group_user.password AS group_user_password,
group_user_audit.id AS group_user_audit_id,
group_user_audit.user_id AS group_user_audit_user_id,
group_user_audit.activity AS group_user_audit_activity
group_user_audit.activity AS group_user_audit_activity,
group_user_audit.subsystem_id AS group_user_audit_subsystem_id
FROM
group
INNER JOIN
Expand Down
29 changes: 29 additions & 0 deletions tests/Subsystem.php
Original file line number Diff line number Diff line change
@@ -0,0 +1,29 @@
<?php

namespace ipl\Tests\Orm;

use ipl\Orm\Model;
use ipl\Orm\Relations;

class Subsystem extends Model
{
public function getTableName()
{
return 'subsystem';
}

public function getKeyName()
{
return 'id';
}

public function getColumns()
{
return ['name'];
}

public function createRelations(Relations $relations)
{
$relations->hasMany('audit', Audit::class);
}
}

0 comments on commit 3a6f687

Please sign in to comment.