Hibernate 5.4 extension updates #36

michaelborn · 2023-02-13T18:20:56Z

Hibernate 5.4 Extension Updates

Thanks to a client project, we (Ortus Solutions) have had the opportunity to assist with closing some of the open ORM / Hibernate extension issues, as well as bring the extension up to modern standards in a few areas. Notably, this PR:

resolves a number of bugs in the current Hibernate extension
adds new features,
cuts the ORM reload time in half via refactoring the initialization process,
adds documentation via the readme and javadocs,
adds or improves the existing test suite,
and does some general code cleanup and housekeeping

Explanation of changes:

Bug Fixes

New Features

Implemented LDEV-3525 (2fc53ef, 5fc76c0, 681b068, 60a4f4c, 7c61672, 57a4309, a075934)

Initialization Rework

See commits 41f0d86, e8d0dd0, and e31782f

Don't reinit ORM except 1. on first run or 2. if ORMReload() is fired
- This fixes a bug where INIT_CFCS and INIT_ALL were mapped to the same int, so each page load would re-inspect all ORM entities.
- Avoids re-hashing the ORM config on every request
- Reduces unnecessary config hashing and component scans on every request
Use hasTempCFCs() boolean method for clarity when looking for ORM entities in the list of pre-inspected entities
Don't attempt to reinit when instantiating an entity

This refactor alone reduces the time to reinit 1,000 entities from ~14s to ~4s - a ~70% improvement!

See performance data collected via Hibernate Playground against the 3.5 extension, 5.4 extension, and 5.4 with modifications:

Documentation

New project README for (cd7b05d)
Added automatic javadocs generation via Maven and GitHub Actions (2235075)
Added javadocs / documentation blocks ( 888a2cb, 8174c01, a5ad71b, d553f6e, 032e07d, 2198aa1, 69854bb, d988746, 946b1bf)

Tests

Imported tests for LDEV-4150 (20a0005)
Imported tests for LDEV-3907 (4271c5f)
Added test for this.ormSettings.ormConfig XML file (c0fdc92)
Added test for this.ormSettings.cacheconfig XML file (2e1455e)
Added test for datasource annotation on individual entity components (6cac457)
Setup Junit tests (d3266be, 73a9939, 70482ce, a5f4b89, b525356)
Added test.sh for easily running Lucee test cases (c997777)

Housekeeping

Moved all ORM*(), entity*() methods such as ORMReload(), entitySave() to extension (e116049, c0879cd, 00681de)
Drop support for Lucee dialect / .lucee components (aa4aa49), (ac5abbf)
Drop unused methods and classes cluttering the source code (c00e275, 96eb530, 461e423, )
Moved all *Util.java classes to .util package (24c4115)
Use Hibernate setting key names if they exist (4c51369, d342d21, 0edede7)
Refactored configuration to use a Builder pattern for clarity (c7d74a4, 929fff4)
Added automatic java source formatting via formatter-maven-plugin (c43c60e, c4c3ad9, 65f44cf)
Updated copyright notice to use "Lucee Association Switzerland" instead of Railo. (882ea9a)

GitHub Actions Secrets

The new javadocs CI build auto-uploads generated java documentation to S3 using the configured secrets.

You'll need to add these secrets to the Github Secrets admin:

AWS_APIDOCS_BUCKET Bucket name to upload to - for example, apidocs.lucee.org
AWS_ACCESS_KEY S3 access key ID
AWS_ACCESS_SECRET S3 access key secret

In addition, the maven java formatter requires read/write permissions in order to commit file changes to the repository. This can be accomplished using a GH_TOKEN secret containing a personal access token with read and write permissions to the extension-hibernate repository.

…ions

Useful for tests or simply acquiring the internal Hibernate transaction

1. Drop the old EVComponent and InterceptorImpl. We only need event handling in a single place (for now). 2. Rename variables, method names, and class properties to make it obvious when we're invoking an EventHandler method, and when we're invoking a method on a standard entity CFC. 3. More java docs 4. A basic unit test - ya know, for science.

Using `ant dist && ./test.sh`, we can easily build the extension to a .lex file and immediately run CFML tests against a specific version of Lucee.

These function definitions belong in the ORM extension. Copied verbatim from Lucee core.

…core

michaeloffner · 2023-04-24T13:51:17Z

source/java/src/org/lucee/extension/orm/functions/EntityNameList.java

@@ -0,0 +1,37 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:51:27Z

source/java/src/org/lucee/extension/orm/functions/EntityNew.java

@@ -0,0 +1,76 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:51:37Z

source/java/src/org/lucee/extension/orm/functions/EntityReload.java

@@ -0,0 +1,34 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:51:45Z

source/java/src/org/lucee/extension/orm/functions/EntitySave.java

@@ -0,0 +1,38 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:51:53Z

source/java/src/org/lucee/extension/orm/functions/EntityToQuery.java

@@ -0,0 +1,39 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:52:02Z

source/java/src/org/lucee/extension/orm/functions/ORMClearSession.java

@@ -0,0 +1,40 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:57:00Z

source/java/src/org/lucee/extension/orm/functions/ORMCloseAllSessions.java

- * 
- * You should have received a copy of the GNU Lesser General Public 
+ *
+ * You should have received a copy of the GNU Lesser General Public
 * License along with this library.  If not, see <http://www.gnu.org/licenses/>.


not sure what is going on here, but print and aprint are classes present in the lucee core and every extension, they are used to make printout while developing and should NOT be removed.

print can be used like the dump function and can better show complex structures than System.out

you can simply remove the "extends aprint" from the print class to see where the class is used, helpful to clean up

please do not simply remove stuff you don't completely understand the need for

michaeloffner · 2023-04-24T13:57:31Z

source/java/src/org/lucee/extension/orm/functions/ORMCloseSession.java

@@ -0,0 +1,40 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:57:40Z

source/java/src/org/lucee/extension/orm/functions/ORMEvictCollection.java

@@ -0,0 +1,42 @@
+/**
+ *


see comment for EntityDelete

michaeloffner · 2023-04-24T13:57:49Z

source/java/src/org/lucee/extension/orm/functions/ORMEvictEntity.java

@@ -0,0 +1,41 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:58:30Z

source/java/src/org/lucee/extension/orm/functions/ORMEvictQueries.java

@@ -0,0 +1,39 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:58:43Z

source/java/src/org/lucee/extension/orm/functions/ORMExecuteQuery.java

@@ -0,0 +1,108 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:58:57Z

source/java/src/org/lucee/extension/orm/functions/ORMFlush.java

@@ -0,0 +1,39 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:59:06Z

source/java/src/org/lucee/extension/orm/functions/ORMGetSession.java

@@ -0,0 +1,47 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:59:15Z

source/java/src/org/lucee/extension/orm/functions/ORMGetSessionFactory.java

@@ -0,0 +1,47 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T13:59:27Z

source/java/src/org/lucee/extension/orm/functions/ORMReload.java

@@ -0,0 +1,44 @@
+/**


see comment for EntityDelete

michaeloffner · 2023-04-24T14:04:05Z

source/java/src/org/lucee/extension/orm/hibernate/util/CommonUtil.java

@@ -0,0 +1,1068 @@
+package org.lucee.extension.orm.hibernate.util;


hard to see the change, when the function not got moved but deleted and created. did you just chnage the package name or also code inside the class. best do not move stuff around at all,this can be done is a separate "clean up PR".
In short move the class back to it's old package

michaeloffner · 2023-04-24T14:04:53Z

source/java/src/org/lucee/extension/orm/hibernate/util/ConfigurationBuilder.java

@@ -0,0 +1,299 @@
+package org.lucee.extension.orm.hibernate.util;


same as CommonUtil i guess

michaeloffner · 2023-04-24T14:06:15Z

source/java/src/org/lucee/extension/orm/hibernate/util/aprint.java

@@ -1,432 +0,0 @@
-/**


revert that change, add it again

michaeloffner · 2023-04-24T14:07:25Z

source/tld/ormTags.tldx

@@ -0,0 +1,21 @@
+


this tld has no use, why did you add it?

michaeloffner · 2023-04-24T14:09:01Z

src/test/java/HibernateORMEngineTest.java

@@ -0,0 +1,28 @@
+package org.lucee.extension.orm.hibernate;


we make all our test cases on CFML level, for one simple reason. we wanna have the test as close to real world as possible, so no java based test cases, please remove

michaeloffner · 2023-04-24T14:23:10Z

I have reviewed part of the PR, not everything because we have some issue repeating itself over and over again.
We have 125 files changed but in my opinion the changes that matter are only in 2 or 3 files. Other changes are just noise hard to filter out.

"Noise problems"
you have moved all the orm functions from the Lucee core to the extension, this adds a lot of functions , but it is not clear if you have simply copy/paste them or also made some changes, we need that info. In addition all this functions are useless because they are not registered. In addition they do not implement the BIF class and because of that create a lot of overhead. Also i'm not sure if they load the session in the correct way. But biggest question is why did you do it? I think i know why, we did similar with the S3 extension.

You have changed couple of package names of classes and not moved them. so they appear as new classes and the old one got deleted, so like with the functions it is unclear if you simply changed the package name or also modified the classes, best revert that change and do it in a separate clean up PR.

Please do not remove code like print/aprint, that is used in every Lucee project and do not indroduce new stuff like junit that is not used in any Lucee project, let's just stick to the necessary changes with that PR.

michaelborn · 2023-04-25T14:08:49Z

The ORM*() and entity*() methods were moved into the extension to resolve several main issues:

The ORM methods can be updated and improved without requiring a Lucee core update.
The ORM functionality is only utilized within the Hibernate extension, so it makes sense to encapsulate those methods therein.
Experimental ORM methods can be added to (and removed from!) the Hibernate extension without requiring "buy-in" from the Lucee core.

Besides these concerns, having ORM functionality encapsulated within the Lucee ORM extension makes practical and organizational sense.

With that said, the ormFunctions.fld file was supposed to point to the ported ORM functions - that is my fault! I will update this.

Other feedback items:

The source/java/.project changes were made automatically by VS Code. That file only pertains to the Hibernate extension project - I am not sure why we should care if it is incompatible with another Lucee project. The IDE shouldn't care, as it should simply pick up the file and move forward with the project configuration. That is the entire reason the .project file exists.
Having all ORM and Entity functions extend BIF will require us to implement the invoke() method for each class for switching between call() methods based on the number and type of arguments. I am not opposed to this, but it will take some time to update the function definitions.
Since the print.java and aprint.java classes are unused in production and provide no production value, I would prefer we not ship them with the Hibernate extension. If dump debugging is to be used in 2023, I would recommend using the aprint.java class in the Lucee core.
The EXTENSION.MF is used in the Maven build, which uses the echo-maven-plugin to append manifest entries into a Lucee-compatible extension manifest. Since Lucee's extension manifest does not adhere to the Jar File Specification, this is the only way to build a manifest in Maven that will parse successfully in Lucee.
I strongly disagree with removing junit tests. YES, we need CFML functionality and syntax to be tested in CFML, but it is a huge benefit to have java-based unit tests which are going to run much faster and have much better integration with the IDE. In addition, it allows us to catch bugs in situations which are very hard or verbose to replicate from CFML.
Finally, moving the Util classes to a .util package makes structural sense. There were several changes to those util classes apart from the package move, though, mostly related to removing reflection when possible and/or reworking the configuration process as part of the ORM reload performance improvements.

Our only aim here is to improve the quality and maintenance overhead of the Hibernate extension. As long as the changes improve maintainability and do not adversely affect the CFML developer, I don't see why these changes should be backed out of the PR.

michaelborn · 2023-04-25T14:16:49Z

With that said, the ormFunctions.fld file was supposed to point to the ported ORM functions - that is my fault! I will update this.

Actually, this file is already committed and does point to the moved functions:

extension-hibernate/source/fld/ormFunctions.fld

Lines 15 to 29 in 934dddc

    
           	<function> 
        
           		<name>EntityDelete</name> 
        
           		<class bundle-name="{bundle-name}" bundle-version="{bundle-version}">org.lucee.extension.orm.functions.EntityDelete</class> 
        
           		<description>Deletes the record from the database for the specified entity. 
        
           EntityDelete(entity)</description> 
        
           		<argument> 
        
           			<name>name</name> 
        
           			<type>object</type> 
        
           			<required>Yes</required> 
        
           		<description>Name of the entity being deleted.</description> 
        
               </argument> 
        
           		<return> 
        
           			<type>void</type> 
        
           		</return> 
        
           	</function>

michaelborn · 2023-05-01T15:41:59Z

@michaeloffner I updated the built-ins (ORM*() and entity*()) to extends BIF as you suggested.
I also dropped the .tld XML config file for configuring built-in tags, since, as you mentioned, it is unused.

Can we get another review on this?

https://luceeserver.atlassian.net/browse/LDEV-3525 https://luceeserver.atlassian.net/browse/LDEV-4017 https://luceeserver.atlassian.net/browse/LDEV-4150 https://luceeserver.atlassian.net/browse/LDEV-4308 from #36

michaelborn added 30 commits January 19, 2023 17:02

📖 DOC: Add project README

cd7b05d

📖 DOC: Add javadocs generation to ant build

a0ff160

🐛 FIX: Update Maven pom.xml to get Maven builds working

63aff0e

🤖 TEST: Set up Junit tests via Maven

d3266be

🤖 CI: Add Maven build for junit tests

73a9939

👌IMPROVE: Refactor deprecated setEl() usage for static Dialects struct

c32d642

👌IMPROVE: Drop commented code, add javadocs to Dialect class

946b1bf

👌 IMPROVE: Ignore all temporay build directories

33fb012

📦 NEW: Add Dialect.getDialects() for seeing all available dialect opt…

b3b1c2e

…ions

📖 DOC: Add Javadocs to Transaction object

a5ad71b

👌 IMPROVE: Drop unused CFCEntityResolver class

8a6ce8d

📖 DOC: More javadocs work, fix all javadoc warnings

d553f6e

📦 NEW: Add auto-generation of Javadocs via Maven

2235075

🐛 FIX: Switch to Java 8 for Maven builds

b3a066b

🐛 FIX: Don't use update-snapshot

0fe2d15

📖 DOC: Fix invalid javadocs

032e07d

📖 DOC: Additional javadocs for main class

2198aa1

👌 IMPROVE: Update gitignore for ROOT build artifacts

addb57a

📖 DOC: Update javadocs

948d3cf

🤖 TEST: Add basic junit tests for future expansion

6bbb031

📦 NEW: Add getTransaction() to Hibernate transaction wrapper

2980665

Useful for tests or simply acquiring the internal Hibernate transaction

👌 IMPROVE: Update Lucee build version

b21bd61

🤖 TEST: Update and streamline event handler tests

ca0aad2

👌 IMPROVE: Drop dead datasourceManager code from CommonUtil

09a8246

📦 NEW: Add .fld, .tld XML config for custom Lucee tags and functions

fcbd2ab

🤖 TEST: Add test.bash for easier test runs

c997777

Using `ant dist && ./test.sh`, we can easily build the extension to a .lex file and immediately run CFML tests against a specific version of Lucee.

👌 IMPROVE: Filter node_modules and .git from IDE resources

8031e96

📦 NEW: Import ORM* and Entity* functions from Lucee core

e116049

These function definitions belong in the ORM extension. Copied verbatim from Lucee core.

🐛 FIX: Updates to ORM functions so they can compile outside of Lucee …

c0879cd

…core