Hi,
I pushed some changes that significantly reduce the number of lines
changed and also the possibility of breakage:
- merged back UtilValidateDateTime and UtilValidateEmpty into UtilValidate
- moved some methods from *Runtime -> former class.
If you test the changes and find some issues please let me know.
I am willing to work to fix those and make the PR mergeable.
I also did an exploratory work to see what would it take to have entity
engine as a library see https://github.com/ieugen/ofbiz-framework/pull/3
On top of this PR, it adds only 670 lines and remove 370.
Very small price to pay IMO.
I have partial integration tests running.
I have added some comments related to the blockers (EntityEca, Tennant)
They should be doable, but I need more info / help.
To reiterate, please let me know of any issue you find and I will work
to fix it.
Regards,
Eugen
La 07.12.2023 20:55, Eugen Stan a scris:
Hi Michael,
Thank you for taking a look.
I will reply inline.
I did a demo project to support my case
https://github.com/ieugen/ofbiz-tooling-demo .
La 07.12.2023 12:53, Michael Brohl a scris:
Hi Eugen,
can you give us some explanations/examples why this is a useful change?
What are the downsides?
I don't see any downsides.
There might be some breaking changes on upgrades.
We could work to mitigate them - will explain bellow.
I see a lot of changes in the base API like UtilValidate,
UtilProperties etc. which will brake almost every custom project out
there.
Have you tried it in your projects?
Do you know how many fail?
All the integrations tests pass.
Also we could do some work to mitigate those braking changes:
- copy those methods back to the classes they came from
- implement delegation: add a method with same signature that calls on
the class from base/util (UtilValidate calls UtilValidateEmpty).
IMO this should be a transition period until the code is cleaned up.
I did not do the mitigations because this is intendend as a POC to be
prepared once we decide it's a good direction.
Also - I don't have access to 'all the projects out there' to test.
The changes also need explanation. For example: why the split of
UtilValidate to UtilValidateEmpty etc.
Because of the cyclic dependency between classes.
There is a LOT of cyclic dependencies which IMO is a code smell.
UtilValidate depens on UtilProperties which depens on UtilValidate.
There are numerous cases for this.
If you run Intellij IDEA Analyze -> Analyze Cyclic dependencies you will
see them.
( I posted an image on gh issue)
https://github.com/apache/ofbiz-framework/pull/678#issuecomment-1845903198
I think we need to have very strong advantages to make those changes
and currently I do not see them.
We open OFBiz to be used as libraries.
Then developers can use things like entity-engine, datafile, bits to
build tooling as CLI or services that integrate with OFBiz.
- It opens a path forward to inovation
- It will make life of integrators MUCH more easy as it will provide
java API's for them to do integrations
- It exposes the dependencies by making them explicit (as gradle deps)
- Developers can focus on a smaller part of the code base: component
loading, XML parsing, entity engine without worrying about the rest of
the code.
- It will allow users to build tools and ofbiz components and package
them as jar files (component/datafile is a component as a jar file - it
depends on base/util and base/crypto - but nothing else from OFBiz).
- It will facilitate the developement of proper java API's - now OFBiz
is an implementation only - the API is the implementation.
I would like to build some import tooling that uses the OFBiz code (so I
don't reinvent the weel).
Currently I don't see a way on how to do that.
See the https://github.com/ieugen/ofbiz-tooling-demo I just did for
Crypto. It can help people migrate data from/to OFBiz by using the same
crypto code and the same xml data processing code.
I do not need to bring whole OFBiz if all I want is to push / pull data
from DB or to some files.
To my knowledge that is not possible with OFBiz right now (unlesss you
reimplement the code). It will be possible once we publish libraries.
There is much more that this can help with but we will see if OFBiz to
accepts some changes.
And again, my changes don't break ANY integration test from the 780?!
tests.
I am very excited of finding this way of breaking OFBiz into smaller
parts without breaking OFBiz runtime.
There will be work to be done to streamline the code as libs but I am
optimistic.
Regards,
--
Eugen Stan
+40770 941 271 / https://www.netdava.com
