Opened 9 years ago

Closed 8 years ago

#255 closed defect (fixed)

Cannot delete media page if the file itself had been deleted

Reported by: Aleksej Owned by: Jorge Araya Navarro
Priority: minor Milestone:
Component: programming Keywords:
Cc: Parent Tickets:

Description (last modified by Will Kahn-Greene)

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.




Subtickets

Change History (16)

comment:1 Changed 9 years ago by 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'
    ------------------------------------------------------------



comment:2 Changed 9 years ago by 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 ">").



comment:3 Changed 9 years ago by 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.>`_)



comment:4 Changed 9 years ago by 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?



comment:5 Changed 9 years ago by Aleksej

Changing the ticket to be about the real issue.



comment:6 Changed 9 years ago by Will Kahn-Greene

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

comment:7 Changed 9 years ago by Sacha

Owner: set to Sacha
Status: acceptedassigned

comment:8 Changed 8 years ago by Jorge Araya Navarro

Resolution: fixed
Status: assignedclosed

comment:9 Changed 8 years ago by Will Kahn-Greene

Description: modified (diff)
Resolution: fixed
Status: closedreopened

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?

comment:10 Changed 8 years ago by 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.

comment:11 Changed 8 years ago by Jorge Araya Navarro

Owner: changed from Sacha to Jorge Araya Navarro
Status: reopenedassigned

comment:12 Changed 8 years ago by Jorge Araya Navarro

I already send a merge request :)
what the user will see: http://ompldr.org/vZWpzMg/Captura%20de%20pantalla%20de%202012-06-28%2021:57:48.png

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

comment:13 Changed 8 years ago by 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!

comment:14 in reply to:  13 Changed 8 years ago by 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!!

Last edited 8 years ago by Christopher Allan Webber (previous) (diff)

comment:15 Changed 8 years ago by 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!

comment:16 Changed 8 years ago by Christopher Allan Webber

Resolution: fixed
Status: assignedclosed
Note: See TracTickets for help on using tickets.