Skip to content

Commit

Permalink
Handle mandatory properties of resources correctly
Browse files Browse the repository at this point in the history
  • Loading branch information
bergie committed Nov 17, 2011
1 parent f94c72d commit ae5c19f
Show file tree
Hide file tree
Showing 3 changed files with 3 additions and 1 deletion.
1 change: 1 addition & 0 deletions lib/Doctrine/ODM/PHPCR/Document/File.php
Original file line number Diff line number Diff line change
Expand Up @@ -180,6 +180,7 @@ private function getContent()
{
if ($this->content === null) {
$this->content = new Resource();
$this->content->setLastModified(new \DateTime());

This comment has been minimized.

Copy link
@uwej711

uwej711 Nov 18, 2011

If I remember properly this should be handled by the cr implementation, jackrabbit does it.

This comment has been minimized.

Copy link
@bergie

bergie Nov 18, 2011

Author Owner

The way I see it, a mandatory property of a node is a bit like required fields in forms - you have to provide valid data there before you can save the node. But I would need to double-check the spec on this.

This comment has been minimized.

Copy link
@bergie

bergie Nov 18, 2011

Author Owner

From Jackrabbit:

Mandatory Status A mandatory property is one that must exist. If a node of a type that specifies a mandatory property is created then any attempt to save that node without adding the mandatory property will fail.

This comment has been minimized.

Copy link
@uwej711

uwej711 Nov 18, 2011

From the JCR Spec:

3.7.11.8 mix:lastModified

[mix:lastModified] mixin
- jcr:lastModified (DATE) autocreated protected? OPV?
- jcr:lastModifiedBy (STRING) autocreated protected? OPV?

This mixin node type can be used to provide standardized modification information properties to a node. In implementations that make these properties protected, their values are controlled by the repository, which should set them appropriately upon a significant modification of the subgraph of a node with this mixin. What constitutes a significant modification will depend on the semantics of the various parts of a node's subgraph and is implementation-dependent.

This comment has been minimized.

Copy link
@uwej711

uwej711 Nov 18, 2011

So actually it is not mandatory, as well as the mimeType (at least not in JCR 2.0)

This comment has been minimized.

Copy link
@bergie

bergie Nov 18, 2011

Author Owner

Yep, seems a mismatch between JCR 1 and 2. In JCR 1 this was mandatory, in JCR 2 it isn't

This comment has been minimized.

Copy link
@dbu

dbu Nov 18, 2011

i found this in the spec:
http://www.day.com/specs/jcr/2.0/3_Repository_Model.html#3.7.11.8%20mix:lastModified

saying that the field /may/ be automatically handled by the repository. so at least for some implementations, what you try to do would be forbidden.
probably you need to get the node type information for mix:lastModified and check if the lastModified property is protected or not. or we could hardcode it for known repository implementations and just get the node type for unknown implementations, to avoid the overhead.

(sad that they did not decide on one or the other imo)

This comment has been minimized.

Copy link
@bergie

bergie Nov 18, 2011

Author Owner

Added a ticket against the Midgard2 PHPCR provider to make all built-in node type defs to conform to JCR 2.0 spec: midgardproject/phpcr-midgard2#62

}
return $this->content;
}
Expand Down
2 changes: 1 addition & 1 deletion lib/Doctrine/ODM/PHPCR/Document/Resource.php
Original file line number Diff line number Diff line change
Expand Up @@ -25,7 +25,7 @@ class Resource
protected $data;

/** @PHPCRODM\String(name="jcr:mimeType") */
protected $mimeType;
protected $mimeType = 'application/octet-stream';

This comment has been minimized.

Copy link
@uwej711

uwej711 Nov 18, 2011

Not sure if the same above applies but it works with jackrabbit.

/** @PHPCRODM\String(name="jcr:encoding") */
protected $encoding;
Expand Down
1 change: 1 addition & 0 deletions tests/Doctrine/Tests/ODM/PHPCR/Functional/FileTest.php
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ public function testCreateFromString()
$this->assertTrue($this->node->getNode('filetest')->getNode('file')->hasNode('jcr:content'));
$this->assertTrue($this->node->getNode('filetest')->getNode('file')->getNode('jcr:content')->hasProperty('jcr:data'));
$binaryStream = $this->node->getNode('filetest')->getNode('file')->getNode('jcr:content')->getProperty('jcr:data')->getBinary();
$this->assertFalse(is_null($binaryStream), 'Ensure that we got a stream from the file');
$content = stream_get_contents($binaryStream);
$this->assertEquals('Lorem ipsum dolor sit amet', $content);
}
Expand Down

0 comments on commit ae5c19f

Please sign in to comment.