Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

Fix orchestra with updated project structure #460

Conversation

david-gibbs-ig
Copy link
Contributor

@david-gibbs-ig david-gibbs-ig commented Jan 13, 2022

Fixes #317

This branch has structural changes that are intended to make it quicker to work with core.

Core no longer has a mutual dependency with ALL the source code for fields, components and messages for all the FIX versions. Core depends on a new module "Base" as do the generated Message, Component and Field classes. Base does depend on a small number of generated classes that are used in Standard Header and Footer. These classes are generated with and packaged with Base, but only these classes. This is to help decouple the messages resources from the core distribution and facilitate the build and maintenance of custom distributions. Core now has only test dependencies on the message packages. This is described at length in project markdown files.

There is a benefit to this structure; "core" can be developed without rebuilding the message packages. Build times for the message packages are long. The FIX Orchestration now has nearly 6000 fields. One only needs to rebuild the message packages when code generation is altered, when a new FIX Standard Orchestration is incorporated or when Base is changed. As a further convenience there is a minimal profile to reduce the size -P"minimal-fix-latest". This This makes fix latest much smaller but includes resources that are needed to test core.

Use of the FTC Code Generator copied from the https://github.com/FIXTradingCommunity/fix-orchestra-quickfix is introduced via a dependency on https://github.com/quickfix-j/quickfixj-orchestra. The XSL based QFJ code generation takes too long for FIXLatest and this was causing C.I. builds to fail. This FTC Code Generator is used for fixt11 and fixlatest to help with future compatibility. Other protocol versions are built with the old code generator to all options to build custom packages with very high backward compatibility but benefitting from the new core (and base).

The convention used in Orchestra for codeset enumerations can lead to sub-optimal constant names. Work is in progress with the FTC to work around this. Where FIX latest is part of the build some constant names will be different from legacy builds. As mentioned above, there are work arounds for this.

You'll see that the new code generation for fixlatest does not de-normalize components. I had extended the code generation to do so and more closely mirror the QFJ code generator but this resulted in enormous classes with extreme build times for compilation and javadoc. I suggest that we adopt normalised components for fixlatest as the convention for this qfj distribution. If you build from orchestra you will get normalised components in the fixlatest package. This could be an optional setting but I think it's a bad idea. There are no actual "components" in fixt11, only groups.

This project now also publishes a QuickFIXJ orchestration, modified to help with a correct QFJ build. This can then be used to "easily" derive and customise QFJ builds, without needing to understand the work required to conform the standard orchestration to QFJ.
Customised builds should now be more maintainable, there is no need to manage forks of QFJ to cope with ROE differences and this will enable more succinct code, documents and test generation.

I've tested this approach in branch "use-orchestration-from-qfj-project" of ig-orchestrations.
see:
https://github.com/quickfix-j/quickfixj-orchestra
https://github.com/IG-Group/ig-orchestrations/tree/use-orchestration-from-qfj-project.
https://github.com/IG-Group/ig-orchestrations/tree/use-orchestration-from-qfj-project/ig-us-rfed/orchestration
https://github.com/IG-Group/ig-orchestrations/blob/use-orchestration-from-qfj-project/ig-us-rfed/quickfixj-messages-fixLatest/pom.xml
I have also tested the resulting customized message package end to end with private QFJ FIX applications.

…ild plans (does not rely on "mvn install", renamed module
…a copy of core but with fields from FIX Latest, I have built this to be like core but I think core is wrong and is not originally intended to include the generated fields in the pacakaging
…already provided in the message packages, fixLatest module now generates the fixLatest artefacts
…s to allow ready end to end testing. The code generator for fixLatest currently writes out the messages in this format. It is presumed that a fixLatest package will be added and the legacy 50SP2 resurrected.
chrjohn added a commit that referenced this pull request Aug 11, 2023
* update FileUtil to resolve new 403 error for requests with User-Agent header

* Updated User-Agent value for FileUtil

* Added check for HttpUrlConnection to changes done in #460

---------

Co-authored-by: david-gibbs-ig <david.gibbs@nadex.com>
chrjohn added a commit that referenced this pull request Aug 11, 2023
* update FileUtil to resolve new 403 error for requests with User-Agent header

* Updated User-Agent value for FileUtil

* Added check for HttpUrlConnection to changes done in #460

---------

Co-authored-by: david-gibbs-ig <david.gibbs@nadex.com>
chrjohn added a commit that referenced this pull request Aug 11, 2023
* update FileUtil to resolve new 403 error for requests with User-Agent header

* Updated User-Agent value for FileUtil

* Added check for HttpUrlConnection to changes done in #460

---------

Co-authored-by: david-gibbs-ig <david.gibbs@nadex.com>
chrjohn added a commit that referenced this pull request Aug 12, 2023
* update FileUtil to resolve new 403 error for requests with User-Agent header

* Updated User-Agent value for FileUtil

* Added check for HttpUrlConnection to changes done in #460

---------

Co-authored-by: david-gibbs-ig <david.gibbs@nadex.com>
@chrjohn
Copy link
Member

chrjohn commented Aug 14, 2023

I have errors like this when building with full-fix-latest.

ComplexEventStartDate.java:[22,16] error: incompatible types: String cannot be converted to LocalDate

Any idea what might be wrong? Was this related to the changes you did here: quickfix-j/quickfixj-orchestra#4 ?

@david-gibbs-ig , I think I know where this error comes from. All fields that have a type of UTCDateOnly are affected by this. The reason seems to be these lines of code that I think were added by you. Do you still recall why we need an extra constructor with String?

https://github.com/quickfix-j/quickfixj-orchestra/blob/1845c4306cb78c786ea3a1c38408eeae76764d6e/quickfixj-from-fix-orchestra-repository/quickfixj-from-fix-orchestra-generator/src/main/java/org/quickfixj/orchestra/CodeGeneratorJ.java#L858-L860

You did some (related ?) changes with this PR: quickfix-j/quickfixj-orchestra#4
I agree that LocalMktTime and LocalMktDate shouldn't be handled by LocalDate/Time since the time zone might not be known. But IMHO UTCDateOnly could be handled by a LocalDate with time zone UTC.

What do you think?

@david-gibbs-ig
Copy link
Contributor Author

I have errors like this when building with full-fix-latest.
ComplexEventStartDate.java:[22,16] error: incompatible types: String cannot be converted to LocalDate
Any idea what might be wrong? Was this related to the changes you did here: quickfix-j/quickfixj-orchestra#4 ?

@david-gibbs-ig , I think I know where this error comes from. All fields that have a type of UTCDateOnly are affected by this. The reason seems to be these lines of code that I think were added by you. Do you still recall why we need an extra constructor with String?

https://github.com/quickfix-j/quickfixj-orchestra/blob/1845c4306cb78c786ea3a1c38408eeae76764d6e/quickfixj-from-fix-orchestra-repository/quickfixj-from-fix-orchestra-generator/src/main/java/org/quickfixj/orchestra/CodeGeneratorJ.java#L858-L860

You did some (related ?) changes with this PR: quickfix-j/quickfixj-orchestra#4 I agree that LocalMktTime and LocalMktDate shouldn't be handled by LocalDate/Time since the time zone might not be known. But IMHO UTCDateOnly could be handled by a LocalDate with time zone UTC.

What do you think?

I did not remove the extra ctor for UTCDateOnly in quickfix-j/quickfixj-orchestra#4. I'm testing a new version locally.

@david-gibbs-ig
Copy link
Contributor Author

david-gibbs-ig commented Nov 12, 2023

@chrjohn I have made the necessary change to the code generation in PR quickfix-j/quickfixj-orchestra#6 .
I have built and tested this change locally with ComplexEventStartDate and can commit the changes when 1.0.2 of quickfixj-orchestra is published. - update - I did push the changes, so you can see, but of course it won't build without 1.0.2.

@david-gibbs-ig
Copy link
Contributor Author

@chrjohn I'm very pleased to see the build checks green https://github.com/quickfix-j/quickfixj/pull/460/checks

@chrjohn chrjohn added this to the QFJ 3.0.0 milestone Nov 26, 2023
check if still `install` is needed instead of `test`
This reverts commit ca7c47e.
@david-gibbs-ig
Copy link
Contributor Author

david-gibbs-ig commented Nov 30, 2023 via email

@chrjohn
Copy link
Member

chrjohn commented Nov 30, 2023

@david-gibbs-ig merging in 3, 2, 1... 🥳

@chrjohn chrjohn merged commit 8d23147 into quickfix-j:master Nov 30, 2023
12 checks passed
@david-gibbs-ig
Copy link
Contributor Author

@david-gibbs-ig merging in 3, 2, 1... 🥳

Fantastic ! I had better make some examples now !

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Support for FIX Latest
2 participants