fix: Add PHP open api generator for list operations #340

isha689 · 2022-11-22T13:37:50Z

Fixes

PHP open api code generation for list operation
Need feedbacks on PHP code
Unit test will be added in next iteration for list operations
Comments resolution from previous PR
Constructor uri resolved for list files

Checklist

If you have questions, please create a GitHub Issue in this repository.

childish-sambino · 2022-11-22T16:28:58Z

src/main/java/com/twilio/oai/TwilioPhpGenerator.java

+    }
+
+    private APIResources processCodegenOperations(List<CodegenOperation> opList) {
+        return new PHPAPIResourceBuilder(new PHPAPIActionTemplate(this), opList, this.allModels)


Prefer CamelCase for Php and Api. Makes reading the name easier.

childish-sambino · 2022-11-22T16:31:05Z

src/main/java/com/twilio/oai/TwilioPhpGenerator.java

+        return new PHPAPIResourceBuilder(new PHPAPIActionTemplate(this), opList, this.allModels)
+                .apiPath()
+                .additionalProps(directoryStructureService)
+                .template()


Naming is too generic. What does apiPath? Return the path, build the path, update the path? Same applies for the other names.

childish-sambino · 2022-11-22T16:32:20Z

src/main/java/com/twilio/oai/TwilioPhpGenerator.java

+    public OperationsMap postProcessOperationsWithModels(final OperationsMap objs, List<ModelMap> allModels) {
+        final OperationsMap results = super.postProcessOperationsWithModels(objs, allModels);
+        final List<CodegenOperation> opList = directoryStructureService.processOperations(results);
+        APIResources apiResources = processCodegenOperations(opList);


I like how clean this is. Should extend the refactoring to Node so we can see how this limits duplication as part of this review.

childish-sambino · 2022-11-22T16:35:07Z

src/main/java/com/twilio/oai/api/APIResourceBuilder.java

+    public APIResourceBuilder operations(ISchemaResolver<CodegenParameter> codegenParameterIResolver) {
+        this.codegenOperationList.stream().forEach(codegenOperation -> {
+            codegenOperation.pathParams = codegenOperation.pathParams.stream().map(item ->
+                    codegenParameterIResolver.resolve(item)).collect(Collectors.toList());


What does it mean to resolve a parameter?

childish-sambino · 2022-11-22T16:37:13Z

src/main/java/com/twilio/oai/api/APIResources.java

+
+import static com.twilio.oai.common.ApplicationConstants.PATH_SEPARATOR_PLACEHOLDER;
+
+public class APIResources {


childish-sambino · 2022-11-22T16:40:25Z

src/main/java/com/twilio/oai/api/APIResourceBuilder.java

+    public final static String META_LIST_PARAMETER_KEY = "x-list-parameters";
+    public final static String META_CONTEXT_PARAMETER_KEY = "x-context-parameters";
+
+    protected IAPIActionTemplate phpTemplate;


Shouldn't be called php.

childish-sambino · 2022-11-22T16:41:33Z

src/main/java/com/twilio/oai/template/PHPAPIActionTemplate.java

+    public final static String TEMPLATE_TYPE_LIST = "list";
+    public final static String TEMPLATE_TYPE_CONTEXT = "context";
+    public final static String TEMPLATE_TYPE_OPTIONS = "options";
+    public final static String TEMPLATE_TYPE_INSTANCE = "instance";
+    public final static String TEMPLATE_TYPE_PAGE = "page";


Should these be defined in a non-php context?

childish-sambino · 2022-11-22T16:41:49Z

src/main/java/com/twilio/oai/template/PHPAPIActionTemplate.java

+        codegen.apiTemplateFiles().clear();
+        codegen.apiTestTemplateFiles().clear();
+        codegen.modelTestTemplateFiles().clear();
+        codegen.apiDocTemplateFiles().clear();
+        codegen.modelDocTemplateFiles().clear();


Isn't the codegen adapter already doing this?

childish-sambino · 2022-11-22T16:42:07Z

src/main/java/com/twilio/oai/template/PHPAPIActionTemplate.java

+
+    private Map<String, List<String>> mapping() {
+        return Map.ofEntries(
+                new AbstractMap.SimpleEntry<>("list", Arrays.asList("list.mustache", "List.php")),


These keys are already defined elsewhere.

childish-sambino · 2022-11-22T16:42:34Z

src/main/resources/twilio-php/context.mustache

+
+        // Path Solution
+        $this->solution = ['accountSid' => $accountSid, 'callSid' => $callSid, 'sid' => $sid, ];
+        $this->uri = '/Accounts/' . \rawurlencode($accountSid) . '/Calls/' . \rawurlencode($callSid) . '/Notifications/' . \rawurlencode($sid) . '.json';
