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 custom response templating #2350

Conversation

manofthepeace
Copy link

@manofthepeace manofthepeace commented Aug 31, 2023

References

fixes #2349

when manually creating a ResponseTemplateTransformer via WireMockConfiguration.extentions. It gets added to the loadedExtensions, but then gets overwritten by the put.

Submitter checklist

  • Recommended: Join WireMock Slack to get any help in #help-contributing or a project-specific channel like #wiremock-java
  • The PR request is well described and justified, including the body and the references
  • The PR title represents the desired changelog entry
  • The repository's code style is followed (see the contributing guide)
  • Test coverage that demonstrates that the change works as expected
  • For new features, there's necessary documentation in this pull request or in a subsequent PR to wiremock.org

@manofthepeace manofthepeace requested a review from a team as a code owner August 31, 2023 21:21
Copy link
Member

@oleg-nenashev oleg-nenashev left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Looks about right. If you could create a test case for this regression, it would be awesome

@tomakehurst
Copy link
Member

Please could you run ./gradlew spotlessApply so that the tests can be executed?

@manofthepeace
Copy link
Author

@tomakehurst done

@manofthepeace
Copy link
Author

@oleg-nenashev I have added a simple test that would fail without the PR.

@tomakehurst
Copy link
Member

Not a necessary fix per #2349 (comment).

Documentation will be updated soon to reflect the new, simplified way to enable templating.

@tomakehurst tomakehurst closed this Sep 5, 2023
@oleg-nenashev
Copy link
Member

@tomakehurst so it would be a new documented breaking change, right?

@tomakehurst
Copy link
Member

Yes, will add to the release note and update the main doc in due course.

@manofthepeace
Copy link
Author

manofthepeace commented Sep 5, 2023

@tomakehurst can you please re-evaluate. As an end user I find it a really hard to use behaviour. globalTemplating /global is one thing. But I need to have to customize the templateEngine, which I then need to put in the ResponseTemplateTransformer.

        WireMockConfiguration config = new WireMockConfiguration();
        FileSource fs = new ClasspathFileSource("src/test/resources/mocks/");
        TemplateEngine engine = new TemplateEngine(Map.of("json", Jackson2Helper.INSTANCE), null, null, true);
        config = config.port(9094).fileSource(fs)
                .extensions(new ResponseTemplateTransformer(engine, true, fs, Collections.emptyList()));
        wireMockServer = new WireMockServer(config);
        wireMockServer.start();

Why if I make the effort of specifying the ResponseTemplateTransformer myself, wiremocks just goes ahead and replaces it with its default implementation..

@tomakehurst
Copy link
Member

Again, I think the docs need improving here - if you want to add custom helpers you do it via another extension point now:
https://wiremock.org/docs/extensibility/adding-template-helpers/

It shouldn't ever be necessary as an end user to new up your own ResponseTemplateTransformer now. If there's anything that can't be customised via an extension or config param, we should be making these available instead.

@manofthepeace
Copy link
Author

OK, I can confirm all this is working as you said right above. Thanks for that, I totally missed that doc page. I think this should be noted in the migration steps of the release page. Since before we had to create the ResponseTemplateTransfomer, it seems sort of natural to still create by using the api available, which I went too far in the rabbit hole to adapt to the internal stuff that possible shoudn't be touched

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

Successfully merging this pull request may close these issues.

[3.0.0] Custom ResponseTemplateTransformer gets overwritten by default implementation
3 participants