Beyond TMM, analysis of a bug and aftermath of architecture work: What happens now?

Posted on by LSN 6 Comments

Bo Durban of Moxie data wrote to tell me about what I am sure is a bug even though I haven’t repro’d it yet. Not horrible, easy workaround, but a bug or bugs nonetheless.

What, you thought there weren’t any?  We’re very proud of the reporting work, and we appreciate your high opinion of it! Still, while the new-in-SP2 architecture elements are quite robust in general, this is bound to happen, and it will happen, more than once, I’m sure.

Try not to faint.

Backstory

FXListener has some supported helper classes to perform the FX and GFX services supported by ReportBuilder extensions out-of-the box.

We shipped more FX and GFX classes than FXListener has specialized class information for, and this distinction is critical, so I’ll say this again:

The ones you see supported explicitly by FXListener with *class, *classlib, *module, and explicit instantiation are the ones required to supply behavior when you use ReportBuilder dialogs to specify extension behavior; we didn’t want to take the chance that these weren’t available at runtime.  We also explicitly instantiate the one that replaced UpdateListener’s therm behavior for default runtime feedback. UpdateListener got replaced by fxTherm in SP2 for some good reasons, which I’ll explain if anybody cares, and we didn’t want that behavior to suddenly “go missing” in default output runs either.

You can put other ones into the collection with impunity, and you can supply setup behavior that ensures yours are available in any number of ways — including, if you wish, extending the checkCollectionMembers method if you wish, with or without additional *class, *classlib, *module properties added to your subclass. It probably makes more sense to simply set up the _oReportOutput collection of reportlisteners with the appropriate collection members at the beginning of your application.  If you build design-time extension behavior, you can also ensure that any previews from the designer are preloaded with your collection members very easily.

Read http://spacefold.com/articles/tmm/ReportOutputApp and the original helpfile entry it extends, if you are unfamiliar with the idea of the cache.  Read Colin’s article http://spacefold.com/articles/DataVizReports for step by step instructions if you are unfamiliar with the idea of adding design-time controls into the builder (specifically the section on Enabling the custom behavior by default will show you how the cache applies in this case). You could easily register behavior into your report design environment that ensures the availability of your decoration objects when the designer is opened.  Start here http://spacefold.com/articles/RSFundamentals-part1  if you are unfamiliar with the idea of registering new behavior elements into the Report Builder.

Let’s return to our backstory.

Each of the supported helper classes (which included, kind of late and clearly very hastily, gfxNoRender) have the own get* method, called by CheckCollectionMembers, precisely because the logic to decide when they are useful is different for each type. (This is covered in FxListener. )

For example, I really thought it was possible that the memberscript object might be useful even to a Successor (and I still do).  But, generally speaking, fx and gfx behavior is only supposed to be going on for the lead listener, the one attached to the internal report engine, not for Successors.

This fact is pretty obvious if you look at fxListener’s SendFX method, which has the following call at the top:

IF THIS.IsSuccessor
     * Only the lead does this work.    
  RETURN m.liRenderBehavior
ENDIF

 

CheckCollectionMembers doesn’t have the same unilateral call at the top, just in case some collection objects are useful even in Successors.

As I told Bo and Martin Haluza (http://www.eqeus.com/)  this morning, I think that I should have put such a check at the top of fxListener.getNoRenderGFXobject(), and probably at the top of fxListener.getRotateGFXObject() as well.  There is no earthly reason to check for the availability of, or the necessity to load, a gfx collection member in a Successor.  Only the lead listener will be decorating and certainly only the lead should be trying to write to the page using GDIPlus calls.  (For one thing, the return value that determines whether the additional GDIPlus rendering will replace or add to the native rendering only makes sense in the lead listener, which is communicating with the engine.)

Bo asked me “does this mean that a Successor should not descend from FXListener?” And the answer is there is absolutely nothing wrong with a Successor deriving from FXListener or one of its derived classes.  The job of a Successor is to create extension output result types.  The same listener class may do this sometimes as the lead (in which case it may also be decorating) and sometimes as a Successor (in which case it receives the benefit of the decorations, but is not in charge of creating them).

The real-life scenario(s)

In fact, Bo and Martin have a classic scenario that both illustrates this and manifests the bug:

Bo has written a listener that he uses (for some reason) as the lead.

I am not sure why that is, and I haven’t asked him. If Bo is writing content decorations he could do the whole thing as fx and gfx objects and he shouldn’t care what listener “hosts” his decoration objects.  But perhaps he is adding more value to fxListener in other ways… and you could see the bug even if he only creates fx and gfx objects… so just assume Bo’s listener, or your listener, is the lead listener for some good reason…  Read on.

Meanwhile Martin has written a listener, also derived from FXListener, that creates extension output.  In fact, as you probably already know, he can chain many copies of his listener together as Successors, to create multiple forms of output (such as PDF, DOC, and HTML) in the same report run.

The fact that Martin could chain many different copies of his own listener together for different forms of output should illustrate why you don’t need multiple listener classes contributed by different authors to see this problem.  One of Martin’s fxListener-derived objects will function as lead, the others will be Successors.  Read on.

So now Martin’s Successor listener (as well as the lead) gets a gfxNoRender helper object, and gfxNoRender does a LoadReport check to see if any NoRender instructions have been included in this FRX.

gfxNoRender has to do this work in LoadReport if it wants to take advantage of SP2’s ability to change the contents of the FRX dynamically during the LoadReport event.  (If you don’t already know about this, start here and read the referenced docoids).  Since LoadReport occurs before the engine has loaded up the contents of the FRX, gfxNoRender “peeks ahead” and opens the FRX temporarily at this point.  It actually has to do a lot of work to do this, check to see if it’s a built-in FRX, etc.

Unfortunately — and Bo identified this — the Successor doesn’t yet have the information it needs to get into the correct FRXDataSession. At the time I wrote this part of the _ReportListener code, I don’t even think we had the correct FRXDataSession information in LoadReport, and I certainly knew that the engine-provided copy of the FRX would not be available in it yet. As a result _ReportListener doesn’t do this until BeforeReport, and, Bo’s right, we should certainly do it earlier now.  Arrgh #1.

One point that Bo missed was that my matching reset of the Successor’s FRXDataSession in UnloadReport is there but it is not working correctly; the lead still has its FRXDataSession value filled out so the Successor’s isn’t get set back to -1, as it should.  That’s arggh #2.

But the thing that really had me gnashing my teeth (big arggh #3) here was… why is a Successor even doing these checks?  Hence my suggesting that the get* methods for the supported collection members need to be fixed.

So what happens now?

We’ve got at least 3 things we’d like to fix here.

You have some easy, and obvious, workarounds you can use in your derived classes right now, which you can implement without worrying that they will screw up anything later when your classes re-inherit from a newer version of FXListener:

  • As Bo suggests, you can assign the correct value of FRXDataSession in LoadReport in your derived class:

    IF NOT ISNULL(THIS.Successor)
       THIS.Successor.FRXDataSession = THIS.FRXDataSession
    ENDIF
    * the above should come before:
    DODEFAULT()

     

  •  As I would also suggest, you should also prevent the Successor from doing a lot of extra work, which it is currently and ridiculously doing.  You can do it like this:

    LOCAL lcgfxClassName
    IF NOT ISNULL(THIS.Successor)
       THIS.Successor.FRXDataSession = THIS.FRXDataSession
    IF TYPE    (“THIS.Successor.gfxNoRenderClass”)=”C”
    lcgfxClassName =     THIS.Successor.gfxNoRenderClass
          THIS.Successor.gfxNoRenderClass = “”
       ENDIF
    ENDIF
    * the above should come before DODEFAULT():
    DODEFAULT()
    * the below should come after DODEFAULT():
    IF VARTYPE(lcgfxClassName) = “C”
       THIS.Successor.gfxNoRenderClass = lcgfxClassName
    ENDIF

     

  • For  neatness’ sake, you might also want to change the FRXDataSession of your listener to -1 after a DODEFAULT() in UnloadReport IF THIS.IsSuccessor… although I don’t think the current behavior is hurting anything in particular.

Is that all there is?

Now, having said that… how can we fix all 3 items in FXListener to make them available to everybody?

Right now you know only slightly less than I do about that — and feel free to post your suggestions here, you’ve never been shy before and I’m sure you have opinions now. Just don’t expect me to suggest anything resembling a Microsoft-hosted refresh that would include these items. Even if there were such a thing, remember the part where this is bound to happen “more than once”?

I’m going to have a chat with Bo and Martin and Colin, and get back to you about this.