Skip to content

Changed the name of the method to be more revelent. Fix the leading slash on the getWebLocation() for portability #558

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

Merged
merged 5 commits into from
Jul 27, 2011
Merged
Changes from all commits
Commits
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
19 changes: 15 additions & 4 deletions cookbook/doctrine/file_uploads.rst
Original file line number Diff line number Diff line change
Expand Up @@ -48,15 +48,26 @@ First, create a simple Doctrine Entity class to work with::
*/
public $path;

public function getFullPath()
public function getAbsolutePath()
{
return null === $this->path ? null : $this->getUploadRootDir().'/'.$this->path;
}

public function getWebPath()
{
return null === $this->path ? null : $this->getUploadDir().'/'.$this->path;
Copy link
Member

Choose a reason for hiding this comment

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

calling it recursively will lead to an infinite loop

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Sorry but i don't see where there is a possible infinite loop :o. Maybe it was in the previous commit. I fixed it :p

Copy link
Member

Choose a reason for hiding this comment

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

I loaded the page before the commit fixing it. Github does not refresh the page automatically when you add a commit to a pull request :)

Copy link
Member

Choose a reason for hiding this comment

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

Do you think that the getWebPath() should return a path that starts with a slash or not?

uploads/documents/foo.pdf

versus

/uploads/documents/foo.pdf

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Stof thinks that is should not start with a leading slash for portability ( see comments here on my closed pull request #557 )

Copy link
Member

Choose a reason for hiding this comment

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

no slash as you would have to remove it to use it in asset(). A path starting with a slash in asset() is not modified as it s assumed to be relative to the root of the domain. A path that don't start by a slash is assumed to be relative to the web folder (This concern the case where you don't configure the helper to use a CDN of course)

Copy link
Member

Choose a reason for hiding this comment

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

@stof Yes sir, you're right - I've been omitting the leading slash without realizing it.

}

protected function getUploadRootDir()
{
// the absolute directory path where uploaded documents should be saved
return __DIR__.'/../../../../web/uploads/documents';
return __DIR__.'/../../../../web/'.$this->getUploadDir();
}

protected function getUploadDir()
{
// get rid of the __DIR__ so it doesn't screw when displaying uploaded doc/image in the view.
return 'uploads/documents';
}
}

Expand Down Expand Up @@ -274,7 +285,7 @@ Next, refactor the ``Document`` class to take advantage of these callbacks::
*/
public function removeUpload()
{
if ($file = $this->getFullPath()) {
if ($file = $this->getAbsolutePath()) {
unlink($file);
}
}
Expand Down Expand Up @@ -331,7 +342,7 @@ property, instead of the actual filename::
*/
public function removeUpload()
{
if ($file = $this->getFullPath()) {
if ($file = $this->getAbsolutePath()) {
unlink($file);
}
}
Expand Down