Uploaded image for project: 'XWiki Platform'
  1. XWiki Platform
  2. XWIKI-8926

WatchList should use the same Velocity Context as the one used by other Scheduler jobs

    XMLWordPrintable

Details

    • Improvement
    • Resolution: Unresolved
    • Major
    • None
    • 5.0-milestone-1
    • {Unused} Watchlist
    • None
    • Unknown

    Description

      The reason is that a new VelocityContext is created in WatchListNotifier.sendEmailNotification:

              // Prepare email template (wiki page) context
              VelocityContext vcontext = new VelocityContext();
              vcontext
                  .put(XWIKI_USER_CLASS_FIRST_NAME_PROP, userObj.getProperty(XWIKI_USER_CLASS_FIRST_NAME_PROP).getValue());
              vcontext.put(XWIKI_USER_CLASS_LAST_NAME_PROP, userObj.getProperty(XWIKI_USER_CLASS_LAST_NAME_PROP).getValue());
              vcontext.put("events", events);
              vcontext.put("xwiki", new com.xpn.xwiki.api.XWiki(context.getWiki(), context));
              vcontext.put("util", new com.xpn.xwiki.api.Util(context.getWiki(), context));
              vcontext.put("msg", context.getMessageTool());
              vcontext.put("modifiedDocuments", modifiedDocuments);
              vcontext.put("previousFireTime", previousFireTime);        
              vcontext.put("context", new Context(context));
      

      Ideally this code should be:

              // Prepare email template (wiki page) velocity context by cloning the velocity context saved in the scheduler
              // job, cloning it and adding extra bindings.
              VelocityContext vcontext = (VelocityContext) ((VelocityContext) context.get("vcontext")).clone();
              vcontext
                  .put(XWIKI_USER_CLASS_FIRST_NAME_PROP, userObj.getProperty(XWIKI_USER_CLASS_FIRST_NAME_PROP).getValue());
              vcontext.put(XWIKI_USER_CLASS_LAST_NAME_PROP, userObj.getProperty(XWIKI_USER_CLASS_LAST_NAME_PROP).getValue());
              vcontext.put("events", events);
              vcontext.put("modifiedDocuments", modifiedDocuments);
              vcontext.put("previousFireTime", previousFireTime);        
      

      However this is not working because the velocity context saved in the job data is reset in the call chain, as follows:

      WatchListJob.executeJob()
      -> setPreviousFireTime();
        -> this.context.getWiki().saveDocument(doc, "Updated last fire time", true, this.context);
          -> om.notify(new DocumentUpdatedEvent(doc.getDocumentReference()), doc, context);
            -> ActivityStreamImpl (listener): displayTitle = originalDoc.getRenderedTitle(Syntax.XHTML_1_0, context);
              -> XWikiDocument:getRenderedTitle
                -> XWikiDocument:backupContext
                  -> Utils.getComponent(VelocityManager.class).getVelocityContext();
                    -> VelocityContext vcontext = (VelocityContext) this.execution.getContext().getProperty(VelocityExecutionContextInitializer.VELOCITY_CONTEXT_ID);
                       not in sync! 7 items vs 163 in XWikiContext (note: in the 7 there's not even $xwiki)
                       163 overwritten by:
                       ((Map<String, Object>) this.execution.getContext().getProperty("xwikicontext")).put("vcontext", vcontext);
      

      So initially there are 163 bindings in the velocitycontext and after the call to setPreviousFireTime() there are only 7 left and $xwiki is not even in there which leads to the watchlist email being sent not having a good titles for modified documents (it displays "$currentDoc.getDisplayTitle()").

      We need to decide what to do. Some thoughts:

      • We need to decide if we want event listeners to be able to change the context. Answer: probably. So we can't isolate each event listener by cloning the passed context
      • we probably need to understand why "velocitycontext" set in the EC has diverged from the "vcontext" set in the xwikicontext. And understand who's the master source of velocity context bindings and why we don't take it from xwikicontext in getVelocityContext().
      • I don't fully understand this code in backupContext and in any case it looks strange to do a get to change something...
              // Trigger the initialization of the new Velocity and Script Context. This will also ensure that the Execution
              // Context and the XWiki Context point to the same Velocity Context instance. There is old code that accesses
              // the Velocity Context from the XWiki Context.
              Utils.getComponent(VelocityManager.class).getVelocityContext();
      

      I need some brainstorming help to take some decisions on this...

      Attachments

        Issue Links

          Activity

            People

              Unassigned Unassigned
              vmassol Vincent Massol
              Votes:
              0 Vote for this issue
              Watchers:
              1 Start watching this issue

              Dates

                Created:
                Updated: