https://linen.dev logo
Powered by
Title
n

Nikita Karpuk

05/09/2023, 6:00 PM
Hello #wiremock-java i have an improvement idea for 
wiremock-testcontainers-java
and would like to validate it with you Current API 
private final Map<String, Extension> extensions = new HashMap<>();

WireMockContainer withExtension(String id, Collection<String> classNames, Collection<File> jars)
WireMockContainer withExtension(String id, Collection<String> classNames, File jarDirectory)
WireMockContainer withExtension(String id, String className)
example of usage 
.withExtension("Webhook",
    Collections.singleton("org.wiremock.webhooks.Webhooks"),
    Collections.singleton(Paths.get("target", "test-wiremock-extension", "wiremock-webhooks-extension-2.35.0.jar").toFile()))
.withExtension("JSON Body Transformer",
    Collections.singleton("com.ninecookies.wiremock.extensions.JsonBodyTransformer"),
    Collections.singleton(Paths.get("target", "test-wiremock-extension", "wiremock-extensions-0.4.1-jar-with-dependencies.jar").toFile()));
Problem: • Field 
id
brings confusion, it forces developers to create random string literal for each method invocation, since this 
id
has no influence on behavior - values will be very random (absence of control / validation) • We store extention details 
className
and 
jar
as a value in 
HashMap<ID, Extention>
, so we are not able to utilize Collections benefits like check for uniqueness (HashSet) • Even if developer mixed classNames with wrong jars 
withExtnetion("ExtA", "JarB").withExtention("ExtB", "JarA")
it will work Improvement Options: Option 1 
private final Set<String> extensionClassNames = new LinkedHashSet<>();
private final Set<File> extensionJars = new LinkedHashSet<>();

WireMockContainer withExtension(Collection<String> classNames, File jar)
WireMockContainer withExtension(Collection<String> classNames, Collection<File> jars);
WireMockContainer withExtension(Collection<String> classNames, Path jarDirectory)
Benefits: • Store all extensionJars in a LinkedHashSet to preserve an order and keep only unique values • Store all extensionClassNames in a LinkedHashSet to preserve an order and keep only unique value • all params for 
--extention
will be stored in one collection 
extensionClassNames
- easy to build cli command for docker • all params for 
/var/wiremock/extentions
will be stored in one collection 
extensionJars
- easy to build cli command for docker • Allow developer to organize and keep related Jars and classNames in one method Problem: • Even if developer mixed classNames with wrong jars 
withExtnetion("ExtA", "JarB").withExtention("ExtB", "JarA")
it will work Option 2 
private final Set<String> extensionClassNames = new LinkedHashSet<>();
private final Set<File> extensionJars = new LinkedHashSet<>();

WireMockContainer withExtensionFile(Collection<String> classNames)
WireMockContainer withExtensionClass(Collection<File> jars);
Benefits: • Store all extensionJars in a LinkedHashSet to preserve an order and keep only unique values • Store all extensionClassNames in a LinkedHashSet to preserve an order and keep only unique value • all params for 
--extention
will be stored in one collection 
extensionClassNames
- easy to build cli command for docker • all params for 
/var/wiremock/extentions
will be stored in one collection 
extensionJars
- easy to build cli command for docker • Very straignt forward api
P.S Personally i like option 1 and here i attempted to implement it https://github.com/wiremock/wiremock-testcontainers-java/pull/36 Would be nice to here your point of view on this Options or suggest another one 🙂
o

Oleg Nenashev

05/10/2023, 11:03 AM
Thanks! And sorry for missing it. Will respond in a few hours
Well, it was quite a few hours :( I think that the idea to simplify the API in the PR is good and definitely should be kept. Right now there is no actual use for ID in the interface and hence it should not be required. For me that field is mostly for observability and future troubleshooting purposes, and I think that some kind of extension IDs will be actually needed in WireMock itself once we have a new extension engine as proposed by @Tom. Extension ID, for better or worse, make declarative definitions easier when there is some kind of automatic discovery in place
So basically my proposal would be to keep the entry in the internal representation but not expose it in the public APIs for now. IDs could be always inferred from the JAR name and class name, for example
We can also make things better by having enum with known extensions so that we could simplify usage of extensions in Testcontainers. I think it would be all so good use case for IDs that we could eventually propagate to WireMock Core
@Nikita Karpuk so, I propose option 3 where we keep the current private Extension class which is indeed suboptimal for now but can be extended in the future. While this class right now offers a venue for collisions of jar names and extensions, this is something that could be verified easily if we want to strengthen this part of the code. My belief is that the risk of collisions won't be that high compared to class loading collisions due to bogus shading or lack of shading at all. But yes we can do some checks
I am ready to submit the extension catalog tomorrow and those refactor your pull request in my Branch to integrate changes if that's fine with you
.... So we can compare two proposals
👍 1
n

Nikita Karpuk

05/12/2023, 7:30 PM
Sounds good, let’s see and compare 😎
👍 1
And i like idea of extension catalog
o

Oleg Nenashev

05/12/2023, 7:34 PM
Arguably, we could just include the common extensions right inside the Docker image so that the results are no build time overhead and external dependency risks. This approach won't be liked by supply chain security experts but it's good from the developer experience perspective
... especially since we don't have so many extensions anyway
n

Nikita Karpuk

05/12/2023, 7:36 PM
including extensions into docker image may bring overhead with versioning
o

Oleg Nenashev

05/12/2023, 7:37 PM
I wouldn't mind having almost+automatic dependency updates if we had proper integration test for the extensions
But yes it requires some investment which is probably too early right now.
n

Nikita Karpuk

05/12/2023, 7:38 PM
I mean it make sence to include 
org.wiremock.webhooks.Webhooks
into doker image since it belongs to same organization but for other third party extensions it could be complicated
o

Oleg Nenashev

05/12/2023, 7:40 PM
Yep. I started reaching out to some maintainers about moving to the organization but it takes some time. Also, in fact we need a lot more active maintainers as pretty much every other open source project :)
#wiremock-javaJoin Slack
Powered by