Velocity
  1. Velocity
  2. VELOCITY-588

Provide a an ubespector that allows chaining other uberspectors

    Details

    • Type: Improvement Improvement
    • Status: Resolved
    • Priority: Minor Minor
    • Resolution: Fixed
    • Affects Version/s: 1.5
    • Fix Version/s: 1.6
    • Component/s: Engine
    • Labels:
      None

      Description

      In XWiki project we're using Velocity and have a need to write several uberspectors that we'd like to chain (see http://jira.xwiki.org/jira/browse/XWIKI-2182). We're writing a custom uberspector that'll allow chaining other uberspectors but we think that uberspector should best be located in the Velocity project if you're interested.

      Let us know what you think and if you agree we can donate it to you or you could create one from scratch if you agree with the use case.

      Thanks

        Activity

        Hide
        Claude Brisson added a comment -

        ok, I implemented uberspectors chaining using Sergiu Dumitriu's work.

        Now you can specify several class names in runtime.introspector.uberspect.

        For each non-leftmost uberspector:

        • if it implements ChainableUberspector then the chaining is made by calling its wrap method
        • if it doesn't, then the chaining is made via the LinkingUberspector utility class that takes two uberspectors

        Added a testcase and updated docs.

        Show
        Claude Brisson added a comment - ok, I implemented uberspectors chaining using Sergiu Dumitriu's work. Now you can specify several class names in runtime.introspector.uberspect. For each non-leftmost uberspector: if it implements ChainableUberspector then the chaining is made by calling its wrap method if it doesn't, then the chaining is made via the LinkingUberspector utility class that takes two uberspectors Added a testcase and updated docs.
        Hide
        Claude Brisson added a comment - - edited

        I take a quick look to see if what you submitted is sufficient.

        At first look, you do implement both "chaining" (where each new uberspector wraps the previous one) and "linking" (where the first uberspector that returns non-null is retained).

        I'll implement only chaining, since:

        • who can do the most can do the least: chaining can simulate linking
        • your LinkingUberspector is standalone and will continue to work
        Show
        Claude Brisson added a comment - - edited I take a quick look to see if what you submitted is sufficient. At first look, you do implement both "chaining" (where each new uberspector wraps the previous one) and "linking" (where the first uberspector that returns non-null is retained). I'll implement only chaining, since: who can do the most can do the least: chaining can simulate linking your LinkingUberspector is standalone and will continue to work
        Hide
        Sergiu Dumitriu added a comment -

        I had it about 60% done, but during the past month I had several hardware problems, starting with a hdd failure, so I'll have to redo it.

        Show
        Sergiu Dumitriu added a comment - I had it about 60% done, but during the past month I had several hardware problems, starting with a hdd failure, so I'll have to redo it.
        Hide
        Nathan Bubna added a comment -

        Optimistically marking this as something to do for 1.6. Sergiu, are you still working on this? Or should someone else take a shot?

        Show
        Nathan Bubna added a comment - Optimistically marking this as something to do for 1.6. Sergiu, are you still working on this? Or should someone else take a shot?
        Hide
        Nathan Bubna added a comment -

        Thanks, Sergiu. I'll look forward to it!

        Show
        Nathan Bubna added a comment - Thanks, Sergiu. I'll look forward to it!
        Hide
        Sergiu Dumitriu added a comment -

        Hi,

        Yes, I'm the creator of the code. It is currently under LGPL because that is the license for all XWiki code, but I'll gladly give the implementation to the Apache foundation. I'll work on providing a clean patch for the velocity trunk, but it will take some time, as I'm busy with my PhD and with XWiki.

        Show
        Sergiu Dumitriu added a comment - Hi, Yes, I'm the creator of the code. It is currently under LGPL because that is the license for all XWiki code, but I'll gladly give the implementation to the Apache foundation. I'll work on providing a clean patch for the velocity trunk, but it will take some time, as I'm busy with my PhD and with XWiki.
        Hide
        Nathan Bubna added a comment -

        Sergiu,

        I think this is something i'd like to move forward with implementing this in Velocity. However, your patch and the SVN version are under LGPL. Are you the creator/copyright owner? If so, could you create an ASL 2.0 version for us to use? If not, that's fine, i'll just make my own clean attempt at it.

        Show
        Nathan Bubna added a comment - Sergiu, I think this is something i'd like to move forward with implementing this in Velocity. However, your patch and the SVN version are under LGPL. Are you the creator/copyright owner? If so, could you create an ASL 2.0 version for us to use? If not, that's fine, i'll just make my own clean attempt at it.
        Hide
        Nathan Bubna added a comment -

        You're right. The wrapping mechanism allows a lot more flexibility. I hadn't thought of that. I'll try to find some more time (hopefully this week) to look at your more up to date stuff. I really like this idea. Since it seems we might be waiting a while on a bug fix before we roll out a 1.6 release, i might try to get something of this into the trunk soon. It's a great idea, and i'm already thinking of ways i'd like to use it.

        Show
        Nathan Bubna added a comment - You're right. The wrapping mechanism allows a lot more flexibility. I hadn't thought of that. I'll try to find some more time (hopefully this week) to look at your more up to date stuff. I really like this idea. Since it seems we might be waiting a while on a bug fix before we roll out a 1.6 release, i might try to get something of this into the trunk soon. It's a great idea, and i'm already thinking of ways i'd like to use it.
        Hide
        Sergiu Dumitriu added a comment -

        The patch is outdated, a better source is in the XWiki repository, at http://svn.xwiki.org/svnroot/xwiki/xwiki-platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/introspection/

        Yes, it works pretty well until now. I also have Chainable Introspectors locally, but the code requires some cleaning before committing. Here is a proposed architecture for how we're planning to use custom introspectors/uberspectors in XWiki: http://www.nabble.com/Deprecating-all-the-APIs-in-favor-of-a-Velocity-uberspecting-chain-td17208816.html#a17208816

        Your idea is good, too. As a pro, it keeps things simple. As a con, it isn't as flexible as the chainable one, basically because it allows enhancing the list of returned methods, but doesn't allow restricting that list, or performing additional tasks with the methods found by previous introspectors/uberspectors. For example, we can't block access to methods that don't meet the access rights restrictions (see RightsCheckingIntrospector in the XWiki proposal), and we can't implement DeprecatedCheckIntrospector that relies entirely on the results of the previous introspectors.

        I guess I could write a different Uberspector that uses simple linking, instead of chaining, and the user (application developer) can choose the one that better suits his needs.

        Show
        Sergiu Dumitriu added a comment - The patch is outdated, a better source is in the XWiki repository, at http://svn.xwiki.org/svnroot/xwiki/xwiki-platform/core/trunk/xwiki-velocity/src/main/java/org/xwiki/velocity/introspection/ Yes, it works pretty well until now. I also have Chainable Introspectors locally, but the code requires some cleaning before committing. Here is a proposed architecture for how we're planning to use custom introspectors/uberspectors in XWiki: http://www.nabble.com/Deprecating-all-the-APIs-in-favor-of-a-Velocity-uberspecting-chain-td17208816.html#a17208816 Your idea is good, too. As a pro, it keeps things simple. As a con, it isn't as flexible as the chainable one, basically because it allows enhancing the list of returned methods, but doesn't allow restricting that list, or performing additional tasks with the methods found by previous introspectors/uberspectors. For example, we can't block access to methods that don't meet the access rights restrictions (see RightsCheckingIntrospector in the XWiki proposal), and we can't implement DeprecatedCheckIntrospector that relies entirely on the results of the previous introspectors. I guess I could write a different Uberspector that uses simple linking, instead of chaining, and the user (application developer) can choose the one that better suits his needs.
        Hide
        Nathan Bubna added a comment -

        Hi Sergiu,

        My apologies for my slow response. But today i'm catching up on some Velocity stuff.

        This looks pretty good. Should i assume that it is working well for you? I only wonder if it might be simpler to keep Uberspectors in an ordered list (in the root/parent Uberspector) and just interrogate them in order until one returns a non-null value. This way you might be able to avoid requiring them to implement a ChainableUberspector interface.

        Of course, i haven't tried implementing it that way. Have you? What do you think of the idea?

        Show
        Nathan Bubna added a comment - Hi Sergiu, My apologies for my slow response. But today i'm catching up on some Velocity stuff. This looks pretty good. Should i assume that it is working well for you? I only wonder if it might be simpler to keep Uberspectors in an ordered list (in the root/parent Uberspector) and just interrogate them in order until one returns a non-null value. This way you might be able to avoid requiring them to implement a ChainableUberspector interface. Of course, i haven't tried implementing it that way. Have you? What do you think of the idea?
        Hide
        Sergiu Dumitriu added a comment -

        This is the first version of the implementation done for XWiki. Please review it, and if it looks good I can make a patch for the velocity trunk.

        Show
        Sergiu Dumitriu added a comment - This is the first version of the implementation done for XWiki. Please review it, and if it looks good I can make a patch for the velocity trunk.
        Hide
        Nathan Bubna added a comment -

        Yeah, this would be great. I'm messing around with uberspectors that magically provide utility functions for all references (e.g. instead of $display.measure($foo), i could just do $foo.measure()) and have been thinking about ways to make the uberspectors more extensible. Chaining would be fantastic.

        Show
        Nathan Bubna added a comment - Yeah, this would be great. I'm messing around with uberspectors that magically provide utility functions for all references (e.g. instead of $display.measure($foo), i could just do $foo.measure()) and have been thinking about ways to make the uberspectors more extensible. Chaining would be fantastic.
        Hide
        Claude Brisson added a comment -

        That is a brillant idea.

        As I understand your use cases, the first uberspector in the chain would check for annotations and, for instance, add log or security.

        We are interested, of course. Plus, using chained uberspectors can help us to factorize existing code.

        and I already think to another use case :

        a view tool chained uberspector that allows syntaxes like #set( $session.foo = bar ) instead of #set( $junk = $session.setAttribute('foo','bar')

        I may have the time to code that these days, I'm not sure. Let us now if you start coding it, I'll also add a comment here if I do so first on my side.

        Show
        Claude Brisson added a comment - That is a brillant idea. As I understand your use cases, the first uberspector in the chain would check for annotations and, for instance, add log or security. We are interested, of course. Plus, using chained uberspectors can help us to factorize existing code. and I already think to another use case : a view tool chained uberspector that allows syntaxes like #set( $session.foo = bar ) instead of #set( $junk = $session.setAttribute('foo','bar') I may have the time to code that these days, I'm not sure. Let us now if you start coding it, I'll also add a comment here if I do so first on my side.

          People

          • Assignee:
            Claude Brisson
            Reporter:
            Vincent Massol
          • Votes:
            0 Vote for this issue
            Watchers:
            1 Start watching this issue

            Dates

            • Created:
              Updated:
              Resolved:

              Development