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

Auto generate unknown fallback #1561

Merged
merged 7 commits into from
Mar 4, 2025

Conversation

habara-k
Copy link
Contributor

@habara-k habara-k commented Feb 28, 2025

Resolve #1537.
#1462

Summary

Previously, in the line-bot-sdk-java, the Unknown class was manually implemented and used as the defaultImpl (fallback) in @JsonTypeInfo. However, with the transition to generating code from OpenAPI definitions, generating the Unknown class directly from the OpenAPI spec became challenging. Consequently, the fallback option was disabled.

This change created an issue when decoding webhooks. The type field in webhooks may receive new values as future features are developed. If an older SDK version encounters these new values, it could crash while parsing the webhook.

This PR aims to re-enable the fallback option by adding code that automatically generates the Unknown class from the Java code produced by OpenAPI, even if the method is a bit rough or not entirely elegant . This ensures that we can maintain the fallback mechanism under OpenAPI management.

@habara-k habara-k force-pushed the auto-generate-unknown-fallback branch from bed706e to 05c8564 Compare February 28, 2025 08:37
Feat: Auto generate unknown fallback class for interface classes

Feat: Auto generate unknown fallback class for interface classes
@habara-k habara-k force-pushed the auto-generate-unknown-fallback branch from 05c8564 to 5d53f18 Compare February 28, 2025 08:39
@habara-k habara-k self-assigned this Feb 28, 2025
@habara-k habara-k requested a review from a team February 28, 2025 08:48
generate-code.py Outdated

CLASS_TEMPLATE = """package {package};

public record {unknown_class_name}() implements {interface_name} {{
Copy link
Contributor

Choose a reason for hiding this comment

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

what if an interface requires implementation class has to have some fields like MessageContent and UnknownMessageContent ?

public record UnknownMessageContent(String id) implements MessageContent {
}

Copy link
Contributor

Choose a reason for hiding this comment

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

It shouldn't compile. though the current patch seems working as of now, but given cases like this, modifying the generator's code might be a better choice.

In the generator's #postProcessModels, it can handle all models, so wouldn't it be better to create an unknown* class (like UnknownMessageContent ) that implements the interface here?

public ModelsMap postProcessModels(ModelsMap objs) {
ModelsMap modelsMap = super.postProcessModels(objs);
for (ModelMap model : modelsMap.getModels()) {
CodegenModel codegenModel = model.getModel();
// remove the `type` field from child classes due to remove duplicated type field with `@JsonTypeName`.
if (codegenModel.parent != null) {
codegenModel.setAllVars(codegenModel.getAllVars().stream()
.filter(codegenProperty -> !codegenProperty.name.equals("type"))
.collect(Collectors.toList()));
}
// if the class have a parent, set ` implements ${parent}`.
// and put @JsonTypeName annotation.
if (codegenModel.parent != null) {
addImplements(codegenModel, codegenModel.parent);
}
// Implements ReplyEvent if the class has "replyToken" field.
if (codegenModel.parent != null && codegenModel.parent.equals("Event")) {
if (codegenModel.vars.stream().anyMatch(codegenProperty -> codegenProperty.name.equals("replyToken"))) {
addImplements(codegenModel, "ReplyEvent");
}
}
}
return modelsMap;
}

(just an idea, probably this doesn't work without modification)

    @Override
    public ModelsMap postProcessModels(ModelsMap objs) {
        ModelsMap modelsMap = super.postProcessModels(objs);

+       List<ModelMap> additionalModels = new ArrayList<>();
        for (ModelMap model : modelsMap.getModels()) {
            CodegenModel codegenModel = model.getModel();

            // remove the `type` field from child classes due to remove duplicated type field with `@JsonTypeName`.
            if (codegenModel.parent != null) {
                codegenModel.setAllVars(codegenModel.getAllVars().stream()
                        .filter(codegenProperty -> !codegenProperty.name.equals("type"))
                        .collect(Collectors.toList()));
            }

            // if the class have a parent, set ` implements ${parent}`.
            // and put @JsonTypeName annotation.
            if (codegenModel.parent != null) {
                addImplements(codegenModel, codegenModel.parent);
            }

            // Implements ReplyEvent if the class has "replyToken" field.
            if (codegenModel.parent != null && codegenModel.parent.equals("Event")) {
                if (codegenModel.vars.stream().anyMatch(codegenProperty -> codegenProperty.name.equals("replyToken"))) {
                    addImplements(codegenModel, "ReplyEvent");
                }
            }

+          // Set additional unknown* class for jackson's defaultImpl to have unknwon* class as fallback
+          if (codegenModel.discriminator != null) {
+              String fallbackModelName = "Unknown" + codegenModel.classname;
+              CodegenModel fallbackModel = new CodegenModel();
+              fallbackModel.name = fallbackModelName;
+              // set some fields 
+              // ...
+              addImplements(fallbackModel, codegenModel.classname);
+              ModelMap fallbackModelMap = new ModelMap();
+              fallbackModelMap.put("model", fallbackModel);
+              additionalModels.add(fallbackModelMap);
            }
        }
+       modelsMap.getModels().addAll(additionalModels);
        return modelsMap;
    }

@habara-k habara-k force-pushed the auto-generate-unknown-fallback branch 2 times, most recently from bed9b99 to 1fc45d9 Compare March 3, 2025 11:07
@habara-k habara-k force-pushed the auto-generate-unknown-fallback branch from 1fc45d9 to 461ca1c Compare March 3, 2025 11:39
@habara-k
Copy link
Contributor Author

habara-k commented Mar 3, 2025

@Yang-33
Thank you for the code idea. As I mentioned in the PR description, the previous approach was rough and inelegant, lacking flexibility. I have revised the code based on the draft you provided.

@habara-k habara-k requested a review from Yang-33 March 3, 2025 11:51
@habara-k habara-k force-pushed the auto-generate-unknown-fallback branch from e395b61 to 1850e1b Compare March 4, 2025 00:46
@habara-k habara-k force-pushed the auto-generate-unknown-fallback branch from 1850e1b to 40c842c Compare March 4, 2025 01:05
@habara-k habara-k merged commit 9f81ade into line:master Mar 4, 2025
5 checks passed
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

Successfully merging this pull request may close these issues.

sdk must not throw any exception when unknown type is came as webhook or api response
2 participants