tsp, json-merge-patch, bug fixes (#2576)

* bug fixes: 1. set toJsonMergePatch as private, 2. switch the order of updatedProperties check and null check * regen * update tests according to comments * regen * add another test case for inheritance * format * add the test * bump a minor version * update the version * only disable convenient method when stream style is explicitly set to false * regen
Azure · Mar 1, 2024 · 79d9d6b · 79d9d6b
1 parent 943564c
commit 79d9d6b
Show file tree

Hide file tree

Showing 29 changed files with 531 additions and 247 deletions.
diff --git a/javagen/src/main/java/com/azure/autorest/template/StreamSerializationModelTemplate.java b/javagen/src/main/java/com/azure/autorest/template/StreamSerializationModelTemplate.java
@@ -262,7 +262,7 @@ private static void writeToJson(JavaClass classBlock, ClientModelPropertiesManag
     private static void writeToJsonMergePatch(JavaClass classBlock, ClientModelPropertiesManager propertiesManager,
         Consumer<JavaClass> addGeneratedAnnotation) {
         addGeneratedAnnotation.accept(classBlock);
-        classBlock.publicMethod("JsonWriter toJsonMergePatch(JsonWriter jsonWriter) throws IOException",
+        classBlock.privateMethod("JsonWriter toJsonMergePatch(JsonWriter jsonWriter) throws IOException",
             methodBlock -> serializeJsonProperties(methodBlock, propertiesManager, true));
     }
 
@@ -337,10 +337,12 @@ private static void serializeJsonProperty(JavaBlock methodBlock, ClientModelProp
 
         if (isJsonMergePatch) {
             if (property.getClientType().isNullable()) {
-                methodBlock.ifBlock(String.format("%s!=null", getPropertyGetterStatement(property, fromSuperType)), codeBlock -> {
-                    serializeJsonProperty(codeBlock, property, serializedName, fromSuperType, isJsonMergePatch);
-                }).elseIfBlock(String.format("updatedProperties.contains(\"%s\")", property.getName()), codeBlock -> {
-                    codeBlock.line(String.format("jsonWriter.writeNullField(\"%s\");", property.getSerializedName()));
+                methodBlock.ifBlock(String.format("updatedProperties.contains(\"%s\")", property.getName()), codeBlock -> {
+                    codeBlock.ifBlock(String.format("%s==null", getPropertyGetterStatement(property, fromSuperType)), ifBlock -> {
+                        ifBlock.line(String.format("jsonWriter.writeNullField(\"%s\");", property.getSerializedName()));
+                    }).elseBlock(elseBlock -> {
+                        serializeJsonProperty(codeBlock, property, serializedName, fromSuperType, isJsonMergePatch);
+                    });
                 });
             } else {
                 serializeJsonProperty(methodBlock, property, serializedName, fromSuperType, isJsonMergePatch);

diff --git a/typespec-extension/changelog.md b/typespec-extension/changelog.md
@@ -1,5 +1,11 @@
 # Release History
 
+## 0.14.0 (2024-02-29)
+
+Compatible with compiler 0.53.
+
+- Enhanced convenience API for "application/merge-patch+json".
+
 ## 0.13.10 (2024-02-28)
 
 Compatible with compiler 0.53.

diff --git a/typespec-extension/package-lock.json b/typespec-extension/package-lock.json
diff --git a/typespec-extension/package.json b/typespec-extension/package.json
@@ -1,6 +1,6 @@
 {
   "name": "@azure-tools/typespec-java",
-  "version": "0.13.10",
+  "version": "0.14.0",
   "description": "TypeSpec library for emitting Java client from the TypeSpec REST protocol binding",
   "keywords": [
     "TypeSpec"

diff --git a/typespec-extension/src/code-model-builder.ts b/typespec-extension/src/code-model-builder.ts
@@ -708,7 +708,7 @@ export class CodeModelBuilder {
         generateConvenienceApi = false;
         apiComment = `Convenience API is not generated, as operation '${op.operation.name}' is multiple content-type`;
         this.logWarning(apiComment);
-      } else if (operationIsJsonMergePatch(op) && !this.options["stream-style-serialization"]) {
+      } else if (operationIsJsonMergePatch(op) && this.options["stream-style-serialization"] === false) {
         // do not generate convenient method for json merge patch operation if stream-style-serialization is not enabled
         generateConvenienceApi = false;
         apiComment = `Convenience API is not generated, as operation '${op.operation.name}' is 'application/merge-patch+json' and stream-style-serialization is not enabled`;

diff --git a/typespec-tests/package.json b/typespec-tests/package.json
@@ -10,7 +10,7 @@
   },
   "dependencies": {
     "@azure-tools/cadl-ranch-specs": "0.31.0",
-    "@azure-tools/typespec-java": "file:/../typespec-extension/azure-tools-typespec-java-0.13.10.tgz"
+    "@azure-tools/typespec-java": "file:/../typespec-extension/azure-tools-typespec-java-0.14.0.tgz"
   },
   "devDependencies": {
     "@typespec/prettier-plugin-typespec": "~0.53.0",

diff --git a/typespec-tests/src/main/java/com/_specs_/azure/core/basic/models/User.java b/typespec-tests/src/main/java/com/_specs_/azure/core/basic/models/User.java
@@ -157,17 +157,21 @@ public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
     }
 
     @Generated
-    public JsonWriter toJsonMergePatch(JsonWriter jsonWriter) throws IOException {
+    private JsonWriter toJsonMergePatch(JsonWriter jsonWriter) throws IOException {
         jsonWriter.writeStartObject();
-        if (this.name != null) {
-            jsonWriter.writeStringField("name", this.name);
-        } else if (updatedProperties.contains("name")) {
-            jsonWriter.writeNullField("name");
+        if (updatedProperties.contains("name")) {
+            if (this.name == null) {
+                jsonWriter.writeNullField("name");
+            } else {
+                jsonWriter.writeStringField("name", this.name);
+            }
         }
-        if (this.orders != null) {
-            jsonWriter.writeArrayField("orders", this.orders, (writer, element) -> writer.writeJson(element));
-        } else if (updatedProperties.contains("orders")) {
-            jsonWriter.writeNullField("orders");
+        if (updatedProperties.contains("orders")) {
+            if (this.orders == null) {
+                jsonWriter.writeNullField("orders");
+            } else {
+                jsonWriter.writeArrayField("orders", this.orders, (writer, element) -> writer.writeJson(element));
+            }
         }
         return jsonWriter.writeEndObject();
     }

diff --git a/typespec-tests/src/main/java/com/_specs_/azure/core/basic/models/UserOrder.java b/typespec-tests/src/main/java/com/_specs_/azure/core/basic/models/UserOrder.java
@@ -141,13 +141,15 @@ public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
     }
 
     @Generated
-    public JsonWriter toJsonMergePatch(JsonWriter jsonWriter) throws IOException {
+    private JsonWriter toJsonMergePatch(JsonWriter jsonWriter) throws IOException {
         jsonWriter.writeStartObject();
         jsonWriter.writeIntField("userId", this.userId);
-        if (this.detail != null) {
-            jsonWriter.writeStringField("detail", this.detail);
-        } else if (updatedProperties.contains("detail")) {
-            jsonWriter.writeNullField("detail");
+        if (updatedProperties.contains("detail")) {
+            if (this.detail == null) {
+                jsonWriter.writeNullField("detail");
+            } else {
+                jsonWriter.writeStringField("detail", this.detail);
+            }
         }
         return jsonWriter.writeEndObject();
     }

diff --git a/typespec-tests/src/main/java/com/cadl/patch/PatchAsyncClient.java b/typespec-tests/src/main/java/com/cadl/patch/PatchAsyncClient.java
@@ -61,6 +61,13 @@ public final class PatchAsyncClient {
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * <p><strong>Response Body Schema</strong></p>
@@ -82,6 +89,13 @@ public final class PatchAsyncClient {
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * 
@@ -121,6 +135,13 @@ public Mono<Response<BinaryData>> createOrUpdateResourceWithResponse(BinaryData
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * <p><strong>Response Body Schema</strong></p>
@@ -142,6 +163,13 @@ public Mono<Response<BinaryData>> createOrUpdateResourceWithResponse(BinaryData
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * 

diff --git a/typespec-tests/src/main/java/com/cadl/patch/PatchClient.java b/typespec-tests/src/main/java/com/cadl/patch/PatchClient.java
@@ -59,6 +59,13 @@ public final class PatchClient {
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * <p><strong>Response Body Schema</strong></p>
@@ -80,6 +87,13 @@ public final class PatchClient {
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * 
@@ -118,6 +132,13 @@ public Response<BinaryData> createOrUpdateResourceWithResponse(BinaryData resour
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * <p><strong>Response Body Schema</strong></p>
@@ -139,6 +160,13 @@ public Response<BinaryData> createOrUpdateResourceWithResponse(BinaryData resour
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * 

diff --git a/typespec-tests/src/main/java/com/cadl/patch/implementation/PatchesImpl.java b/typespec-tests/src/main/java/com/cadl/patch/implementation/PatchesImpl.java
@@ -141,6 +141,13 @@ Response<BinaryData> createOrUpdateFishSync(@HostParam("endpoint") String endpoi
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * <p><strong>Response Body Schema</strong></p>
@@ -162,6 +169,13 @@ Response<BinaryData> createOrUpdateFishSync(@HostParam("endpoint") String endpoi
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * 
@@ -203,6 +217,13 @@ public Mono<Response<BinaryData>> createOrUpdateResourceWithResponseAsync(Binary
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * <p><strong>Response Body Schema</strong></p>
@@ -224,6 +245,13 @@ public Mono<Response<BinaryData>> createOrUpdateResourceWithResponseAsync(Binary
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * 
@@ -264,6 +292,13 @@ public Response<BinaryData> createOrUpdateResourceWithResponse(BinaryData resour
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * <p><strong>Response Body Schema</strong></p>
@@ -285,6 +320,13 @@ public Response<BinaryData> createOrUpdateResourceWithResponse(BinaryData resour
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * 
@@ -330,6 +372,13 @@ public Mono<Response<BinaryData>> createOrUpdateOptionalResourceWithResponseAsyn
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * <p><strong>Response Body Schema</strong></p>
@@ -351,6 +400,13 @@ public Mono<Response<BinaryData>> createOrUpdateOptionalResourceWithResponseAsyn
      *     array (Optional): [
      *         (recursive schema, see above)
      *     ]
+     *     fish (Optional): {
+     *         kind: String (Optional)
+     *         id: String (Required)
+     *         name: String (Required)
+     *         age: int (Optional, Required on create)
+     *         color: String (Optional)
+     *     }
      * }
      * }</pre>
      * 

diff --git a/typespec-tests/src/main/java/com/cadl/patch/models/Fish.java b/typespec-tests/src/main/java/com/cadl/patch/models/Fish.java
@@ -213,18 +213,22 @@ public JsonWriter toJson(JsonWriter jsonWriter) throws IOException {
     }
 
     @Generated
-    public JsonWriter toJsonMergePatch(JsonWriter jsonWriter) throws IOException {
+    private JsonWriter toJsonMergePatch(JsonWriter jsonWriter) throws IOException {
         jsonWriter.writeStartObject();
-        if (this.kind != null) {
-            jsonWriter.writeStringField("kind", this.kind);
-        } else if (updatedProperties.contains("kind")) {
-            jsonWriter.writeNullField("kind");
+        if (updatedProperties.contains("kind")) {
+            if (this.kind == null) {
+                jsonWriter.writeNullField("kind");
+            } else {
+                jsonWriter.writeStringField("kind", this.kind);
+            }
         }
         jsonWriter.writeIntField("age", this.age);
-        if (this.color != null) {
-            jsonWriter.writeStringField("color", this.color);
-        } else if (updatedProperties.contains("color")) {
-            jsonWriter.writeNullField("color");
+        if (updatedProperties.contains("color")) {
+            if (this.color == null) {
+                jsonWriter.writeNullField("color");
+            } else {
+                jsonWriter.writeStringField("color", this.color);
+            }
         }
         return jsonWriter.writeEndObject();
     }