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

Versioning support #96

Merged
merged 33 commits into from Feb 3, 2012

Conversation

Projects
None yet
4 participants
@dbu
Member

dbu commented Feb 2, 2012

adding versioning support as discussed on the mailing list. please see the added doc in the readme to see how this is to be used. the rudimentary versioning support that existed before is heavily refactored. if you used it, you will need to change:

  • Document attribute versionable is now required. it must be set to either "simple" or "full"
  • the @Version annotation is gone, it makes not much sense. for old version snapshot documents, you can use @ VersionName and @VersionCreated to track the metadata.
  • checkin and checkout are now lowercased to be consistent. note that we added checkpoint for convenience
  • restore has been renamed to restoreVersion and expects as parameter an old version rather than the document and version name.
  • instead of getPredecessors (which was misnamed anyways), use getAllLinearVersions to get metadata about versions, and use findVersionByName to get a detached document of the old version of your data.

apart from that, we added a removeVersion and update to latest jackalope with lots of fixes in version handling code.

Daniel Barsotti and others added some commits Nov 18, 2011

Daniel Barsotti
Cleanup about versioning annotation
- The correct versionable mixin is set on document creation, checkin and checkout
- A document cannot have a @Version field if it is not versionable
- Corrected the exiting version related tests
Merge remote branch 'upstream/master' into versioning, remove alias f…
…rom test documents

Conflicts:
	README.md
	lib/Doctrine/ODM/PHPCR/UnitOfWork.php
	lib/vendor/jackalope
	tests/Doctrine/Tests/ODM/PHPCR/Functional/VersioningTest.php
$node->addMixin('mix:simpleVersionable');
} elseif ($metadata->versionable === 'full') {
$node->addMixin('mix:versionable');
} elseif ($metadata->versionable) {

This comment has been minimized.

@lsmith77

lsmith77 Feb 2, 2012

Member

no need for those 2 last "else"

This comment has been minimized.

@dbu

dbu Feb 2, 2012

Member

i removed the check whether the document is versionable of an unknown type.
but i want to keep the last line, we call this method on checkin and checkout as well, whithout sanitizing. and there we want to notice if somebody tries to checkin a document that is not versionable.

This comment has been minimized.

@lsmith77

lsmith77 Feb 2, 2012

Member

ah no .. my point wasn't that the exception throwing should be removed .. but actually my comment was bogus .. never mind :)

README.md Outdated
@@ -445,6 +441,20 @@ class DocumentRepository extends BaseDocumentRepository implements RepositoryIdI
</td>
</tr>
<tr>
<td> VersionName: </td>

This comment has been minimized.

@lsmith77

lsmith77 Feb 2, 2012

Member

some extra spaces

@@ -14,7 +14,7 @@
public $nodeType = 'nt:unstructured';
/** @var string */
public $repositoryClass;
/** @var boolean */
/** @var string */
public $versionable = false;

This comment has been minimized.

@stof

stof Feb 2, 2012

Member

string with a boolean default ? this seems weird

This comment has been minimized.

@dbu

dbu Feb 2, 2012

Member

removed the default

@@ -372,6 +382,9 @@ public function setLifecycleCallbacks(array $callbacks)
*/
public function setVersioned($versionable)
{
if (!in_array($versionable, self::$validVersionableAnnotations)) {

This comment has been minimized.

@stof

stof Feb 2, 2012

Member

does it work when the attribute is false ?

This comment has been minimized.

@stof

stof Feb 2, 2012

Member

ah nvm, the method is not called in such case

@@ -141,6 +141,7 @@ public function testRemoveChildParent()
$parent = $this->dm->find('Doctrine\Tests\ODM\PHPCR\Functional\ChildrenTestObj', '/functional/parent');
$this->assertNull($parent);
}
public function assertCount($exp, $col) {$this->assertTrue($exp==count($col));}

This comment has been minimized.

@stof

stof Feb 2, 2012

Member

what is it ?

This comment has been minimized.

@lsmith77

lsmith77 Feb 2, 2012

Member

its time we require phpunit 3.6

This comment has been minimized.

@dbu

dbu Feb 2, 2012

Member

ups, damn. i just put that in so i could run the tests without updating my phpunit during a train ride...

i promise to update, remove this and will look for tests that could use assertCount

$versions = $this->dm->getAllLinearVersions($doc);
$this->assertEquals(5, count($versions));

This comment has been minimized.

@stof

stof Feb 2, 2012

Member

why not using the dedicated assertion assertCount ?

This comment has been minimized.

@dbu

dbu Feb 2, 2012

Member

done. there is lots of existing code with this issue too. will update when i find time.

@dbu

This comment has been minimized.

Member

dbu commented Feb 2, 2012

i will merge tomorrow unless somebody finds more issues. went through the api with lukas again and he is ok with it.

@lsmith77

This comment has been minimized.

Member

lsmith77 commented Feb 3, 2012

lsmith77 added a commit that referenced this pull request Feb 3, 2012

@lsmith77 lsmith77 merged commit a574615 into doctrine:master Feb 3, 2012

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