Invert abstractness of generated vs "handwritten" client classes (ope…

…nsearch-project#1170) * Invert abstractness of generated vs "handwritten" client classes Signed-off-by: Thomas Farr <[email protected]> * Address review comments Signed-off-by: Thomas Farr <[email protected]> --------- Signed-off-by: Thomas Farr <[email protected]> (cherry picked from commit 42e2928)
Xtansia · Aug 30, 2024 · 4ccbab3 · 4ccbab3
1 parent ef61281
commit 4ccbab3
Show file tree

Hide file tree

Showing 19 changed files with 413 additions and 141 deletions.
diff --git a/java-codegen/src/main/java/org/opensearch/client/codegen/model/Namespace.java b/java-codegen/src/main/java/org/opensearch/client/codegen/model/Namespace.java
@@ -82,8 +82,10 @@ public void render(ShapeRenderingContext ctx) throws RenderException {
 
         if (operations.isEmpty()) return;
 
-        new Client(this, false, operations).render(ctx);
-        new Client(this, true, operations).render(ctx);
+        var asBaseClass = "".equals(name);
+
+        new Client(this, false, asBaseClass, operations).render(ctx);
+        new Client(this, true, asBaseClass, operations).render(ctx);
     }
 
     private Collection<RequestShape> getOperationsForClient() {
@@ -95,32 +97,37 @@ private String getClientClassName(boolean async, boolean base) {
         return "OpenSearch" + Strings.toPascalCase(name) + (async ? "Async" : "") + "Client" + (base ? "Base" : "");
     }
 
-    private Type getClientType(boolean async, boolean base) {
-        var type = Type.builder().pkg(getPackageName()).name(getClientClassName(async, base));
-        if (base) {
-            type.typeParams(getClientType(async, false));
-        }
-        return type.build();
+    private Type getClientType(boolean async) {
+        return Type.builder().withPackage(getPackageName()).withName(getClientClassName(async, false)).build();
     }
 
     private static class Client extends Shape {
         private final boolean async;
+        private final boolean base;
         private final Collection<RequestShape> operations;
 
-        private Client(Namespace parent, boolean async, Collection<RequestShape> operations) {
-            super(parent, parent.getClientClassName(async, false), null, "Client for the " + parent.name + " namespace.");
+        private Client(Namespace parent, boolean async, boolean base, Collection<RequestShape> operations) {
+            super(parent, parent.getClientClassName(async, base), null, "Client for the " + parent.name + " namespace.");
             this.async = async;
+            this.base = base;
             this.operations = operations;
         }
 
+        @Override
+        public TypeParameterDiamond getTypeParameters() {
+            if (!base) return null;
+            var thisType = getType().withTypeParams(Type.builder().withName("Self").build());
+            return TypeParameterDiamond.builder()
+                .withParams(TypeParameterDefinition.builder().withName("Self").withExtends(thisType).build())
+                .build();
+        }
+
         @Override
         public Type getExtendsType() {
-            switch (parent.name) {
-                case "":
-                    return parent.getClientType(async, true);
-                default:
-                    return Types.Client.ApiClient(Types.Client.Transport.OpenSearchTransport, getType());
-            }
+            return Types.Client.ApiClient(
+                Types.Client.Transport.OpenSearchTransport,
+                !base ? getType() : Type.builder().withName("Self").build()
+            );
         }
 
         public String getName() {
@@ -139,6 +146,10 @@ public boolean isAsync() {
             return this.async;
         }
 
+        public boolean isBase() {
+            return this.base;
+        }
+
         @Override
         public String toString() {
             return new ToStringBuilder(this).append("type", getType()).toString();
@@ -149,7 +160,7 @@ private static class ClientRef {
             private final String name;
 
             public ClientRef(Namespace namespace, boolean async) {
-                this.type = namespace.getClientType(async, false);
+                this.type = namespace.getClientType(async);
                 this.name = namespace.name;
             }
 

diff --git a/java-codegen/src/main/java/org/opensearch/client/codegen/model/RequestShape.java b/java-codegen/src/main/java/org/opensearch/client/codegen/model/RequestShape.java
@@ -78,7 +78,7 @@ public void addSupportedHttpMethod(String method) {
     }
 
     public Type getResponseType() {
-        return Type.builder().pkg(getPackageName()).name(responseClassName(operationGroup)).build();
+        return Type.builder().withPackage(getPackageName()).withName(responseClassName(operationGroup)).build();
     }
 
     public boolean canBeSingleton() {

diff --git a/java-codegen/src/main/java/org/opensearch/client/codegen/model/Shape.java b/java-codegen/src/main/java/org/opensearch/client/codegen/model/Shape.java
@@ -61,6 +61,10 @@ public Collection<Type> getAnnotations() {
         return Collections.emptyList();
     }
 
+    public TypeParameterDiamond getTypeParameters() {
+        return null;
+    }
+
     public Type getExtendsType() {
         return null;
     }
@@ -70,7 +74,7 @@ public Collection<Type> getImplementsTypes() {
     }
 
     public Type getType() {
-        return Type.builder().pkg(getPackageName()).name(className).build();
+        return Type.builder().withPackage(getPackageName()).withName(className).build();
     }
 
     public Namespace getParent() {

diff --git a/java-codegen/src/main/java/org/opensearch/client/codegen/model/ShapeRenderingContext.java b/java-codegen/src/main/java/org/opensearch/client/codegen/model/ShapeRenderingContext.java
@@ -16,6 +16,7 @@
 import org.opensearch.client.codegen.renderer.TemplateLoader;
 import org.opensearch.client.codegen.renderer.TemplateRenderer;
 import org.opensearch.client.codegen.renderer.TemplateValueFormatter;
+import org.opensearch.client.codegen.utils.ObjectBuilderBase;
 import org.opensearch.client.codegen.utils.Strings;
 
 public final class ShapeRenderingContext implements AutoCloseable {
@@ -71,12 +72,21 @@ public static Builder builder() {
         return new Builder();
     }
 
-    public static final class Builder {
+    public static final class Builder extends ObjectBuilderBase<ShapeRenderingContext, Builder> {
         private File outputDir;
         private TemplateLoader templateLoader;
         private JavaCodeFormatter javaCodeFormatter;
         private boolean ownedJavaCodeFormatter;
 
+        private Builder() {
+            super(ShapeRenderingContext::new);
+        }
+
+        @Override
+        protected @Nonnull Builder self() {
+            return this;
+        }
+
         @Nonnull
         public Builder withOutputDir(@Nonnull File outputDir) {
             this.outputDir = Objects.requireNonNull(outputDir, "outputDir must not be null");
@@ -118,10 +128,5 @@ public Builder withJavaCodeFormatter(@Nonnull Function<JavaCodeFormatter.Builder
                 true
             );
         }
-
-        @Nonnull
-        public ShapeRenderingContext build() {
-            return new ShapeRenderingContext(this);
-        }
     }
 }
diff --git a/java-codegen/src/main/java/org/opensearch/client/codegen/model/SpecTransformer.java b/java-codegen/src/main/java/org/opensearch/client/codegen/model/SpecTransformer.java
@@ -387,8 +387,8 @@ private Type mapTypeInner(OpenApiSchema schema) {
             visit(schema);
 
             return Type.builder()
-                .pkg(Types.Client.OpenSearch.PACKAGE + "." + schema.getNamespace().orElseThrow())
-                .name(schema.getName().orElseThrow())
+                .withPackage(Types.Client.OpenSearch.PACKAGE + "." + schema.getNamespace().orElseThrow())
+                .withName(schema.getName().orElseThrow())
                 .isEnum(schema.hasEnums())
                 .build();
         }

diff --git a/java-codegen/src/main/java/org/opensearch/client/codegen/model/Type.java b/java-codegen/src/main/java/org/opensearch/client/codegen/model/Type.java
@@ -12,12 +12,14 @@
 import static org.opensearch.client.codegen.model.Types.Java;
 
 import com.samskivert.mustache.Mustache;
-import java.util.Arrays;
 import java.util.Set;
-import java.util.stream.Collectors;
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
 import org.opensearch.client.codegen.renderer.lambdas.TypeIsDefinedLambda;
 import org.opensearch.client.codegen.renderer.lambdas.TypeQueryParamifyLambda;
 import org.opensearch.client.codegen.renderer.lambdas.TypeSerializerLambda;
+import org.opensearch.client.codegen.utils.ObjectBuilderBase;
+import org.opensearch.client.codegen.utils.Strings;
 
 public class Type {
     private static final Set<String> PRIMITIVES = Set.of(
@@ -44,37 +46,41 @@ public static Builder builder() {
         return new Builder();
     }
 
-    private final String pkg;
+    @Nullable
+    private final String packageName;
+    @Nonnull
     private final String name;
+    @Nullable
     private final Type[] typeParams;
     private final boolean isEnum;
 
     private Type(Builder builder) {
-        this.pkg = builder.pkg;
-        this.name = builder.name;
+        this.packageName = builder.packageName;
+        this.name = Strings.requireNonBlank(builder.name, "name must not be blank");
         this.typeParams = builder.typeParams;
         this.isEnum = builder.isEnum;
     }
 
     public Builder toBuilder() {
-        return new Builder().pkg(pkg).name(name).typeParams(typeParams).isEnum(isEnum);
+        return new Builder().withPackage(packageName).withName(name).withTypeParameters(typeParams).isEnum(isEnum);
     }
 
     @Override
     public String toString() {
-        String str = name;
-        if (typeParams != null && typeParams.length > 0) {
-            str += "<";
-            str += Arrays.stream(typeParams).map(Type::toString).collect(Collectors.joining(", "));
-            str += ">";
+        var out = new StringBuilder();
+        out.append(name);
+        if (typeParams != null) {
+            TypeParameterDiamond.builder().withParams(typeParams).build().toString(out);
         }
-        return str;
+        return out.toString();
     }
 
+    @Nonnull
     public String getName() {
         return name;
     }
 
+    @Nonnull
     public Type getBoxed() {
         switch (name) {
             case "char":
@@ -171,7 +177,7 @@ public Type getBuilderFnType() {
     }
 
     public Type getNestedType(String name) {
-        return builder().pkg(pkg).name(this.name + "." + name).build();
+        return builder().withPackage(packageName).withName(this.name + "." + name).build();
     }
 
     public Mustache.Lambda serializer() {
@@ -183,9 +189,9 @@ public Mustache.Lambda directSerializer() {
     }
 
     public void getRequiredImports(Set<String> imports, String currentPkg) {
-        if (pkg != null && !pkg.equals(Java.Lang.PACKAGE) && !pkg.equals(currentPkg)) {
+        if (packageName != null && !packageName.equals(Java.Lang.PACKAGE) && !packageName.equals(currentPkg)) {
             var dotIdx = name.indexOf('.');
-            imports.add(pkg + '.' + (dotIdx > 0 ? name.substring(0, dotIdx) : name));
+            imports.add(packageName + '.' + (dotIdx > 0 ? name.substring(0, dotIdx) : name));
         }
         if (typeParams != null) {
             for (Type arg : typeParams) {
@@ -195,7 +201,7 @@ public void getRequiredImports(Set<String> imports, String currentPkg) {
     }
 
     public Type withTypeParams(Type... typeParams) {
-        return toBuilder().typeParams(typeParams).build();
+        return toBuilder().withTypeParameters(typeParams).build();
     }
 
     public Mustache.Lambda queryParamify() {
@@ -206,34 +212,43 @@ public Mustache.Lambda isDefined() {
         return new TypeIsDefinedLambda(this);
     }
 
-    public static final class Builder {
-        private String pkg;
+    public static final class Builder extends ObjectBuilderBase<Type, Builder> {
+        private String packageName;
         private String name;
         private Type[] typeParams;
         private boolean isEnum;
 
-        public Builder pkg(String pkg) {
-            this.pkg = pkg;
+        private Builder() {
+            super(Type::new);
+        }
+
+        @Override
+        protected @Nonnull Builder self() {
+            return this;
+        }
+
+        @Nonnull
+        public Builder withPackage(@Nullable String packageName) {
+            this.packageName = packageName;
             return this;
         }
 
-        public Builder name(String name) {
-            this.name = name;
+        @Nonnull
+        public Builder withName(@Nonnull String name) {
+            this.name = Strings.requireNonBlank(name, "name must not be blank");
             return this;
         }
 
-        public Builder typeParams(Type... typeParams) {
+        @Nonnull
+        public Builder withTypeParameters(@Nullable Type... typeParams) {
             this.typeParams = typeParams;
             return this;
         }
 
+        @Nonnull
         public Builder isEnum(boolean isEnum) {
             this.isEnum = isEnum;
             return this;
         }
-
-        public Type build() {
-            return new Type(this);
-        }
     }
 }
diff --git a/java-codegen/src/main/java/org/opensearch/client/codegen/model/TypeParameterDefinition.java b/java-codegen/src/main/java/org/opensearch/client/codegen/model/TypeParameterDefinition.java
@@ -0,0 +1,71 @@
+/*
+ * SPDX-License-Identifier: Apache-2.0
+ *
+ * The OpenSearch Contributors require contributions made to
+ * this file be licensed under the Apache-2.0 license or a
+ * compatible open source license.
+ */
+
+package org.opensearch.client.codegen.model;
+
+import javax.annotation.Nonnull;
+import javax.annotation.Nullable;
+import org.opensearch.client.codegen.utils.ObjectBuilderBase;
+import org.opensearch.client.codegen.utils.Strings;
+
+public final class TypeParameterDefinition {
+    @Nonnull
+    private final String name;
+    @Nullable
+    private final Type extendsType;
+
+    private TypeParameterDefinition(Builder builder) {
+        this.name = Strings.requireNonBlank(builder.name, "name must not be blank");
+        this.extendsType = builder.extendsType;
+    }
+
+    @Nonnull
+    public String getName() {
+        return name;
+    }
+
+    @Nullable
+    public Type getExtends() {
+        return extendsType;
+    }
+
+    @Override
+    public String toString() {
+        return name + (extendsType != null ? " extends " + extendsType : "");
+    }
+
+    public static Builder builder() {
+        return new Builder();
+    }
+
+    public static final class Builder extends ObjectBuilderBase<TypeParameterDefinition, Builder> {
+        private String name;
+        private Type extendsType;
+
+        private Builder() {
+            super(TypeParameterDefinition::new);
+        }
+
+        @Override
+        protected @Nonnull Builder self() {
+            return this;
+        }
+
+        @Nonnull
+        public Builder withName(@Nonnull String name) {
+            this.name = Strings.requireNonBlank(name, "name must not be blank");
+            return this;
+        }
+
+        @Nonnull
+        public Builder withExtends(@Nullable Type type) {
+            this.extendsType = type;
+            return this;
+        }
+    }
+}