Skip to content

New Transit::attach() method to attach domain to the current context #16

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.

Sign up for GitHub

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

Open
ElGigi wants to merge 2 commits into
base: 0.x
Choose a base branch
from
Open

New Transit::attach() method to attach domain to the current context #16

Show file tree
Hide file tree
Changes from all commits
Commits
Show all changes
2 commits
Select commit Hold shift + click to select a range
3594858
New Transit::attach() method to merge an entity to the handler storage
ElGigi May 2, 2019
f2909d0
Fix PhpDoc
ElGigi May 2, 2019
File filter

Filter by extension

Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
18 changes: 18 additions & 0 deletions src/Transit.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -68,6 +68,24 @@ public function select(string $domainClass, array $whereEquals = []) : TransitSe
);
}

/**
* Attach a domain object to plan.
*
* @param object $domain
*
* @return \Atlas\Transit\Transit
* @throws \Atlas\Transit\Exception
*/
public function attach(object $domain): Transit
Copy link

@froschdesign froschdesign May 2, 2019 •
edited
Loading

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I see a problem here: which method should the end user use? store or attach? This results in an unclear API.

Copy link
Contributor Author

@ElGigi ElGigi May 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Like Doctrine; it's needed to "attach" ("merge" with Doctrine) the entity to Transit before modification.
If the entity is modified and stored, she is already present and an update is done.

Copy link
Contributor Author

@ElGigi ElGigi May 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If you have an another suggestion of method name, i'm ready :)

Copy link

@froschdesign froschdesign May 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Doctrine no longer works as an example, because merge and detach are deprecated and will be removed in version 3. 😉

https://github.com/doctrine/orm/blob/master/UPGRADE.md#bc-break-removed-entitymanagermerge-and-entitymanagerdetach-methods
doctrine/orm#1577 (comment)

Copy link
Contributor Author

@ElGigi ElGigi May 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Yes i see that 👍 .
But how do for the current problematic?
It's the job of a framework? or need to integrate a concept into Transit?

Copy link

@froschdesign froschdesign May 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

See also at Doctrine:

It is a good idea to avoid storing entities in serialized formats such as $_SESSION: instead, store the entity identifiers or raw data.

https://github.com/doctrine/orm/blob/master/docs/en/cookbook/entities-in-session.rst#entities-in-the-session

…and this is the path you should go.

Copy link
Contributor Author

@ElGigi ElGigi May 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not too similar to Transit::select(string $domainClass, array $whereEquals = []) method; with second parameter who accept a scalar or array argument?

Copy link

@froschdesign froschdesign May 2, 2019

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

It's not too similar to…

You are on the right track. 😉
Don't serialize your domain objects, instead:

  • store the entity identifiers or raw data (in session, etc.)
  • use the existing select method to get your domain object

…and the entire pull request is not needed.

{
$handler = $this->handlerLocator->get($domain);
/** @var \Atlas\Mapper\Record $record */
$record = $handler->updateSource($domain, $this->plan);
$record->getRow()->init('');

return $this;
}

// PLAN TO insert/update
public function store(object $domain) : void
{
Expand Down
25 changes: 25 additions & 0 deletions tests/TransitTest.php
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -533,4 +533,29 @@ public function testStore()
$this->assertSame(101, $actual['responses'][0]['responseId']);
$this->assertSame(14, $actual['responses'][0]['author']['authorId']);
}

public function testAttach()
{
// Creation of author in first context
$entity = new Author('Author');
$this->transit->store($entity);
$this->transit->persist();

// New transit class for new context
$secondTransit = FakeTransit::new(
$this->atlas,
'Atlas\\Testing\\DataSource\\'
);
$secondTransit->attach($entity);

// Modify entity after attach to context
$entity->setName('Arthur 2123 fdgfd');

$secondTransit->store($entity);
$secondTransit->persist();

/** @var \Atlas\Mapper\Record $record */
$record = $secondTransit->getStorage()->offsetGet($entity);
$this->assertEquals('UPDATED', $record->getRow()->getStatus());
}
}