Edgewall Software
Modify

Opened 17 years ago

Closed 17 years ago

#7267 closed defect (fixed)

Pagination variables are temporarly shared betweens user sessions in ReportModule

Reported by: farialima@… Owned by: Christian Boos
Priority: normal Milestone: 0.11
Component: report system Version: 0.11rc1
Severity: major Keywords:
Cc: Branch:
Release Notes:
API Changes:
Internal Changes:

Description (last modified by Christian Boos)

[6901] has added pagination handling for report. From looking at the code, it adds data attributes ('page', 'limit', 'num_rows', as well as 'asc', 'sort', 'USER') to the class "ReportModule" (see source:/trunk/trac/ticket/report.py@6901#L25 ) . But, AFAIK, only one instance of each "Component" class (of which ReportModule inherits) is shared across all user requests, so I believe that these attributes will be shared across users. Or maybe I'm reading the code wrong…

The risk is low, but unpredictable: it will be a problem when two users request a (different) report at the same time; in that case, the result will be unpredictable.

It also makes it impossible to use the ReportModule.execute_report without first settings the 'limit' attribute (which is why I caught this issue: I had some automated tests that would call execute_report and that suddenly started failing…)

Why not instead add an optional parameter to execute_report instead, that would contain (and return, as attributes) the pagination information ?

I can propose a patch if you want,

Again, maybe I'm wrong and not reading the code correctly…

Attachments (1)

fix-report-pagination-r7157.diff (6.1 KB ) - added by Christian Boos 17 years ago.
Should fix the scoping issues for report pagination

Download all attachments as: .zip

Change History (6)

comment:1 by francois@…, 17 years ago

the code is actually on line source:/trunk/trac/ticket/report.py@6901#L293, not 25 as I wrote above

comment:2 by Christian Boos, 17 years ago

Description: modified (diff)
Milestone: 0.11

Oh, you're completely right, this has been overlooked when cleaning up the #216 patch.

comment:3 by Christian Boos, 17 years ago

Description: modified (diff)

([OT] looks like we also have a problem with eating up the blank lines of a ticket description, as the (diff) above happened without any expliciting editing from my part)

by Christian Boos, 17 years ago

Should fix the scoping issues for report pagination

comment:4 by Christian Boos, 17 years ago

Owner: changed from Matthew Good to Christian Boos
Status: newassigned

The patch above should fix the issue and apparently doesn't break things - but more tests are welcomed.

comment:5 by Christian Boos, 17 years ago

Resolution: fixed
Status: assignedclosed

Patch checked in as r7212 for 0.11 and as part of r7214 for 0.12.

Modify Ticket

Change Properties
Set your email in Preferences
Action
as closed The owner will remain Christian Boos.
The resolution will be deleted. Next status will be 'reopened'.
to The owner will be changed from Christian Boos to the specified user.

Add Comment


E-mail address and name can be saved in the Preferences .
 
Note: See TracTickets for help on using tickets.