Cannot delete media page if the file itself had been deleted

[Aleksej]

1. Have some media uploaded.
2. Delete the files manually from the user\_dev directory.
3. Try deleting a media through MediaGoblin (I used an "admin"
   user).

    Fails with "No such file or directory", shows an error page.

[Aleksej]

/exec/mediagoblin$ ./lazyserver.sh Using ./bin/paster +
CELERY\_ALWAYS\_EAGER=true ./bin/paster serve paste.ini --reload
Starting subprocess with file monitor Starting server in PID 23540.
serving on http://127.0.0.1:6543 Error - <type
'exceptions.OSError'>: [Errno 2] No such file or directory:
'/exec/mediagoblin/user\_dev/media/public/media\_entries/4e6e4f25bf7ec67f4e000007/medium.jpg'
URL:
http://127.0.0.1:6543/u/avrs/m/4e6e4f25bf7ec67f4e000007/confirm-delete/
File
'/exec/mediagoblin/eggs/Paste-1.7.5.1-py2.6.egg/paste/exceptions/errormiddleware.py',
line 144 in **call** app\_iter = self.application(environ,
sr\_checker) File
'/exec/mediagoblin/eggs/Paste-1.7.5.1-py2.6.egg/paste/urlmap.py',
line 203 in **call** return app(environ, start\_response) File
'/exec/mediagoblin/eggs/Beaker-1.5.4-py2.6.egg/beaker/middleware.py',
line 152 in **call** return self.wrap\_app(environ,
session\_start\_response) File
'/exec/mediagoblin/mediagoblin/app.py', line 157 in **call**
response = controller(request) File
'/exec/mediagoblin/mediagoblin/decorators.py', line 118 in wrapper
return controller(request, media=media, \*args,
**kwargs) File '/exec/mediagoblin/mediagoblin/decorators.py', line 50 in new\_controller\_func return controller(request, *args,**kwargs) File '/exec/mediagoblin/mediagoblin/decorators.py', line 66 in wrapper return controller(request,*args,**kwargs)
File '/exec/mediagoblin/mediagoblin/user\_pages/views.py', line 161
in media\_confirm\_delete delete\_media\_files(media) File
'/exec/mediagoblin/mediagoblin/util.py', line 694 in
delete\_media\_files listpath) File
'/exec/mediagoblin/mediagoblin/storage.py', line 216 in
delete\_file os.remove(self.\_resolve\_filepath(filepath)) OSError:
[Errno 2] No such file or directory:
'/exec/mediagoblin/user\_dev/media/public/media\_entries/4e6e4f25bf7ec67f4e000007/medium.jpg'

::

    CGI Variables
    -------------
      CONTENT_TYPE: 'multipart/form-data; boundary=---------------------------1179720268780089530547592974'
      HTTP_ACCEPT: 'text/html,application/xhtml+xml,application/xml;q=0.9,*/*;q=0.8'
      HTTP_ACCEPT_CHARSET: 'UTF-8,*'
      HTTP_ACCEPT_ENCODING: 'gzip, deflate'
      HTTP_ACCEPT_LANGUAGE: 'eo,ru;q=0.8,ru-ru;q=0.6,en-us;q=0.4,en;q=0.2'
      HTTP_CONNECTION: 'keep-alive'
      HTTP_COOKIE: 'mediagoblin=107c6aa1bb0092b307fd21febb97f93a'
      HTTP_DNT: '1'
      HTTP_HOST: '127.0.0.1:6543'
      HTTP_REFERER: 'http://127.0.0.1:6543/u/avrs/m/4e6e4f25bf7ec67f4e000007/confirm-delete/'
      HTTP_USER_AGENT: 'Mozilla/5.0 (X11; Linux x86_64; rv:7.0) Gecko/20100101 Firefox/7.0'
      PATH_INFO: '/u/avrs/m/4e6e4f25bf7ec67f4e000007/confirm-delete/'
      REMOTE_ADDR: '127.0.0.1'
      REQUEST_METHOD: 'POST'
      SERVER_NAME: '127.0.0.1'
      SERVER_PORT: '6543'
      SERVER_PROTOCOL: 'HTTP/1.1'
    
    WSGI Variables
    --------------
      application: {(None, ''): <beaker.middleware.SessionMiddleware object at 0x1e50110>, (None, '/mgoblin_static'): <StaticURLParser '/exec/mediagoblin/mediagoblin/static'>, (None, '/mgoblin_media'): <StaticURLParser '/exec/mediagoblin/user_dev/media/public'>}
      beaker.get_session: <bound method SessionMiddleware._get_session of <beaker.middleware.SessionMiddleware object at 0x1e50110>>
      beaker.session: {'user_id': u'4e6e6306bf7ec67be8000000', '_accessed_time': 1315862055.0335779, 'messages': [], '_creation_time': 1315861640.707809}
      paste.httpserver.thread_pool: <paste.httpserver.ThreadPool object at 0xfa7110>
      paste.throw_errors: True
      webob._body_file: (<LimitedLengthFile(<socket._fileobject object at 0x20747d0 length=173>, maxlen=173)>, <socket._fileobject object at 0x20747d0 length=173>)
      webob._parsed_post_vars: (MultiDict([('confirm', 'y')]), <FakeCGIBody at 0x20aaa10 viewing MultiDict([('co...y')])>)
      webob._parsed_query_vars: (GET([]), '')
      webob.adhoc_attrs: {'locale': 'eo', 'staticdirect': <mediagoblin.staticdirect.RemoteStaticDirect object at 0x1e41690>, 'app': <mediagoblin.app.MediaGoblinApp object at 0x1105dd0>, 'db': Database(Connection('localhost', 27017), u'mediagoblin'), 'urlgen': <routes.util.URLGenerator object at 0x20aaa90>, 'start_response': <function session_start_response at 0x20727d0>, 'session': {'user_id': u'4e6e6306bf7ec67be8000000', '_accessed_time': 1315862055.0335779, 'messages': [], '_creation_time': 1315861640.707809}, 'user': {u'username': u'avrs2', u'status': u'active', u'_id': ObjectId('4e6e6306bf7ec67be8000000'), u'bio': None, u'url': None, u'verification_key': None, u'bio_html': None, u'created': datetime.datetime(2011, 9, 12, 19, 52, 38, 384000), u'pw_hash': u'$2a$12$GCFdPFHjZgetWBhPL2RUiu4VA9kLq7192bKpJU0pWijppFzOS4Z1m', u'is_admin': True, u'email_verified': True, u'fp_token_expire': None, u'fp_verification_key': None, u'email': u'me2@example.com', u'plugin_data': {}}, 'template_env': <jinj...'user': u'avrs'}}
      webob.is_body_readable: True
      webob.is_body_seekable: False
      wsgi process: 'Multithreaded'
    ------------------------------------------------------------

[Aleksej]

Aleksej Serdjukov wrote:

    
.. raw:: html

       <del>
       
        Fails with "No such file or directory", shows an error page.
        4. Try deleting any media (that or other).
        
.. raw:: html

           </del>
           
            Nothing happens at all, redisplays the media page.




Sorry, those steps were not meant to be strike-though (there was a
"-" before each ">").

[Aleksej]

(I suspect the issue for which I originally filed this ticket was
actually from me forgetting to set the "I am sure" checkbox:
`http://bugs.foocorp.net/issues/597. <http://bugs.foocorp.net/issues/597.>`_)

[Christopher Allan Webber]

One simple solution to this is to simply return without doing
anything if there's nothing to delete. However that could cause
problems of its own...

Suggestions on how to handle this pleasantly?

[Aleksej]

Changing the ticket to be about the real issue.

[Will Kahn-Greene]

The original url for this bug was ​http://bugs.foocorp.net/issues/590 .
Relations:
#262: related

[Jorge Araya Navarro]

here is the fix ​https://gitorious.org/mediagoblin/mediagoblin/merge_requests/37 :D

[Will Kahn-Greene]

I'm pretty sure that fix hasn't been applied, so this shouldn't be closed.

Also, there are some issues with that merge request. First, it probably shouldn't have the comments that are there. Second, I think that's not the only reason that an OSError can get kicked up. Is silently doing nothing the right thing to do here? Should a message get logged?

[joar]

Replying to shackra:

here is the fix ​https://gitorious.org/mediagoblin/mediagoblin/merge_requests/37 :D

This issue seems to have been solved on a bit too low level in the infrastructure.

Fixing it in BasicFileStorage is not desirable since it's basically our interface to the "file systems" we use for storage.

I'd rather we fix it in tools/files.py's delete_media_files and user_pages/views.py's confirm_delete_media. Make the delete_media_files return the exceptions for all files along with the file name or path and log the errors.

In the view, use messages.add_message to print the errors, but go through with the deletion of the DB entry despite the errors.

[Jorge Araya Navarro]

I already send a merge request :)
what the user will see:

what the admin will read:

2012-06-28 22:04:04,589 ERROR   [mediagoblin.user_pages.views] No such files from the user "test" to delete: media_entries/1/146948.medium.jpg, media_entries/1/146948.thumbnail.jpg, media_entries/1/146948.jpg

I hope this bugfix is ok :D

[Christopher Allan Webber]

Hey Shackra,

This is great! And I think it's almost ready for merge.

I only have one requested change: as per PEP-8, we don't do mixedCase. Could you change:

    noSuchFiles = []

to be:

    no_such_files = []

In the future, it would be best not to use merge requests and use a feature branch that you link here instead. But anyway, looks good... make that change and I'll merge!

[Jorge Araya Navarro]

Replying to cwebber:

Hey Shackra,

This is great! And I think it's almost ready for merge.

I only have one requested change: as per PEP-8, we don't do mixedCase. Could you change:

    noSuchFiles = []

to be:

    no_such_files = []

In the future, it would be best not to use merge requests and use a feature branch that you link here instead. But anyway, looks good... make that change and I'll merge!

DONE! :D
you can merge my ​code now!!

[Christopher Allan Webber]

I'm not sure why your comment linked to the uncyclopedia article on viagra instead of the merge request, but I edited your post.

Also, looks like you missed one no_such_files switch, but I got it.

Anyway, merged and pushed! Thank you very much for working on this!