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

Allow to define a default DocTypeTransformer for each content type #2770

Open
driver733 opened this issue Jun 21, 2023 · 4 comments
Open

Allow to define a default DocTypeTransformer for each content type #2770

driver733 opened this issue Jun 21, 2023 · 4 comments

Comments

@driver733
Copy link

driver733 commented Jun 21, 2023

🤔 What's the problem you're trying to solve?

I am trying to find a way to define a single common DocType transformer for a contentType, which would be able to transform the given docString to the given Type. Similarly to the way the @DefaultParameterTransformer and other @Default... transformers work. This is necessary in order to get pre-converted objects of the targetType in the test steps.

@Given("Step with a JSON DocString with type A")
public void step1(A a) {
        
}

@Given("Step with a JSON DocString with type B")
public void step1(B a) {

}

✨ What's your proposed solution?

Add an option to define a @DefaultDocTypeTransformer for a contentType.

@DefaultDocStringTransformer(contentType = "json")
public Object transformJsonDocString(String json, Type toValueType) throws JsonProcessingException {
    return objectMapper.readValue(json, objectMapper.constructType(toValueType));
}

It would also be useful to be able to define, a fallback transformer, which is used if no transformers have been found for the corresponding contentType.

@DefaultDocStringTypeTransformer
public Object convert(String contentType, String docString, Type toType) {
    return ...
}

⛏ Have you considered any alternatives or workarounds?

As of now it's necessary to define a DocType transformer for each of the targetTypes, which leads to writing a lot of transformer methods with the same code.

@DocStringType(contentType = "json")
public A transformToA(String json) throws JsonProcessingException {
    return objectMapper.readValue(json, A.class);
}

@DocStringType(contentType = "json")
public B transformToA(String json) throws JsonProcessingException {
    return objectMapper.readValue(json, B.class);
}

📚 Any additional context?

The way I see it is that the DocStringTypeRegistryDocStringConverter class will fallback to the @DefaultDocTypeTransformer(contentType = "X") for the given contentType if it fails to find a DocString transformer for the given pair of a contentType and targetType. If there is no @DefaultDocTypeTransformer(contentType = "X") for the given contenType then the DocStringTypeRegistryDocStringConverter will fallback to the @DefaultDocTypeTransformer, which can transform a DocString with any given contentType to the given targetType.

@jkronegg
Copy link
Contributor

Having a DefaultDocTypeTransformer is not very flexible because you can have only one. Would it make more sense to have instead a DocType transformer/converter which is able to convert more than one class ? Let's say a transformer which can convert classes A and B, and another transformer which could convert classes C and D. In terms of implementation, this transformer whould have a method to determine if a type can be converted:

public boolean canConvert(Type type) {
    return ...
}

For a transformer which can convert any JSON into a Java type, the canConvert(Type) method would return always true.

@driver733
Copy link
Author

driver733 commented Jul 1, 2023

@jkronegg
What I meant is to have one transformer method for each contentType and a fallback transformer.

For example, you can have a transformer method for the JSON contentType.

@DefaultDocStringTypeTransformer(contentType="json")
public Object convert(String json, Type toType) {
      return objectMapper.readValue(json, objectMapper.constructType(toType));
}

And you can also have a transformer method for the XML contentType.

@DefaultDocStringTypeTransformer(contentType="xml")
public Object convert(String xml, Type toType) {
      return ...
}

Lastly, you can have a fallback transformer, which is used if no transformers have been found for the corresponding contentType.

@DefaultDocStringTypeTransformer
public Object convert(String contentType, String docString, Type toType) {
      return ...
}

@driver733 driver733 changed the title Allow to define a @DefaultDocTypeTransformer Allow to define a default DocTypeTransformer for each content type Jul 1, 2023
@jkronegg
Copy link
Contributor

jkronegg commented Jul 2, 2023

@driver733 yes, sorry, I was unclear. I was suggesting to add a doctype transformer interface such as:

    interface MultiDocTypeTransformer {
        /**
         Returns true if a content of the given contentType can be converted to toType.
        */
        public boolean canConvert(String contentType, Type toType);

        /**
         * Converts a string content of the given contentType to the toType type. Throws an exception when not possible.
         */
        public Object convert(String content, String contentType, Type toType);
    }

Then your implementation would be:

    class MyMultiDocTypeTransformer implements MultiDocTypeTransformer {
        public boolean canConvert(String contentType, Type toType) {
            return "json".equals(contentType);
        }
        public Object convert(String json, String contentType, Type toType) {
            return objectMapper.readValue(json, objectMapper.constructType(toType));
        }
    }

Wouldn't be this approach more flexible than the default doctype transformer you proposes ? (this is a simple question, not a proposal, because I didn't check the feasibility).

@driver733
Copy link
Author

@jkronegg
I think that it can work (theoretically), but since Cucumber-JVM has transitioned from the TypeRegistryConfigurer interface to the @ParameterType and @DocStringType annotations, I believe it would be better to use the annotations here too.

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