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

Not suited for Java's modular world #1

Closed
TomerFi opened this issue Oct 28, 2020 · 7 comments
Closed

Not suited for Java's modular world #1

TomerFi opened this issue Oct 28, 2020 · 7 comments

Comments

@TomerFi
Copy link

TomerFi commented Oct 28, 2020

Hello

I wouldn't necessarily call this an issue, or even a question.
And it's not directly related to this repository,
It's more of a jtapi codebase issue,
but maybe it's worth mentioning in this repo's README.

The jtapi jar cannot be used in a JPMS project.
You can easily get away with the lack of module-info using the moditect plugin.

But the two package-less classes residing in the jar's root:

  • CiscoJtapiVersion.class
  • DefaultJtapiPeer.class

Afaik, are a no-go in Java's modular world.

I'm going to have to rebuild my current project in a non JPMS manner.

Maybe it's worth mentioning in the README that these samples,
or even the jtapi jar itself, is not suitable for a modular project.

Thank you.

@dstaudt
Copy link
Collaborator

dstaudt commented Oct 28, 2020

Good catch, and thanks for the feedback!
We've opened a defect against Cisco JTAPI for tracking: CSCvw28384
I don't have a JPMS compliant project handy, but I am able to just delete CiscoJtapiVersion.class and DefaultJtapiPeer.class from the .jar and the sample seems to still run - could be a possible workaround (along with creating an ad hoc module-info.java as you suggest.)

@dstaudt
Copy link
Collaborator

dstaudt commented Oct 30, 2020

Added a note to README.md re JPMS support and jtapi.jar

@dstaudt dstaudt closed this as completed Oct 30, 2020
@TomerFi
Copy link
Author

TomerFi commented Nov 1, 2020

Thank you for your response and the offered workaround.
I'll test it out as soon as I can and report back.

@TomerFi
Copy link
Author

TomerFi commented Nov 12, 2020

Deleting DefaultJtapiPeer.class might be troublesome.

It is used by JtapiPeerFactory as the default class for instantiating when invoking getJtapiPeer as it's done in the examples found in this repository:

CiscoJtapiPeer peer = (CiscoJtapiPeer) JtapiPeerFactory.getJtapiPeer(null);

If I read the code correctly, will return a new instance of DefaultJtapiPeer which does nothing but extend CiscoJtapiPeerImpl which is an implementation of CiscoJtapiPeer.

So I believe that deleting DefaultJtapiPeer.class might be troublesome.

The following line works exactly the same,
and should allow the safe deletion of DefaultJtapiPeer.class:

CiscoJtapiPeer peer = (CiscoJtapiPeer) JtapiPeerFactory.getJtapiPeer("com.cisco.jtapi.CiscoJtapiPeerImpl");

@dstaudt
Copy link
Collaborator

dstaudt commented Jan 4, 2021

FYI JPMS support is known to be part of the scoping for enhancing JTAPI in support of Java 1.8+ generally (in a yet-unidentified CUCM release.)

@TomerFi
Copy link
Author

TomerFi commented Jan 5, 2021

That's awesome!
Thank you for sharing this.
:-)

BTW
I was able to accomplish what I needed.
My initial goal was to distribute my app as a JPMS image using jlink.
But jlink doesn't work with automatic modules,
So I had to figure out how to convert the jtapi jar to a modular one.

I was able to get this working, and it's sweeeetttt! 🔥

If anyone bumps into this, these were the pitfalls, from the jar point of view:

  • The instantiation of the peer using the peer factory needed to be more explicit, using the fqdn com.cisco.jtapi.CiscoJtapiPeerImpl as mention in this comment.
  • The two package-less classes, CiscoJtapiVersion and DefaultJtapiPeer needed to be removed, as mentioned in this comment.

These ☝️, were handled manually, and need to be repeated if the jar will ever be replaced in my project, e.g. when bumping versions.
This of course needs to be done prior to deploying the jar to the relative lib folder.

The following 👇, however, can be handled manually, or in build time.

  • There are two extra dependencies, log4j and jsafe, afaik these, are old, unmaintained, and unused dependencies.
    This was in fact, only discovered in runtime (testing) as there are only a couple of sub-packages inside the jtapi jar using these dependencies, and I'm not using any of them, so it was caught when my testing plugin was unable to construct the modulepath.
    Getting around this was easy (eventually), as the new version of the jtapi jar was now modular and has a module-info descriptor, so I just stopped exposing these sub-packages, and all was right in the world again. 😄

From a build point of view, it was a bit tricky.
The easiest way was of course to just replace the actual jar in the lib with the new one, and be done.
But that would mean that my project will be tightly coupled with the specific version I'm currently using (11.5).

To accomplish this in build time, basically, one needs to create the new modular jar and add it to the modulepath.
But (and that's a big but) the original jar needs to be removed from the modulpath, if not, one will end up with two jars, one as a named module and one in the unamed module.

The tricky part is that the original jar, is on the classpath, that's where the modulepath gets it from to begin with.
And as we can't actually manipulate the classpath, I was left with manipulating the modulepath prior to its construction.
That took some effort, but I was eventually able to get it to work, I've shared my findings, you can have a look here.

@dstaudt
Copy link
Collaborator

dstaudt commented Jul 27, 2022

Note, new enhancement issue tracking id: CSCvw64751

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

No branches or pull requests

2 participants