Bug #34668 for CGI-Session: Documentation on 'delete()' is misleading

Fri Apr 04 05:24:40 2008 colin.fine [...] pace.co.uk - Ticket created

Subject:

Documentation on 'delete()' is misleading

The perldoc on 'delete()' says: "Technically speaking, it will only set object's status to STATUS_DELETED and will trigger flush(), and flush() will do the actual removal." I took this to mean that I did not need to call flush() after delete(), but this doesn't appear to be the case.

Sun Apr 06 20:06:44 2008 RSAVAGE [...] cpan.org - Correspondence added

POD for CGI::Session patched in V 4.29_3 to emphasize even more that auto-flushing is unreliable, and that flush() should always be called explicitly before the program exits.

Sun Apr 06 20:07:21 2008 The RT System itself - Status changed from 'new' to 'open'

Sun Apr 06 20:07:25 2008 RSAVAGE [...] cpan.org - Status changed from 'open' to 'resolved'

Mon Apr 07 04:29:53 2008 colin.fine [...] pace.co.uk - Correspondence added

From:

colin.fine [...] pace.co.uk

On Sun Apr 06 20:06:44 2008, RSAVAGE wrote: Show quoted text

> POD for CGI::Session patched in V 4.29_3 to emphasize even more that > auto-flushing is unreliable, and that flush() should always be called > explicitly before the program exits.

Yes, that's a good move, but it doesn't address my specific comment. The pod for 'delete()' says "and will trigger flush()", which I read as saying I do not need to call flush. This is nothing to do with program exit - I am using delete() early in my program because in this particular instance I want to ensure that I get a brand new session (or is there a better way to do that?).

Mon Apr 07 04:29:54 2008 The RT System itself - Status changed from 'resolved' to 'open'

Mon Apr 07 04:57:28 2008 RSAVAGE [...] cpan.org - Correspondence added

From:

RSAVAGE [...] cpan.org

On Mon Apr 07 04:29:53 2008, COLINFINE wrote: Show quoted text

> Yes, that's a good move, but it doesn't address my specific comment. The > pod for 'delete()' says "and will trigger flush()", which I read as > saying I do not need to call flush.

The new POD explicitly warns that auto-flushing is unreliable, so I'm hopeful that answers that part of your problem. Show quoted text

> This is nothing to do with program exit - I am using delete() early in > my program because in this particular instance I want to ensure that I > get a brand new session (or is there a better way to do that?).

This is a different topic. If you'd read the docs for new() very carefully you'd see you should be calling new(undef), or new($dsn, undef) to create a new session. That has nothing to do with delete(), although it's not clear from your msg whether or not you're expecting delete() to have the magical side-effect of creating a new session. If so, don't do that :-). So, execute 2 statements, first a delete() and then a new(). Is that clear? Let me know so I can close this ticket.

Mon Apr 07 05:42:17 2008 colin.fine [...] pacemicro.com - Correspondence added

Subject:	Re: [rt.cpan.org #34668] Documentation on 'delete()' is misleading
Date:	Mon, 07 Apr 2008 10:40:44 +0100
To:	bug-CGI-Session [...] rt.cpan.org
From:	Colin Fine <colin.fine [...] pacemicro.com>

RSAVAGE via RT wrote: Show quoted text

> <URL: http://rt.cpan.org/Ticket/Display.html?id=34668 > > > On Mon Apr 07 04:29:53 2008, COLINFINE wrote: >

>> Yes, that's a good move, but it doesn't address my specific comment. The >> pod for 'delete()' says "and will trigger flush()", which I read as >> saying I do not need to call flush. >>

> > The new POD explicitly warns that auto-flushing is unreliable, so I'm > hopeful that answers that part of your problem. >

Not really, because that discussion is in the context of exiting the session. I want you to change the POD for delete() to say that you must explicitly call flush() after it. Show quoted text

> >

>> This is nothing to do with program exit - I am using delete() early in >> my program because in this particular instance I want to ensure that I >> get a brand new session (or is there a better way to do that?). >>

> > This is a different topic. If you'd read the docs for new() very > carefully you'd see you should be calling new(undef), or new($dsn, > undef) to create a new session. That has nothing to do with delete(), > although it's not clear from your msg whether or not you're expecting > delete() to have the magical side-effect of creating a new session. If > so, don't do that :-). So, execute 2 statements, first a delete() and > then a new(). > >

No, I wasn't clear. I am calling new($CGI) after the delete(). The reason I need to do this is that there may be a cookie identifying the existing session, which I want to trash. So new(undef) or new($CGI) will pick up the existing session, which I then delete before creating a new one. While I'm on the subject of the documentation, I'd like the POD for load() to say explicitly that if it does not find an existing session it still returns a session object (which is empty). I know that this is implicit in the example, but it would be clearer if it said so. Best, Colin -- *Colin Fine* *Engineering Tools Group Pace Micro Technology plc* */Bringing Technology Home/* *Tel: +44 1274 538038* *Fax: +44 1274 532029* *Victoria Road, Saltaire, West Yorkshire. BD18 3LF _ __www.pacemicro.com_* This E-mail and any attachments hereto are strictly confidential and intended solely for the addressee. If you are not the intended addressee please notify the sender by return and delete the message. You must not disclose, forward or copy this E-mail or attachments to any third party without the prior consent of the sender. Pace Micro Technology plc (registered in England and Wales no. 1672847) whose head office is based at Victoria Road, Saltaire, West Yorkshire, BD18 3LF, UK. Tel +44 (0) 1274 532000 Fax +44 (0) 1274 532010 and at_ http://www.pacemicro.com_. This message has been scanned for viruses by BlackSpider MailControl - www.blackspider.com

Mon Apr 07 05:59:39 2008 RSAVAGE [...] cpan.org - Correspondence added

On Mon Apr 07 05:42:17 2008, colin.fine@pacemicro.com wrote: Show quoted text

> Not really, because that discussion is in the context of exiting the > session. I want you to change the POD for delete() to say that you

must explicitly call flush() after it. The new text under delete() (which I wrongly assumed you'd seen) is: Warning: Auto-flushing can be unreliable, and always explicitly calling C<flush()> on the session before the program exits should be regarded as mandatory until this problem is rectified. Clear enough? Show quoted text

> No, I wasn't clear. I am calling new($CGI) after the delete().

OK. Show quoted text

> The reason I need to do this is that there may be a cookie identifying > the existing session, which I want to trash. So new(undef) or new($CGI) > will pick up the existing session, which I then delete before creating a > new one.

I'll run some tests tomorrow. It's 8pm here now. Frankly, I did not expect new(undef) to use an existing session. We'll see. Show quoted text

> While I'm on the subject of the documentation, I'd like the POD for > load() to say explicitly that if it does not find an existing session it > still returns a session object (which is empty). I know that this is > implicit in the example, but it would be clearer if it said so.

That's a good point. I'll clarify the docs (tomorrow...).

Mon Apr 07 06:07:50 2008 colin.fine [...] pacemicro.com - Correspondence added

Subject:	Re: [rt.cpan.org #34668] Documentation on 'delete()' is misleading
Date:	Mon, 07 Apr 2008 11:07:24 +0100
To:	bug-CGI-Session [...] rt.cpan.org
From:	Colin Fine <colin.fine [...] pacemicro.com>

RSAVAGE via RT wrote: Show quoted text

> <URL: http://rt.cpan.org/Ticket/Display.html?id=34668 > > > On Mon Apr 07 05:42:17 2008, colin.fine@pacemicro.com wrote: > >

>> Not really, because that discussion is in the context of exiting the >> session. I want you to change the POD for delete() to say that you >>

> must explicitly call flush() after it. > > The new text under delete() (which I wrongly assumed you'd seen) is: > > Warning: Auto-flushing can be unreliable, and always explicitly calling > C<flush()> on the session before the program exits > should be regarded as mandatory until this problem is rectified. > > Clear enough? > >

Sorry, no I haven't seen it - I'm working from the CPAN website, and it doesn't appear to have updated yet. Does it still say that it 'triggers' flush()? Show quoted text

>> No, I wasn't clear. I am calling new($CGI) after the delete(). >>

> > OK. > >

>> The reason I need to do this is that there may be a cookie identifying >> the existing session, which I want to trash. So new(undef) or new($CGI) >> will pick up the existing session, which I then delete before creating a >> new one. >>

> > I'll run some tests tomorrow. It's 8pm here now. > > Frankly, I did not expect new(undef) to use an existing session. We'll see. > >

I think that what's happening is that new(undef) creates a new CGI, which contains the CGISESSID cookie - this is what you want most of the time, but in this particular case I want to trash it. Show quoted text

>> While I'm on the subject of the documentation, I'd like the POD for >> load() to say explicitly that if it does not find an existing session it >> still returns a session object (which is empty). I know that this is >> implicit in the example, but it would be clearer if it said so. >>

> > That's a good point. I'll clarify the docs (tomorrow...). > > >

Thanks for your quick responses. Sleep well! -- *Colin Fine* *Engineering Tools Group Pace Micro Technology plc* */Bringing Technology Home/* *Tel: +44 1274 538038* *Fax: +44 1274 532029* *Victoria Road, Saltaire, West Yorkshire. BD18 3LF _ __www.pacemicro.com_* This E-mail and any attachments hereto are strictly confidential and intended solely for the addressee. If you are not the intended addressee please notify the sender by return and delete the message. You must not disclose, forward or copy this E-mail or attachments to any third party without the prior consent of the sender. Pace Micro Technology plc (registered in England and Wales no. 1672847) whose head office is based at Victoria Road, Saltaire, West Yorkshire, BD18 3LF, UK. Tel +44 (0) 1274 532000 Fax +44 (0) 1274 532010 and at_ http://www.pacemicro.com_. This message has been scanned for viruses by BlackSpider MailControl - www.blackspider.com

Mon Apr 07 20:35:35 2008 RSAVAGE [...] cpan.org - Correspondence added

On Mon Apr 07 05:42:17 2008, colin.fine@pacemicro.com wrote: Show quoted text

> > hopeful that answers that part of your problem. > >

> Not really, because that discussion is in the context of exiting the > session. I want you to change the POD for delete() to say that you must > explicitly call flush() after it.

OK. I've made various changes to the POD for delete() and flush() as per your suggestions, and a new section near the start of the POD. Show quoted text

> While I'm on the subject of the documentation, I'd like the POD for > load() to say explicitly that if it does not find an existing session it > still returns a session object (which is empty). I know that this is > implicit in the example, but it would be clearer if it said so.

OK. I've made various changes to the POD for new() and load() as per your suggestions, and added t/new_with_undef.t and t/load_with_undef.t to demonstrate some effects.

Mon Apr 07 21:00:28 2008 RSAVAGE [...] cpan.org - Correspondence added

Various POD patches and new tests updated in SVN.

Mon Apr 07 21:00:30 2008 RSAVAGE [...] cpan.org - Status changed from 'open' to 'resolved'

Wed May 14 21:05:44 2008 ron [...] savage.net.au - Correspondence added

Subject:	Re: [rt.cpan.org #34668] Documentation on 'delete()' is misleading
Date:	Thu, 15 May 2008 11:04:25 +1000
To:	bug-CGI-Session [...] rt.cpan.org
From:	Ron Savage <ron [...] savage.net.au>

Hi Colin V 4.30 has been released. Does it satisfy your concerns? -- Ron Savage ron@savage.net.au http://savage.net.au/index.html

Wed May 14 21:05:46 2008 The RT System itself - Status changed from 'resolved' to 'open'

Fri May 16 05:37:36 2008 colin.fine [...] pacemicro.com - Correspondence added

Subject:	Re: [rt.cpan.org #34668] Documentation on 'delete()' is misleading
Date:	Fri, 16 May 2008 10:35:37 +0100
To:	<bug-CGI-Session [...] rt.cpan.org>
From:	"Colin Fine" <colin.fine [...] pacemicro.com>

ron@savage.net.au via RT wrote: Show quoted text

> > <URL: http://rt.cpan.org/Ticket/Display.html?id=34668 > > > Hi Colin > > V 4.30 has been released. Does it satisfy your concerns? >

Very much improved. However, every time you talk about requiring an explicit flush() you say 'before the program exits'. No doubt the normal circumstance for wanting to delete a session is when you're finally closing down, but in my case I never do so before exit, because I don't know when the user has finished with the tool. The time I want to delete a session is when the user explicitly or implicitly restarts the tool - so in that case the *first* thing I do is to delete an existing session - and it is then that I need to flush it, before creating a new one. -- *Colin Fine* *Engineering Tools Group Pace plc* */Bringing Technology Home/* *Tel: +44 1274 538038* *Fax: +44 1274 532029* *Victoria Road, Saltaire, West Yorkshire. BD18 3LF _ __www.pace.com_* */Think before you print! Please consider the environment before printing this e-mail./* This E-mail and any attachments hereto are strictly confidential and intended solely for the addressee. If you are not the intended addressee please notify the sender by return and delete the message. You must not disclose, forward or copy this E-mail or attachments to any third party without the prior consent of the sender. Pace Micro Technology plc is registered in England and Wales (Company no. 1672847) and our Registered Office is at Victoria Road, Saltaire, West Yorkshire, BD18 3LF, UK. Tel +44 (0) 1274 532000 Fax +44 (0) 1274 532010. <http://www.pace.com> Save where otherwise agreed in writing between you and Pace (i) all orders for goods and/or services placed by you are made pursuant to Pace's standard terms and conditions of sale which may have been provided to you, or in any event are available at http://www.pace.com/uktcsale.pdf (ii) all orders for goods and/or services placed by Pace are subject to Pace's standard terms and conditions of purchase which may have been provided to you, or in any event are available at http://www.pace.com/uktcpurch.pdf. All other inconsistent terms in any other documentation including without limitation any purchase order, reschedule instruction, order acknowledgement, delivery note or invoice are hereby excluded. This message has been scanned for viruses by BlackSpider MailControl - www.blackspider.com

Fri May 16 08:07:02 2008 ron [...] savage.net.au - Correspondence added

Subject:	Re: [rt.cpan.org #34668] Documentation on 'delete()' is misleading
Date:	Fri, 16 May 2008 22:05:54 +1000
To:	bug-CGI-Session [...] rt.cpan.org
From:	Ron Savage <ron [...] savage.net.au>

Hi Colin Show quoted text

> Very much improved. However, every time you talk about requiring an > explicit flush() you say 'before the program exits'. No doubt the normal

Yep, that's what I was thinking of. I'll re-consider the phraseology... Thanx. -- Ron Savage ron@savage.net.au http://savage.net.au/index.html

Tue May 27 13:31:24 2008 mark [...] summersault.com - Correspondence added

Subject:	Re: [rt.cpan.org #34668] Documentation on 'delete()' is misleading
Date:	Tue, 27 May 2008 13:30:40 -0400
To:	bug-CGI-Session [...] rt.cpan.org
From:	Mark Stosberg <mark [...] summersault.com>

On Fri, 16 May 2008 08:07:03 -0400 "ron@savage.net.au via RT" <bug-CGI-Session@rt.cpan.org> wrote: Show quoted text

> > Queue: CGI-Session > Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=34668 > > > Hi Colin >

> > Very much improved. However, every time you talk about requiring an > > explicit flush() you say 'before the program exits'. No doubt the normal

> > Yep, that's what I was thinking of. I'll re-consider the phraseology... Thanx.

I'm OK with leaving this alone. Someone writing an always-on service should be able to translate the meaning for their application without further complicating the documentation. ( I'm also OK with attempting to further improve the language about this too, as long as clarifies Colin's case doesn't complicate describing the common case. ) Mark

Wed May 28 07:49:33 2008 colin.fine [...] pace.co.uk - Correspondence added

On Tue May 27 13:31:24 2008, mark@summersault.com wrote: Show quoted text

> On Fri, 16 May 2008 08:07:03 -0400 > "ron@savage.net.au via RT" <bug-CGI-Session@rt.cpan.org> wrote: >

Show quoted text

> > I'm OK with leaving this alone. Someone writing an always-on service > should be > able to translate the meaning for their application without further > complicating > the documentation. > > ( I'm also OK with attempting to further improve the language about > this too, > as long as clarifies Colin's case doesn't complicate describing the > common > case. ) >

How about replacing "explicitly calling flush() on the session before the program exits" by "explicitly calling flush() after delete()" just in the 'delete' section? That meets my requirements, and doesn't complicate anything. Show quoted text

> Mark

Fri Oct 31 23:15:37 2008 MARKSTOS [...] cpan.org - Correspondence added

The section for "delete" seems clear on this point now: "always explicitly calling C<flush()> on the session after C<delete()> should be regarded as mandatory". I'm considering this resolved for now. Mark On Wed May 28 07:49:33 2008, COLINFINE wrote: Show quoted text

> On Tue May 27 13:31:24 2008, mark@summersault.com wrote:

> > On Fri, 16 May 2008 08:07:03 -0400 > > "ron@savage.net.au via RT" <bug-CGI-Session@rt.cpan.org> wrote: > >

>

> > > > I'm OK with leaving this alone. Someone writing an always-on service > > should be > > able to translate the meaning for their application without further > > complicating > > the documentation. > > > > ( I'm also OK with attempting to further improve the language about > > this too, > > as long as clarifies Colin's case doesn't complicate describing the > > common > > case. ) > >

> How about replacing "explicitly calling flush() on the session before > the program exits" by "explicitly calling flush() after delete()" just > in the 'delete' section? That meets my requirements, and doesn't > complicate anything. > >

> > Mark

> >

Fri Oct 31 23:15:39 2008 MARKSTOS [...] cpan.org - Status changed from 'open' to 'resolved'

Mon Nov 03 04:26:05 2008 colin.fine [...] pace.co.uk - Correspondence added

On Fri Oct 31 23:15:37 2008, MARKSTOS wrote: Show quoted text

> The section for "delete" seems clear on this point now: > > "always explicitly calling C<flush()> on the session after C<delete()> > should be regarded as mandatory". > > I'm considering this resolved for now. >

Fine Colin

Mon Nov 03 04:26:07 2008 The RT System itself - Status changed from 'resolved' to 'open'

Mon Nov 03 04:26:31 2008 cgi-session-user-owner [...] lists.sourceforge.net - Correspondence added

Subject:	[rt.cpan.org #34668] Documentation on 'delete()' is misleading
Date:	Mon, 03 Nov 2008 09:26:15 +0000
To:	bug-cgi-session [...] rt.cpan.org
From:	cgi-session-user-owner [...] lists.sourceforge.net

You are not allowed to post to this mailing list, and your message has been automatically rejected. If you think that your messages are being rejected in error, contact the mailing list owner at cgi-session-user-owner@lists.sourceforge.net.

Subject:	[rt.cpan.org #34668] Documentation on 'delete()' is misleading
Date:	Mon, 03 Nov 2008 04:26:07 -0500
To:	cgi-session-user [...] lists.sourceforge.net
From:	"Colin Fine via RT" <bug-CGI-Session [...] rt.cpan.org>

Mon Nov 03 04:26:05 2008: Request 34668 was acted upon. Transaction: Correspondence added by COLINFINE Queue: CGI-Session Subject: Documentation on 'delete()' is misleading Broken in: 4.20 Severity: Normal Owner: Nobody Requestors: colin.fine@pace.co.uk Status: resolved Ticket <URL: http://rt.cpan.org/Ticket/Display.html?id=34668 > On Fri Oct 31 23:15:37 2008, MARKSTOS wrote: Show quoted text

> The section for "delete" seems clear on this point now: > > "always explicitly calling C<flush()> on the session after C<delete()> > should be regarded as mandatory". > > I'm considering this resolved for now. >

Fine Colin

Mon Nov 03 09:35:21 2008 MARKSTOS [...] cpan.org - Status changed from 'open' to 'resolved'