Bug 196526 - add a wizard for simplified EJB timer
add a wizard for simplified EJB timer
Status: RESOLVED FIXED
Product: javaee
Classification: Unclassified
Component: EJB
7.0
PC Linux
: P3 (vote)
: 7.1
Assigned To: Martin Fousek
issues@javaee
: PLAN
Depends on:
Blocks:
  Show dependency treegraph
 
Reported: 2011-03-09 22:50 UTC by David Konecny
Modified: 2011-06-02 20:42 UTC (History)
2 users (show)

See Also:
Issue Type: DEFECT
:


Attachments
patch v1 (63.72 KB, text/plain)
2011-03-23 14:54 UTC, Martin Fousek
Details
patch v2 (69.11 KB, text/plain)
2011-03-24 13:20 UTC, Martin Fousek
Details
patch v3 (68.86 KB, patch)
2011-04-06 05:59 UTC, Martin Fousek
Details | Diff
patch v4 (56.40 KB, patch)
2011-04-22 19:04 UTC, Martin Fousek
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description David Konecny 2011-03-09 22:50:43 UTC
[see chapter 18 in EJB spec 3.1]

The wizard would be regular Java class wizard with one extra field for 'Schedule'. Default value for Schedule field could be something like:

second="0", minute="*", hour="9-17", dayOfMonth="*",month="*", dayOfWeek="Mon-Fri", year="*"

which means each minute between nine to five whole year between Monday to Friday. It might be good idea to show a description box underneath saying that it is cron syntax and few other syntax examples might be shown for illustration. Or perhaps it could be covered in Help system.

Example of generated class is:

package org.xxx;

import javax.ejb.Stateless;
import javax.ejb.LocalBean;
import javax.ejb.Schedule;
import javax.ejb.Timer;

/**
 *
 */
@Stateless
@LocalBean
public class xxxTimerBean {

    @Schedule(second = "0", minute = "*", hour = "9-17", dayOfMonth = "*", month = "*", dayOfWeek = "Mon-Fri", year = "*")
    private void myTimer(final Timer t) {
        System.out.println("timer event "+new java.util.Date());
    }
}
Comment 1 Martin Fousek 2011-03-11 15:19:49 UTC
I read thru the chapter and I'm not sure if I fully understand you. You mean to create wizard for new file type like "Timed Session Bean" which would be the same like SessionBean wizard and it would have Schedule part in addition and then it would create similar code to the mentioned one? In that case I would imagine it like an extension of actual wizard for session beans.

BTW, it would create just a sample code, wouldn't be better to create action into code generation which would be enabled when cursor would be inside some method and it would invoke dialog for creating @Schedule annotation using UI?

Or did I understand it wrongly and you would like to have wizard for generation of TimedObjects (but in that case there aren't any @Schedule items)?

Thanks in addition for specifying that...
Comment 2 David Konecny 2011-03-13 19:45:06 UTC
Here is more details how I was thinking about it but please feel free to disagree or suggest different solution. 

(In reply to comment #1)
> I read thru the chapter and I'm not sure if I fully understand you. You mean to
> create wizard for new file type like "Timed Session Bean" which would be the
> same like SessionBean wizard and it would have Schedule part in addition and
> then it would create similar code to the mentioned one? In that case I would
> imagine it like an extension of actual wizard for session beans.

Yes, exactly. It is just a customized version of Session Bean wizard to create some initial code which users can customize. It's been suggested we provide such a wizard in two independent offline discussion.

> BTW, it would create just a sample code, wouldn't be better to create action
> into code generation which would be enabled when cursor would be inside some
> method and it would invoke dialog for creating @Schedule annotation using UI?

We could do that as well. Sounds useful doesn't it? On the other hand timers are needed relatively rarely so offering this code generator on each method might be just cluttering the UI. Something to think about.

> Or did I understand it wrongly and you would like to have wizard for generation
> of TimedObjects (but in that case there aren't any @Schedule items)?

No, you got it right. I would call it "Timer Session Bean" and it would look pretty much like current Session Bean wizard with exception that "Stateful" option would not be displayed and a new Schedule field would be shown above Session Type as it is more important in this instance.

But as I said, please, feel free to disagree with me if you think it should be done differently. :-)

Btw. TimedObject is older way how EJB timers were implemented and this issues ignores it and concentrate on new @Schedule annotation only.
Comment 3 Martin Fousek 2011-03-14 06:04:37 UTC
Thanks a much for additional explanation.

I have a little bit mixed feelings about that since the wizard would contain new filetype (in Enterprise JavaBeans category) and it would create almost the same Session Bean just with one annotated sample method. So a little bit trimmed idea... What about add into Session Bean wizard checkbox, which would extend the dialog about Schedule section? Theoretically we could do the same then also for MDBs.

You are perhaps right that the new insert code item would uselessly slow down the UI if it is rarely used. Also agree that we should focus on new EJB features - it was just additional question for specifying our imagines.
Comment 4 David Konecny 2011-03-14 22:17:16 UTC
(In reply to comment #3)
> I have a little bit mixed feelings about that since the wizard would contain
> new filetype (in Enterprise JavaBeans category) and it would create almost the
> same Session Bean just with one annotated sample method.

Yes. It is more of a marketing for Timer Bean than a feature for daily use. I do not mind it there as long as it is not the first item in the list of wizards.

> So a little bit
> trimmed idea... What about add into Session Bean wizard checkbox, which would
> extend the dialog about Schedule section?

As I said earlier I would not want to include "Schedule" option anywhere else as I think it is just not going to be used that often. And single, dedicated, easily to find wizard which can be in future easily removed (if necessary) seem to me like a good start (and the end).

> Theoretically we could do the same
> then also for MDBs.

Let MDBs rest in piece. :-) If EE 7 decide to refresh JMS spec then we may do some work there but otherwise I would keep it as is. 

> You are perhaps right that the new insert code item would uselessly slow down
> the UI if it is rarely used.

It would be nice though if user starts typing @Schedule in editor to give them some richer code completion or some help with syntax to set up schedule correctly. Even just replacing "@Schedule" with something like "@Schedule(second = "0", minute = "*", hour = "9-17", dayOfMonth = "*",
month = "*", dayOfWeek = "Mon-Fri", year = "*")" might be better then starting with blank line.
Comment 5 Martin Fousek 2011-03-23 14:54:53 UTC
Created attachment 107220 [details]
patch v1

David, I'm attaching something like pre-preview patch. :) It will still need many things like new text fields validation, whole annotation generation (just @Schedule is generated for now and just sometimes due to race condition which I wasn't solving for now), javadocs etc.

But I wanted to attach it because it will still need much of code and it would be better to catch all big changes of the behaviour at the beginning. ;)

So could I ask you to import patch and take a look into the new file wizard if it looks according to your ideas? The unfinished rest of code should properly create bean and it's interfaces and annotate the "myTimer" method with the proper @Schedule(...). Thanks.
Comment 6 Martin Fousek 2011-03-24 13:20:03 UTC
Created attachment 107242 [details]
patch v2

I fixed mentioned issues and attached newer patch. 

I didn't do any another validation in the form for now, because I think that syntax is too free for validation rules. Or would you prefer at least some regexp with warning messages there?

So could you take a look and let me know if it looks well to you? BTW, the patch has almost 70kB so I suppose that would be perhaps better postpone the integration to next release. :)
Comment 7 David Konecny 2011-03-25 21:53:54 UTC
Sorry for the delay. It is on my TODO list! :-)
Comment 8 Petr Jiricka 2011-03-26 16:04:20 UTC
This is definitely for NB 7.1, not NB 7.0.1.
Comment 9 Martin Fousek 2011-03-28 05:03:29 UTC
No problem, enough of time for that. 
Fully agree with NB 7.1.
Comment 10 David Konecny 2011-04-05 22:23:56 UTC
I had a look at patch v2 and here are some comments:

* I build it but could not run it really - some exception in IDE output and now "timer wizard shown". And current session ejb wizard has only first page. Might be issue on my side but I applied your patch and rebuild common and ejbcore modules.

* in MethodModel.java - what are the String in constructor "Map<String, String>" for? It is good to document what's key and value. Why getArguments() returns Map<String, Object>?

* I have not seen what UI looks like but I was thinking only about single editbox for all @Schedule attributes. So that user can type whatever they want. Individual fields might be too much work maybe. They each would require to have its own validation and I would not want to spent too much effort on it. Single editor box with default value of 'second="0", minute="*", hour="9-17", dayOfMonth="*",month="*",dayOfWeek="Mon-Fri", year="*"' is what I thought would do it. But I have not seen your UI...
Comment 11 Martin Fousek 2011-04-06 05:56:01 UTC
(In reply to comment #10)
> I had a look at patch v2 and here are some comments:
> 
> * I build it but could not run it really - some exception in IDE output and now
> "timer wizard shown". And current session ejb wizard has only first page. Might
> be issue on my side but I applied your patch and rebuild common and ejbcore
> modules.
> 

That's strange. It should work, I tried it now again and new File Type in Enterprise JavaBeans category was shown w/out any exception. BTW, did you used Clean&Build action? C&B will be needed since there are changes also in the layer file.

> * in MethodModel.java - what are the String in constructor "Map<String,
> String>" for? It is good to document what's key and value. Why getArguments()
> returns Map<String, Object>?

Truth, I updated the javadoc there.
Because GenerationUtils supports three combinations of parameters for construction of annotations: String|Object, String|List, String|String|String where the first string is always argument name and the another params are for arg. types or values. So holding them in Map<String, Object> should be extensible enough for all mentioned constructions. For now I added support for String|Object annotations but by requiring another one should be easy just add into MethodModel new constructor+getter and then a little bit update MethodModelSupport.createMethodTree method. In other words getArguments is assimilated to GenerationUtils.createAnnotationArgument methods. 

> 
> * I have not seen what UI looks like but I was thinking only about single
> editbox for all @Schedule attributes. So that user can type whatever they want.
> Individual fields might be too much work maybe. They each would require to have
> its own validation and I would not want to spent too much effort on it. Single
> editor box with default value of 'second="0", minute="*", hour="9-17",
> dayOfMonth="*",month="*",dayOfWeek="Mon-Fri", year="*"' is what I thought would
> do it. But I have not seen your UI...

Your idea seems to be really simplier. You will see if you like current UI. I would add there perhaps only validations that no textfield can be empty. Of course, I'm willing to redone it to one textField if you wouldn't like current solution. :)

Thanks for your comments, I will attach patch v3 with updated javadoc for MethodModel constructor.
Comment 12 Martin Fousek 2011-04-06 05:59:08 UTC
Created attachment 107535 [details]
patch v3
Comment 13 David Konecny 2011-04-06 22:50:41 UTC
(In reply to comment #11)
> BTW, did you used Clean&Build action? C&B will be needed since there are
> changes also in the layer file.

That must be it as I did not C&B it. I will try it again later this week. Thx.
Comment 14 David Konecny 2011-04-18 22:11:27 UTC
Let's use just single edit box please. I do not think this is such important or frequent feature to give it that much UI real estate. I liked the UI at first but on second thought there are a few issues which we would have to resolve. For example: the wizard panel is now quite big and likely we would have to move all schedule fields to a separate second wizard panel; people would start asking why fields have such strange alignment, why they are not localized, why there is no better validation; QA would ask for mnemonics, missing ":" characters, accessibility; etc. So considering all that a single edit box is still winning my heart. Just give it a default value like

 'minute = "*", second = "0", dayOfMonth = "*", month = "*", year = "*", hour = "9-17", dayOfWeek = "Mon-Fri"'

and it's done.
Comment 15 Martin Fousek 2011-04-20 06:03:31 UTC
Thanks David for looking on that. Actually, the UI was only prototype which I didn't want to complete till I will get response if it should look in that way or not. But I agree that the panel is quite big then and it would require moving all new fileds into next page of wizard and it would be additional overhead etc. so generally I welcome your idea of one text field for everything. :) Ok, so I will rewrite it and then wait for 7.1 trunk. Thanks again...
Comment 16 Martin Fousek 2011-04-22 19:04:34 UTC
Created attachment 107909 [details]
patch v4

Finally it required some validation because the string had to be divided into separated attributes but I think that it works quite well now, after some tuning. :) It's up to you if you would be interested into looking on that again - I can just say that I would be glad.
Comment 17 David Konecny 2011-05-10 01:16:10 UTC
I had a look at the diff and it all looks good. Just push it. Thanks!

Perhaps I would change template description little bit:

"Creates an example of Session Enterprise JavaBean using Timer Service. An empty session bean is created with a timer callback which is scheduled using a calendar-based syntax that is modeled after the UNIX cron facility."
Comment 18 Martin Fousek 2011-05-10 09:42:16 UTC
Thank you David for your review - I will change the description, sure.
In that case I'm just waiting till trunk will be opened for nb71 changesets and then I will push it there.
Comment 19 Martin Fousek 2011-05-30 09:23:18 UTC
Ok, I changed the description, fixed JUnit tests and pushed it in http://hg.netbeans.org/web-main/rev/4f511f07fb72.
Comment 20 Quality Engineering 2011-05-31 19:31:03 UTC
Integrated into 'main-golden', will be available in build *201105310954* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/4f511f07fb72
User: Martin Fousek <marfous@netbeans.org>
Log: #196526 - add a wizard for simplified EJB timer
Comment 21 Quality Engineering 2011-06-01 13:18:36 UTC
Integrated into 'main-golden', will be available in build *201106010401* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/4f511f07fb72
User: Martin Fousek <marfous@netbeans.org>
Log: #196526 - add a wizard for simplified EJB timer
Comment 22 Quality Engineering 2011-06-02 20:42:53 UTC
Integrated into 'main-golden', will be available in build *201106021001* on http://bits.netbeans.org/dev/nightly/ (upload may still be in progress)
Changeset: http://hg.netbeans.org/main/rev/4f511f07fb72
User: Martin Fousek <marfous@netbeans.org>
Log: #196526 - add a wizard for simplified EJB timer


By use of this website, you agree to the NetBeans Policies and Terms of Use. © 2012, Oracle Corporation and/or its affiliates. Sponsored by Oracle logo