3270 spi classes exposed on the API/TPI

[techcobweb]

Background

macro4 have noted that if you use an ITerminal and some methods on that interface, you are forced to import 'spi' classes...

dev.galasa.zos3270.ITerminal exposes:

import dev.galasa.zos3270.spi.Colour;  << an enum of colours
import dev.galasa.zos3270.spi.Highlight; << an enum of highlights
import dev.galasa.zos3270.spi.NetworkException; << an exception (obviously).

So you have little choice but import those if you want to use ITerminal.

Those classes need to be migrated into the 'api' package if the api exposes them.

This would be a breaking changed if implemented, so need to tread carefully on when this change is made to minimise the disruption.

Chat https://galasa.slack.com/archives/C013FE51SAD/p1683814876054829

Julian Levens

I note that we appear to be using classes with spi in the name, but according to https://galasa.dev/docs/writing-own-tests/writing-test-classes we shouldn't galasa.devgalasa.dev Writing test classes Having created your project structure and built the parent and sub-projects, you are ready to start writing your own test classes. The… 13 replies

Mike Cobbett

Which classes ? The objects you are given via injections from annotations of galasa managers will use interfaces which don't have either 'internal' or 'spi' in the name... so you shouldn't ever refer to them via any java imports directly. But from logs or reflection, or from stack traces, you will see that the implementations of those interfaces often have spi or internal in their class names. ie: It's ok to use objects which are injected, as you are using them through a published interface, and the classes hold the implementation... but it's not OK to do anything which needs an import of spi or internal code. Perhaps we could spell that out better.

Julian Levens

They are referred to with imports, that was my concern

Mike Cobbett

yea. Which ones ?

Julian Levens

import dev.galasa.zos3270.spi.NetworkException;

(He mentioned 2 others, which aren't important for this issue)

Mike Cobbett

And this is from normal galasa test code, not code for a galasa manager ?

Julian Levens

Yes Although I do wonder if it should become a manger

Mike Cobbett

Maybe the code which deals with those classes should be elevated into a custom manager (which are allowed to access spi classes), so perhaps exposing a custom interface and delegating to the spi objects...etc. We'll take a look at the implementations of those classes and get back to you.

Mike Cobbett

Looks like dev.galasa.zos3270.ITerminal exposes

import dev.galasa.zos3270.spi.Colour;
import dev.galasa.zos3270.spi.Highlight;
import dev.galasa.zos3270.spi.NetworkException;

So you have little choice but import those if you want to use ITerminal. I think ideally those classes should be moved into the non-spi java package... but that would be a breaking change... I will raise an issue about that.

[techcobweb]

The best way to fix this issue is to re-structure all of the managers so that the 'api' 'spi' and 'implementation' are all in separate OSGi bundles, and let OSGi enforce that the the dependencies all run from impl->spi->api. Then OSGi would police our recommendations about what package name things should be inside. However, this is never going to happen, as it would be too disruptive. If we were doing this, then the above mistake couldn't have been made.

[techcobweb]

I prototyped a fix for Colour. It worked OK. Partly because I could create a new API class called Color (USA spelling) to disambiguate the two.

When I came to Highlight, the code gets more messy, disambiguation requires fully qualified package names to be injected into the code ... less readable.

The Network exception cascades into soooo many areas that fixing it is not an option.

My current thought is to leave this story, and fix at some point later (between v1.0 and v2.0 of Galasa). Where we can handle the breaking change more easily perhaps.

Current customers are top priority, so putting up with a bit of a messy interface here in favour of not breaking their test cases.