This Bugzilla instance is a read-only archive of historic NetBeans bug reports. To report a bug in NetBeans please follow the project's instructions for reporting issues.

Bug 199405 - Create java.queries - abstraction for working with Java without JavaC
Summary: Create java.queries - abstraction for working with Java without JavaC
Status: RESOLVED FIXED
Alias: None
Product: java
Classification: Unclassified
Component: Source (show other bugs)
Version: 7.1
Hardware: All All
: P3 normal (vote)
Assignee: apireviews
URL:
Keywords: API, API_REVIEW, PLAN
: 199089 (view as bug list)
Depends on:
Blocks:
 
Reported: 2011-06-14 14:47 UTC by Tomas Zezula
Modified: 2011-06-30 20:00 UTC (History)
4 users (show)

See Also:
Issue Type: TASK
Exception Reporter:


Attachments
Diff file (50.61 KB, patch)
2011-06-14 14:49 UTC, Tomas Zezula
Details | Diff
Diff file with NB implementation and unit tests (134.58 KB, patch)
2011-06-21 16:38 UTC, Tomas Zezula
Details | Diff

Note You need to log in before you can comment on or make changes to this bug.
Description Tomas Zezula 2011-06-14 14:47:51 UTC
To allow the form to be independent on the NetBeans java cluster a friend api among the form and java is needed. The API should be string based (form has to be independent on javac types).

Use cases:
- find out if given file represents a JavaBean (either as .java file or .class
file) (BeanInstaller)
  - plus get the class' FQN if it does
- check if "java support" is available for given file
(JavaSource.forFileObject(fobj) != null) (InstallToPaletteAction)
- conditionally add "SuppressWarnings" annotation to initComponents method
(only if available - maybe could be just hardcoded, it's since 1.5)
(FormEditor)
- get all fields of the form's class (FormJavaSource)
- get all methods of the form's class returning certain type (Class) - for the
"method picker" property editor (FormJavaSource)
- obtain annotation of an event handler method (looks like only for undo
support, maybe could be avoided) (FormJavaSource)
- resolve FQN to simple names (adding impoerts as needed) in the generated code
(i.e. not in the whole class) (FQNImporter)
- determine declared superclass of the form's class (GandalfPersistenceManager)
- set the user specified superclass for a newly created form (one type of
template) (TemplateWizardIterator)
- change the form's class declaration to implement certain listeners
(JavaCodeGenerator)
- add a method to java class - needed by test utils (LayoutTestUtils), could
possibly be avoided
- determine the binary name of the form's class (not sure why it needs to be so
special) (MetaComponentCreator)
- rename a private field (all occurrences) in the form's file - silent, not
invoking refactoring (RADComponentRenameRefactoringSupport)
Comment 1 Tomas Zezula 2011-06-14 14:49:21 UTC
Created attachment 108893 [details]
Diff file
Comment 2 Jaroslav Tulach 2011-06-20 14:31:53 UTC
*** Bug 199089 has been marked as a duplicate of this bug. ***
Comment 3 Jaroslav Tulach 2011-06-20 14:32:15 UTC
I don't understand why new bug has been created instead of using bug 199089?
Comment 4 Tomas Zezula 2011-06-21 16:36:59 UTC
I suggest normal API_REVIEW not FAST_API_REVIEW as some use cases are unclear.
Comment 5 Tomas Zezula 2011-06-21 16:38:30 UTC
Created attachment 109027 [details]
Diff file with NB implementation and unit tests

Fixed some problems in the API regarding raw types.
Added an implementation of the queries based on the java.source
Added unit tests
Comment 6 Jesse Glick 2011-06-21 18:29:30 UTC
[JG01] The parameterTypes param to Queries.getMethodNames should be of type String[], not String..., if you expect null to be passed in. Otherwise the caller must write getMethodNames(..., ..., ..., (String[]) null) which is awkward.


[JG02] "name of the primitive type or fully qualified name of the declared type" does not specify how array types are named. Better I think to refer to a standard format, e.g. Class.getCanonicalName.


[JG03] modifyMethodAnnotations does not seem very useful, since you cannot specify annotation attributes. Would suggest a pair of methods

void addMethodAnnotation(..., String annotationFQN, Map<String,Object> annotationAttributes);
void removeMethodAnnotation(..., String annotationFQN);

where the Object could be a primitive wrapper, String, Class, impl of an annotation interface, or array of any of these.


[JG04] Updates.update takes a function which can return false to rollback. Why not just let the function throw QueryException?


[JG05] Typo - testIsAvialable


[JG06] ModelOperationsTest cannot pass if run directly; would just throw IllegalStateException. So it should rather be called QueryOperationsTestBase so it is not included in the test patternset for the module. It could even be written to have an abstract method to produce a QueryOperations impl, rather than finding this indirectly in lookup.
Comment 7 Tomas Zezula 2011-06-22 13:57:51 UTC
Thanks Jesse for review!

JG01 - fixed.

JG02 - fixed. Changed to "... specified by the class canonical name, @see Class#getCanonicalName"
I will need to somehow add a mention that it also supports parametrized types when useRawTypes is false, like java.util.List<ActionListener>

JG03 - fixed.

JG04 - Not fixed. I prefer the boolean. Exception represent an error (QueryException is thrown by queries) and the QueryException is propagated to user code calling Queries.query or Updates.update.

JG05 - fixed.

JG06 - fixed.
Comment 8 Jesse Glick 2011-06-22 14:01:21 UTC
(In reply to comment #7)
> JG04 - Not fixed. I prefer the boolean.

Then update(...) should return boolean as well. And I assume that the transaction is also rolled back automatically when an exception is thrown? This needs to be documented.
Comment 9 Tomas Zezula 2011-06-22 14:03:02 UTC
>Then update(...) should return boolean as well.
OK 
>And I assume that the transaction is also rolled back automatically when an exception is thrown? 
Right
>This needs to be documented.
OK
Comment 10 Jaroslav Tulach 2011-06-23 11:29:00 UTC
Javadoc available as
http://deadlock.netbeans.org/hudson/job/prototypes-matisse/javadoc/

Y01 The fact that Context has P parameter (e.g. <P extends Queries>) is an implementation detail and it should not be visible in the API. Type the Context only by return type R.

Y02 I know that this can wait, but anyway: missing apichanges with initial change, arch.xml with at least answer to arch-what and arch-usecases, two missing package.html. arch.xml should also export two APIs (API and SPI).

Y03 JavaOperationsImplTest.suite() should not enumerate individual tests, but rather add the whole class. That will help to ensure all newly created tests are also executed.
Comment 11 Tomas Zezula 2011-06-27 14:24:18 UTC
Tomas P. started to update the form to use the queries API and he found that some use cases are different.

So here is a list of changes:

1st) Adding or removing of annotations is not needed as the annotation is generated by template - the addMethodAnnotation, removeMethodAnnotation should be removed.

2nd) Getting a list of method annotations is not needed instead of this a method bounds are needed. So the getMethodAnnotations method will be replaced by getMethodBounds method

3rd) fixImports should behave in different way. It should change all fqn to simple names for some parts of the source files. The parts will be given as offset ranges.

4th) isAvailable, isJavaBean is not needed as palette is not a part of form and should be removed.

I will change (remove) methods as described above.
Do I forgot something?
Comment 12 Tomas Zezula 2011-06-28 14:41:28 UTC
I've resolved the changes in use cases.
Integrated into prototypes (matisse):
7c9c13a74a5d
9cd56220d0a9
23f46a0a90db
539de1306411
cb64ac129935
32428d7f48c1
3396b79240c9
360ebdf0b00c
Comment 13 Tomas Zezula 2011-06-28 18:54:48 UTC
Y01: I don't agree. It's because of type safety. If the Context does not have <P extends Queries> type parameter there will be a need for cast. The constructor of Context takes P param and Function<P,R>.

Y02: Fixed.

Y03: Adding the base class as a suite does not work because setUp is not called. I've looked into FSTest which do something like this using a map and static methods. I will not do it as it makes the test much less readable.
Comment 14 Tomas Pavek 2011-06-30 09:09:48 UTC
I've made two small changes (enabled changes in guarded blocks and added missing dependency on javac):
480c04a2e733
8c3015befe8c

Now it works well in all use cases for the form editor and from my point of view it's ready to be integrated to main.
Comment 15 Tomas Zezula 2011-06-30 19:50:27 UTC
Resolved YT01 and YT03:
prototypes b88521512bd0, 2c4b219a2bdf, ec725432a415
Comment 16 Tomas Zezula 2011-06-30 19:50:53 UTC
OK, I am integrating it into jet-main.
Comment 17 Tomas Zezula 2011-06-30 20:00:29 UTC
Integrated into jet-main d139701f453b