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 <tsfarr@amazon.com> * Address review comments Signed-off-by: Thomas Farr <tsfarr@amazon.com> --------- Signed-off-by: Thomas Farr <tsfarr@amazon.com>
Xtansia · Aug 30, 2024 · 42e2928 · 42e2928
1 parent eb5ba5f
commit 42e2928
Show file tree

Hide file tree

Showing 23 changed files with 439 additions and 169 deletions.
diff --git a/...ent/opensearch/OpenSearchAsyncClient.java → ...opensearch/OpenSearchAsyncClientBase.java b/...ent/opensearch/OpenSearchAsyncClient.java → ...opensearch/OpenSearchAsyncClientBase.java
@@ -40,6 +40,7 @@
 import java.util.concurrent.CompletableFuture;
 import javax.annotation.Generated;
 import javax.annotation.Nullable;
+import org.opensearch.client.ApiClient;
 import org.opensearch.client.opensearch._types.OpenSearchException;
 import org.opensearch.client.opensearch.core.InfoRequest;
 import org.opensearch.client.opensearch.core.InfoResponse;
@@ -52,20 +53,11 @@
  * Client for the namespace.
  */
 @Generated("org.opensearch.client.codegen.CodeGenerator")
-public class OpenSearchAsyncClient extends OpenSearchAsyncClientBase<OpenSearchAsyncClient> {
-    public OpenSearchAsyncClient(OpenSearchTransport transport) {
-        super(transport, null);
-    }
-
-    public OpenSearchAsyncClient(OpenSearchTransport transport, @Nullable TransportOptions transportOptions) {
+public abstract class OpenSearchAsyncClientBase<Self extends OpenSearchAsyncClientBase<Self>> extends ApiClient<OpenSearchTransport, Self> {
+    public OpenSearchAsyncClientBase(OpenSearchTransport transport, @Nullable TransportOptions transportOptions) {
         super(transport, transportOptions);
     }
 
-    @Override
-    public OpenSearchAsyncClient withTransportOptions(@Nullable TransportOptions transportOptions) {
-        return new OpenSearchAsyncClient(this.transport, transportOptions);
-    }
-
     // ----- Child clients
 
     public OpenSearchDanglingIndicesAsyncClient danglingIndices() {

diff --git a/...h/client/opensearch/OpenSearchClient.java → ...ient/opensearch/OpenSearchClientBase.java b/...h/client/opensearch/OpenSearchClient.java → ...ient/opensearch/OpenSearchClientBase.java
@@ -39,6 +39,7 @@
 import java.io.IOException;
 import javax.annotation.Generated;
 import javax.annotation.Nullable;
+import org.opensearch.client.ApiClient;
 import org.opensearch.client.opensearch._types.OpenSearchException;
 import org.opensearch.client.opensearch.core.InfoRequest;
 import org.opensearch.client.opensearch.core.InfoResponse;
@@ -51,20 +52,11 @@
  * Client for the namespace.
  */
 @Generated("org.opensearch.client.codegen.CodeGenerator")
-public class OpenSearchClient extends OpenSearchClientBase<OpenSearchClient> {
-    public OpenSearchClient(OpenSearchTransport transport) {
-        super(transport, null);
-    }
-
-    public OpenSearchClient(OpenSearchTransport transport, @Nullable TransportOptions transportOptions) {
+public abstract class OpenSearchClientBase<Self extends OpenSearchClientBase<Self>> extends ApiClient<OpenSearchTransport, Self> {
+    public OpenSearchClientBase(OpenSearchTransport transport, @Nullable TransportOptions transportOptions) {
         super(transport, transportOptions);
     }
 
-    @Override
-    public OpenSearchClient withTransportOptions(@Nullable TransportOptions transportOptions) {
-        return new OpenSearchClient(this.transport, transportOptions);
-    }
-
     // ----- Child clients
 
     public OpenSearchDanglingIndicesClient danglingIndices() {

diff --git a/...opensearch/OpenSearchAsyncClientBase.java → ...ent/opensearch/OpenSearchAsyncClient.java b/...opensearch/OpenSearchAsyncClientBase.java → ...ent/opensearch/OpenSearchAsyncClient.java
@@ -36,7 +36,6 @@
 import java.util.concurrent.CompletableFuture;
 import java.util.function.Function;
 import javax.annotation.Nullable;
-import org.opensearch.client.ApiClient;
 import org.opensearch.client.opensearch._types.ErrorResponse;
 import org.opensearch.client.opensearch._types.OpenSearchException;
 import org.opensearch.client.opensearch.cat.OpenSearchCatAsyncClient;
@@ -137,12 +136,20 @@
 /**
  * Client for the namespace.
  */
-public abstract class OpenSearchAsyncClientBase<Self extends OpenSearchAsyncClientBase<Self>> extends ApiClient<OpenSearchTransport, Self> {
+public class OpenSearchAsyncClient extends OpenSearchAsyncClientBase<OpenSearchAsyncClient> {
+    public OpenSearchAsyncClient(OpenSearchTransport transport) {
+        super(transport, null);
+    }
 
-    public OpenSearchAsyncClientBase(OpenSearchTransport transport, @Nullable TransportOptions transportOptions) {
+    public OpenSearchAsyncClient(OpenSearchTransport transport, @Nullable TransportOptions transportOptions) {
         super(transport, transportOptions);
     }
 
+    @Override
+    public OpenSearchAsyncClient withTransportOptions(@Nullable TransportOptions transportOptions) {
+        return new OpenSearchAsyncClient(this.transport, transportOptions);
+    }
+
     // ----- Child clients
 
     public OpenSearchCatAsyncClient cat() {

diff --git a/...ient/opensearch/OpenSearchClientBase.java → ...h/client/opensearch/OpenSearchClient.java b/...ient/opensearch/OpenSearchClientBase.java → ...h/client/opensearch/OpenSearchClient.java
@@ -35,7 +35,6 @@
 import java.io.IOException;
 import java.util.function.Function;
 import javax.annotation.Nullable;
-import org.opensearch.client.ApiClient;
 import org.opensearch.client.opensearch._types.ErrorResponse;
 import org.opensearch.client.opensearch._types.OpenSearchException;
 import org.opensearch.client.opensearch.cat.OpenSearchCatClient;
@@ -137,12 +136,20 @@
 /**
  * Client for the namespace.
  */
-public abstract class OpenSearchClientBase<Self extends OpenSearchClientBase<Self>> extends ApiClient<OpenSearchTransport, Self> {
+public class OpenSearchClient extends OpenSearchClientBase<OpenSearchClient> {
+    public OpenSearchClient(OpenSearchTransport transport) {
+        super(transport, null);
+    }
 
-    public OpenSearchClientBase(OpenSearchTransport transport, @Nullable TransportOptions transportOptions) {
+    public OpenSearchClient(OpenSearchTransport transport, @Nullable TransportOptions transportOptions) {
         super(transport, transportOptions);
     }
 
+    @Override
+    public OpenSearchClient withTransportOptions(@Nullable TransportOptions transportOptions) {
+        return new OpenSearchClient(this.transport, transportOptions);
+    }
+
     // ----- Child clients
     public OpenSearchGenericClient generic() {
         return new OpenSearchGenericClient(this.transport, this.transportOptions);

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();
         }