Added script support for ExtendedItem

[claesrosell]

@hvbtup I am not that sharp with the scripting stuff, but these changes add script support for OnCreate and OnRender for all ExtendedItem. Do you think this is enough?

[wimjongman]

@claesrosell is this for 4.14?

[claesrosell]

Not necessarily. I have no personal interest in that. I just decided to look into it since @hvbtup consider it important with script support for the QR-code things and I promised him long ago to take a look at it. I think it makes sense to wait with this until the QR-code extension is picked up again.
Strategically I think it makes sense with QR-code support in Birt.

[claesrosell]

Come to think about it: if RotatedText makes it into the 4.14 release, perhaps it would be good with this patch as well?

[hvbtup]

I was ill last week (and still am). I'll need time to take a look at this.
From my POV this can wait until after 4.14.

[hvbtup]

I tested, with partial success, but I think there's more to do.

test_rotated_text_with_script.zip

hat I would like to have is sth like this (for the RotatedTextItem):

this.setRotationAngle(-45);

Note: I think it is not necessary to show the available methods in auto completion, this is probably out of scope for anyway.

But we want:

1. The typical Scripts (e.g. onCreate) should run at all.
2. It should be possible to set style properties and generic properties like width or height in onCreate.
3. It should be possible to (some of) the methods of the ExtendItem in the onCreate script,
   e.g. for the RotatedTextItem to set the rotation angle (the text is an expression anyway here).

Current Status:

1. DONE - at least with this PR, maybe it did work before already.
2. TODO - workaround exists, see previous grid row
3. TODO - no workaround

I looked at the code briefly.

One of the main differences between ExtendedItemScriptExecutor.java and eg DynamicTextScriptExecutor.java is the following:

When the onCreate event is called, the DynamicTextScriptExecutor creates an instance of DynamicTextInstance, which inherits indirectly from ReportElementInstance and thus provides the method getStyle etc.

When the onCreateevent is called, the ExtendedItemScriptExecutor creates an instance of OnCreateEvent, which derives from ReportEvent only and thus does not provide getStyle etc.

[hvbtup]

With some changes (in fact quite a lot of) I am able to solve issue 2.

That is, I am able to use this.getStyle().backgroundColor = "yellow"; in the onCreate event of a rotated text item.
For this, I had to change 10 files.
Basically I replaced IReportEventContext with ExecutionContext like this:

See the attached diff.txt

Not quite sure if it is a good idea, though.

Issue 3 is much harder.

What I have is a fake solution:
I can use this.setRotationAngle(-45) in the onCreate event and that is in fact working.

But the implementation is a hack which is only working exactly for this example, by adding an additional method to OnCreateEvent.java:

public void setRotationAngle(int angle) {
	System.err.println("Not implemented: setRotationAngle");
	Object o = handle.getElement();
	if (o instanceof ExtendedItem) {
		ExtendedItem itm = (ExtendedItem)o;
		org.eclipse.birt.report.model.api.extension.IReportItem extensionElement = itm.getExtensibilityProvider()
				.getExtensionElement();
		try {
			Class c = extensionElement.getClass();
			Method m = c.getMethod("setRotationAngle", Integer.TYPE);
			m.invoke(extensionElement, angle);
			System.out.println(itm.toString());
		} catch (Exception ignore) {
			ignore.printStackTrace();
		}
	}
}

So, what is working for issue 3: Calling a Java method per Reflection (args matching in more complicated cases could be difficult, though).
But what is not working: You see that I created a specifiy method for RotatedTextItem to the class OnCreateEvent.java which is generic for all ExtendedItems.
What we need instead is either a generic solution (something like catch-all method for JS calls to ExtendedItems) or the ability for ExtendedItem authors to say which methods can be called from JS, and use this somehow in the ScriptExecutor. I don't know, how, though.

BTW I don't know if (with my changes) the chart scripting is still working; didn't test this.

[hvbtup]

What we need instead is either a generic solution (something like catch-all method for JS calls to ExtendedItems) or the ability for ExtendedItem authors to say which methods can be called from JS, and use this somehow in the ScriptExecutor

A rough idea how this could work:

The plugin for the ExtendedItem could provide a factory method which would be called by ExtendedItemScriptExecutor.
It would return an instance of an extension-specific subclass of ReportEvent, which adds the missing methods and delegates them to handle.element.getExtensibilityProvider().getExtensionElement() similar to what I showed above in setRotationAngle, just without all this reflection stuff. So, these glue methods would be a snap to write for the extension author.

Probably the original authors will say that all of this is already supported in the extension framework, but I have to admit that I don't grep how it works really. There are only 2 other extensions apart from the rotated text example: CrossTab and Chart. Both of them are by far too complicated to understand how they work or to serve as an example.

[hvbtup]

A working solution

I created a branch on my repo:

https://github.com/triestram-partner/birt/tree/Test-ExtendedItem-Script-Support

The code is based on PR #1531.
Only the last commit 6f1f4db contains changes regarding this PR.

It seems to work, but I am not at all sure if the programming style is OK.

What I did in addition to the changes I already mentioned yesterday:

I created a new interface IScriptableExtendedItem.
Inside ExtendedItemScriptExecutor, first check if the item implements this interface.
If yes, call its buildEvent method which has to create an instance of a subclass of OnCreateEvent (see the RotatedTextItem class for an example).
This subclass inherits all the common methods like getStyle() from ReportItemInstance and the developer adds simple wrappers for those methods that should be accessible from Javascript.

So this is not really complicated.

What is not implemented

Auto-completion for the available JS methods, e.g. for a Rotated Text Item, auto-completion does not offer setRotationAngle. But I'm pretty sure this could be added.

Things to think about:

In case of onPrepare, there isn't content yet, so the method uses null instead.
I'm not sure what the onPrepare JS event could be used for in real world, so this might be a problem.

Not sure if the architecture is clean and good and in compliance with other BIRT code.

Not sure if the same result could be achieved with a less invasive code change.

I had to (trivially) change the CrossTab implementation due to the switch from IReportContext to ExecutionContext.
So, maybe custom extended items we don't know of would need suche a trivial code change as well.

Maybe you @claesrosell or others (@wimjongman, @speckyspooky) have suggestions for improving my working code.

[hvbtup]

Exhibit A:

created using this.getStyle().background = "yellow;" resp. this.setRotationAngle(-45);

[hvbtup]

I'd appreciate if someone could take a look at my branch with a focus on the things I mentioned earlier regarding code quality etc.
Ideally we would add autocompletion support for the in the JS editor for the events, but even without that, extended items would be much more useful and we could continue the effort for a QR Code item as an example (I abandoned that PR due to the lacking script support).

[claesrosell]

I'd appreciate if someone could take a look at my branch with a focus on the things I mentioned earlier regarding code quality etc. Ideally we would add autocompletion support for the in the JS editor for the events, but even without that, extended items would be much more useful and we could continue the effort for a QR Code item as an example (I abandoned that PR due to the lacking script support).

I will try to take a look into this later this week. Perhaps in the weekend.

[wimjongman]

Move this item after the next if block. No need to get the handle if it is not used.

[wimjongman]

Same here.

[hvbtup]

BTW I spent some hours to find out how JS script auto-completion works, but I gave up.
What I think is: The extended item plugin must provide an implementation for the extension point
org.eclipse.birt.report.model.IScriptableObjectClassInfo.

But I failed to do so.
I created a simple class

package org.eclipse.birt.sample.reportitem.rotatedtext;

import org.eclipse.birt.report.model.api.metadata.IClassInfo;
import org.eclipse.birt.report.model.api.scripts.ClassInfo;
import org.eclipse.birt.report.model.api.scripts.IScriptableObjectClassInfo;

public class RotatedTextScriptableClassInfo implements IScriptableObjectClassInfo {

        public RotatedTextScriptableClassInfo() {

        }

        /*
         * (non-Javadoc)
         *
         * @see org.eclipse.birt.report.model.api.scripts.IScriptableObjectClassInfo#
         * getScriptableClass(java.lang.String)
         */
        @Override
        public IClassInfo getScriptableClass(String className) {
                try {
                        Class clazz = Class.forName(className);
                        ClassInfo info = new ClassInfo(clazz);
                        return info;
                } catch (ClassNotFoundException | RuntimeException e) {
                        return null;
                }
        }

}

just as a starting point (copied from the Chart item) and added this to plugin.xml:

   <extension
         point="org.eclipse.birt.report.model.IScriptableObjectClassInfo">
      <scriptableClassInfo
            class="org.eclipse.birt.sample.reportitem.RotatedTextScriptableClassInfo"
            extensionName="RotatedText">
      </scriptableClassInfo>
   </extension>

But the extension manager fails to load this class and I don't know why.

[hvbtup]

But the extension manager fails to load this class and I don't know why.

That was caused by a combination of two bugs in my code: First, I added the new class at the wrong place (tutorial_1 instead of tutorial_2) and I missed a level in the fully qualified class name in plugin.xml (BTW shouldn't the IDE provide better support here? I had to manually edit plugin.xml).
After fixing those mistakes, the class RotatedTextScriptableClassInfo was loaded, but the method getScriptableClass never gets called.
Sigh...

[wimjongman]

What do you mean by auto completion? Is that CTRL+SPACE like we have in the editor?

[hvbtup]

You don't even have to hit ctrl-space.
Just type this. in the editor eg for the onCreate event.
Then you'll see suggestions like getStyle(), but what's missing for eg the RotatedTextItem ist the method setRotationAngle (or a property rotationAngle) - that is, the available methods which are specific for the extended item are missing.

[wimjongman]

Who takes care of that? Is it a js editor we pull in from webtools?

[hvbtup]

I don't know, but all this is BIRT code, not from somewhere else.
A starting point for investigation might be to set breakpoints at the various setVariable methods inside JSSyntaxContext.java.
The interesting case here is when the variable name is this.
Then there are some getMethods calls involved.
I think the fact that we get a generic class / class name (I don't know the exact value right now) instead of RotatedText.... is directly related to the issue.
Do you know anyone who knows a bit more how all this ExtendedItem stuff is supposed to work?
Maybe I'm not on the right track here and an expert will come up with a relatively simple solution...
I think if we could add auto-completion support to the RotatedText item and add some more comments to the code so that it can server as a (relatively) simple, documented example, that would allow others to come up with useful additions, too.

[hvbtup]

Just wanted to inform you that I spent some more afternoons with debugging, but I have to admit that I don't really understand what's going on. I was not able to add auto-completion support for the specific methods of the RotatedText item as an example and I give up for now.