Hi Sahoo,
I am working away at learning how this code works. Here are a few
initial comments. I will send more when I have learned it all.
- our coding convention requires braces around all blocks even if they
are a single line. Where are some examples where there are issues:
oracle\toplink\essentials\ejb\cmp3\persistence\DirectoryArchive.java - 92
oracle\toplink\essentials\ejb\cmp3\persistence\JarInputStreamURLArchive.java
- 93
- There are a number of exceptions we do not expect to get to that are
not internationalized. Can we either internationalize them or use the
strategy used in DirectoryInsideJarArchive an use assertions instead of
exceptions.
-
oracle\toplink\essentials\ejb\cmp3\persistence\DirectoryInsideJarURLArchive.java
- The comment indicates this file is covered by a different license. If
this is true, it could be an issue. Can this file be covered by CDDL?
- Several classes have import statements reordered and several classes
have explicit import statements (x.y.z) changed to import x.y.*. Is
this the result of some kind of tool use? If we are going to choose to
do something like this, we should do it for the whole of essentials
rather than file by file.
-
oracle\toplink\essentials\internal\ejb\cmp3\metadata\MetadataProcessor.java
- 80 - // TODO: Sahoo: fix this code path - what is this?
I'll send more comments soon,
-Tom
Sanjeeb Kumar Sahoo wrote:
>(resending with jar file attachment instead of zip file)
>
>Hi Tom, Guy,
>
>Attached here with are the fixes for following issues:
>https://glassfish.dev.java.net/issues/show_bug.cgi?id=1051
>https://glassfish.dev.java.net/issues/show_bug.cgi?id=1074
>https://glassfish.dev.java.net/issues/show_bug.cgi?id=1115
>https://glassfish.dev.java.net/issues/show_bug.cgi?id=1131
>https://glassfish.dev.java.net/issues/show_bug.cgi?id=1206
>
>They are all related issues, so I am fixing them together. In the
>process I am also refactoring the PU processing code a bit. List of
>files affected are: (A stands for new files and M stands for modified files)
>A src/java/oracle/toplink/essentials/ejb/cmp3/persistence/Archive.java
>A
>src/java/oracle/toplink/essentials/ejb/cmp3/persistence/ArchiveFactoryImpl.java
>A
>src/java/oracle/toplink/essentials/ejb/cmp3/persistence/DirectoryArchive.java
>A
>src/java/oracle/toplink/essentials/ejb/cmp3/persistence/DirectoryInsideJarURLArchive.java
>A
>src/java/oracle/toplink/essentials/ejb/cmp3/persistence/JarFileArchive.java
>A
>src/java/oracle/toplink/essentials/ejb/cmp3/persistence/JarInputStreamURLArchive.java
>M src/java/oracle/toplink/essentials/ejb/cmp3/StaticWeaver.java
>M
>src/java/oracle/toplink/essentials/ejb/cmp3/persistence/PersistenceUnitProcessor.java
>M src/java/oracle/toplink/essentials/exceptions/ValidationException.java
>M
>src/java/oracle/toplink/essentials/exceptions/i18n/ValidationExceptionResource.java
>M
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java
>M
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/JavaSECMPInitializer.java
>
>
>M
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/metadata/MetadataProcessor.java
>M
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/metadata/MetadataProject.java
>M
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/metadata/MetadataValidator.java
>M src/java/oracle/toplink/essentials/internal/ejb/cmp3/xml/XMLHelper.java
>
>What are the new files:
>The new files (Archive, ArchiveFactoryImpl and various ArchiveImpls) are
>to do with URL handling. They offer functionality like listing entries
>in a URL, getting a resource from a URL etc. This abstraction helps us
>in centralizing URL handling code to a few known classes. They have been
>extensively tested in various environment. Archive and
>ArchiveFactoryImpl are the starting point classes. Please refer to the
>javadocs in all those classes for more information. They can be tested
>in standalone Java application.
>
>How are they used?
>Most of the code in PersistenceUnitProcessor and MetadataProcessor now
>uses Archive interface as opposed to URL interface.
>
>Changes to existing files:
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/JavaSECMPInitializer.java:
>
>Uses Archive interface as opposed to URL.
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/EntityManagerSetupImpl.java:
>
>The metadata processing code from this class have been moved to
>MetadataProcessor. It plays the role of a director in builder pattern.
>It uses MetadataProcessor to build MetadataProject object.
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/metadata/MetadataProcessor.java:
>
>A lot of code have been moved from EntityManagerSetupImpl to this class.
>Also, it now follows builder pattern. It acts as the Builder while
>EntityManagerSetupImpl is the director and MetadataProject is the product.
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/metadata/MetadataProject.java:
>
>To avoid repeated reading of mapping files, annotations etc, we now
>store Document object for each mapping file in MetadataProject. We also
>store entity class names here. That allows us to use the same set of
>class names in predeploy and deploy step.
>src/java/oracle/toplink/essentials/ejb/cmp3/persistence/PersistenceUnitProcessor.java:
>
>URL processing code have been moved to various Archive implementation
>classes.
>src/java/oracle/toplink/essentials/ejb/cmp3/StaticWeaver.java: Trivial
>changed required to accommodate API change in PersistenceUnitProcessor.
>
>Changes to Validator and Exception classes have to do with decision to
>throw exception when a mapping file is not found or multiple mapping
>files are found(issue 1051 & 1074).
>
>The following files are changed to address
>https://glassfish.dev.java.net/issues/show_bug.cgi?id=1206:
>src/java/oracle/toplink/essentials/internal/ejb/cmp3/xml/parser/PersistenceContentHandler.java:
>
>When jar-file element value is read from xml stream, it is stored in an
>instance variable in SEPersistenceUnitInfo.
>src/java/oracle/toplink/essentials/ejb/cmp3/persistence/SEPersistenceUnitInfo.java:
>
>URL corresponding to each jar-file is created and returned in
>getJarFileUrls() method.
>
>
>Testing done so far:
>--------------------------
>1) entity-persistence-tests pass after setting
>toplink.orm.throw.exceptions=false in config/META-INF/persistence.xml.
>2) Unit tests for all the bugs fixed here.
>3) successful run of TCK
>4) GF QuickLook tests
>5) JPA packaging tests in verifier that extensively test PU processing
>code in TopLink.
>6) Testing where file name contains space.
>7) Testing JPA application in Tomcat
>8) Running JPA apps in GlassFish
>
>In one of the bug notes you mentioned about testing inside Oracle AS. I
>don't have access to any such set up. If someone at your end is ready to
>run some tests in Oracle AS, then I am happy to provide a binary patch
>to her or him.
>
>Thanks,
>Sahoo
>
>
>
--
Tom Ware
Principal Software Engineer
Oracle Canada Inc.
Direct: (613) 783-4598
Email: tom.ware_at_oracle.com