Skip to content
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.

Sign up for GitHub

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

Jump to bottom

Duplicated entries in the profiler filesystem storage index #4719

Closed
stof opened this issue Jul 3, 2012 · 3 comments
Closed

Duplicated entries in the profiler filesystem storage index #4719

stof opened this issue Jul 3, 2012 · 3 comments
Labels
Bug HttpKernel WebProfilerBundle

Comments

@stof
Copy link
Member

stof commented Jul 3, 2012

The CSV file storing the index for the FilesystemStorage of the profiler contains duplicates for each request. Each request appears 3 times in the index. This breaks the search as limiting to 10 results only gives me 4 results.
@gajdaw
Copy link
Contributor

gajdaw commented Jul 6, 2012

@stof Second solution, i.e. https://github.com/gajdaw/symfony/commit/f3328e7be046b67c2b27136dbbac73840c58572c, passes tests. There are no duplicates. Implementation uses file() and file_put_contents() - I read/write complete file. The alternative to erase an entry in CSV file is (probably) to replace the old one with spaces and append a new one (at least if a new entry is longer than the old one). Should I leave it as it is or use fopen(), fseek(), fputs() functions?

@stof
Copy link
Member Author

stof commented Jul 6, 2012

@gajdaw please open a pull request with your changes

@gajdaw gajdaw mentioned this issue Jul 6, 2012
Closed
@gajdaw
Copy link
Contributor

gajdaw commented Jul 6, 2012

@stof Here is a fix and a test: #4770

@fabpot fabpot closed this as completed in 74aa502 Jul 8, 2012
fabpot added a commit that referenced this issue Jul 8, 2012
@fabpot
merged branch yanoosh/ticket_4719 (PR #4779)
dd4235a 
Commits
-------

74aa502 [HttpKernel] Fix #4719. Added condition which verify existence of profiler dump file. If file does not exists script inserts record to csv file.

Discussion
----------

[HttpKernel] Ticket #4719. Each call of FileProfilerStorage->write method adds row to index file.

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: yes
License of the code: MIT

Each call of FileProfilerStorage->write method adds row to index file even if profiler dump is only updated. Event dispatcher in kernel call several times write method to save fresh profiler info. I add condition which checks if profiler file already exist then ignore add row to index file.

---------------------------------------------------------------------------

by fabpot at 2012-07-08T08:04:33Z

Can you squash your commits before I merge? Thanks.

---------------------------------------------------------------------------

by yanoosh at 2012-07-08T10:08:48Z

Ready to merge.
fabpot added a commit that referenced this issue Jul 9, 2012
@fabpot
merged branch gajdaw/issue_4719_appendix (PR #4795)
c16de9b 
Commits
-------

d1a142e Issue #4719 - (Redis and Memcached fixes and test)

Discussion
----------

Issue #4719 - (Redis and Memcached fixes and test)

Bug fix: yes
Feature addition: no
Backwards compatibility break: no
Symfony2 tests pass: [![Build Status](https://secure.travis-ci.org/gajdaw/symfony.png?branch=issue_4719_appendix)](http://travis-ci.org/gajdaw/symfony)
Fixes the following tickets: 4719
Todo:
License of the code: MIT
Documentation PR: -

The same problem concerns `RedisProfilerStorage` and `BaseMemcacheProfilerStorage`.
Solution is similar.
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Assignees
No one assigned
Labels
Bug HttpKernel WebProfilerBundle
Projects
None yet
Milestone
No milestone
Development

No branches or pull requests
3 participants
@fabpot @stof @gajdaw