search

eclipse-basyx / basyx-java-sdk

java-sdk
MIT License
25 stars 32 forks source link

Suggest to remove obsolete (?) package org.eclipse.basyx.vab.protocol.opcua.server and dependency to Milo "sdk-server" #269

Closed in-fke closed 1 year ago

in-fke commented 1 year ago

Suggest to remove obsolete (?) package org.eclipse.basyx.vab.protocol.opcua.server and dependency to Milo "sdk-server". In fact, the only class used from Milo "sdk-server" is org.eclipse.milo.opcua.sdk.server.util.HostnameUtil and that would be a lot of dependency just for this class. See https://github.com/eclipse-basyx/basyx-java-sdk/blob/main/src/main/java/org/eclipse/basyx/vab/protocol/opcua/server/KeyStoreLoaderClient.java

in-fke commented 1 year ago

Dependency Footprint of Milo "sdk-server" (though, some are overlapping with "sdk-client")

[INFO] +- org.eclipse.milo:sdk-server:jar:0.6.8:compile
[INFO] |  +- org.eclipse.milo:stack-core:jar:0.6.8:compile
[INFO] |  |  +- org.bouncycastle:bcprov-jdk15on:jar:1.69:compile
[INFO] |  |  +- org.bouncycastle:bcpkix-jdk15on:jar:1.69:compile
[INFO] |  |  |  \- org.bouncycastle:bcutil-jdk15on:jar:1.69:compile
[INFO] |  |  \- org.glassfish.jaxb:jaxb-runtime:jar:2.3.6:compile
[INFO] |  |     +- jakarta.xml.bind:jakarta.xml.bind-api:jar:2.3.3:compile
[INFO] |  |     +- org.glassfish.jaxb:txw2:jar:2.3.6:compile
[INFO] |  |     +- com.sun.istack:istack-commons-runtime:jar:3.0.12:compile
[INFO] |  |     \- com.sun.activation:jakarta.activation:jar:1.2.2:compile
[INFO] |  +- org.eclipse.milo:stack-server:jar:0.6.8:compile
[INFO] |  \- org.eclipse.milo:bsd-generator:jar:0.6.8:compile
[INFO] |     \- org.eclipse.milo:bsd-core:jar:0.6.8:compile
FrankSchnicke commented 1 year ago

Thanks for pointing this out. However, removing this package would result in a breaking change for users who are utilizing the opcua integration of the VAB - e.g., the developers at Papyrus4Manufacturing. Thus, removing this package is not possible

in-fke commented 1 year ago

Well I guess 0.x version would tend to have breaking changes (that need to be announced) - and the class KeyStoreLoaderClient is already marked deprecated. It's nothing you need to rush - just something I noted when looking at all the transitive dependencies. If Papyrus4Manufacturing would depend on milo-server, it could include it as a direct dependency. If Papyrus4Manufacturing would "just" depend on KeyStoreLoaderClient they might as well "copy" it to their project?

If both are NOT depending on milo-server, - or just on HostnameUtil class in milo-server. You might as well copy the source code of org.eclipse.milo.opcua.sdk.server.util.HostnameUtil for the use in KeyStoreLoaderClient to basyx.sdk, thus effectively removing the dependency to milo-server, just because of this single class HostnameUtil.

FrankSchnicke commented 1 year ago

These are good ideas, however, they will also result most likely in breaking changes for our users. Thus, similar to https://github.com/eclipse-basyx/basyx-java-sdk/issues/268, we won't address this anymore in BaSyx Java V1.X