guardrail-dev / guardrail

Principled code generation from OpenAPI specifications
https://guardrail.dev
MIT License
522 stars 132 forks source link

dropwizard paths conflict with manually supplied paths #339

Open blast-hardcheese opened 5 years ago

blast-hardcheese commented 5 years ago

Given the following spec, with an already registered /Blix in a manually created resource, the route deduplication logic that comes from applying this patch changes the resource's path handler from @Path("/v1/Foo/{foo}/Bar/{bar}") to @Path("v1"), which causes some of the paths to start yielding 405 Method Not Allowed with no way to call those paths anymore.

 openapi: 3.0.1
 info:
   version: 1.0.0
   title: ExampleServer
   license:
     name: None

 servers:
   - url: http://127.0.0.1:1234/v1

 paths:
   /Foo/{foo}/Bar/{bar}/Baz/{baz}:
     parameters:
       - $ref: "#/components/parameters/fooPathParam"
       - $ref: "#/components/parameters/barPathParam"
       - $ref: "#/components/parameters/versionPathParam"
     get:
       operationId: getBazForBar
       x-jvm-package: foobarbaz
       responses:
         '200':
           description: OK
           content:
             application/json:
               schema:
                 $ref: "#/components/schemas/Blix"
         '400':
           $ref: "#/components/responses/BadRequest"
         '404':
           $ref: "#/components/responses/NotFound"
         '500':
           $ref: "#/components/responses/InternalServerError"
+
+  /Blix:
+    get:
+      operationId: getBlixList
+      x-jvm-package: foobarbaz
+      responses:
+        '200':
+          description: OK
+          content:
+            application/json:
+              schema:
+                $ref: "#/components/schemas/BlixList"
+        '400':
+          $ref: '#/components/responses/BadRequest'
+        '500':
+          $ref: "#/components/responses/InternalServerError"
 components:
   parameters:
     fooPathParam:
       in: path
       name: foo
       required: true
       schema:
         type: string
     barPathParam:
       in: path
       name: bar
       required: true
       schema:
         type: string
     bazPathParam:
       in: path
       name: baz
       required: true
       schema:
         type: integer
         format: int32

   responses:
     BadRequest:
       description: Bad Request
       content:
         application/json:
           schema:
             $ref: "#/components/schemas/ErrorResponse"
     NotFound:
       description: Not Found
       content:
         application/json:
           schema:
             $ref: "#/components/schemas/ErrorResponse"
     InternalServerError:
       description: Internal Server Error
       content:
         application/json:
           schema:
             $ref: "#/components/schemas/ErrorResponse"

   schemas:
     Foo:
       type: string

     ErrorResponse:
       type: object
       required:
         - code
         - message
       properties:
         message:
           type: string
         code:
           type: integer
           format: int32

     BlixList:
       type: object
       required:
         - results
         - meta
       properties:
         results:
           type: array
           items:
             $ref: "#/components/schemas/Blix"

     Blix:
       type: object
       required:
         - foo
       properties:
         foo:
           $ref: "#/components/schemas/Foo"
         friendly_name:
           type: string
kelnos commented 5 years ago

I'm not sure how we'd go about fixing this. DW/Jersey's resource class system assumes that you group related endpoints (with related URLs) together in the same resource. If you don't do that, then this happens. The solution is just: "don't do this".

blast-hardcheese commented 4 years ago

@kelnos Perhaps x-java-superclass for a group of routes then? Otherwise there's really no way to mix user-defined methods with guardrail-supplied methods.

kelnos commented 4 years ago

Ah, I think I misunderstood the use case. -superclass seems like a neat idea. So the idea would be that user-defined routes are defined in some class, and then you tell guardrail to make that class the superclass of the guardrail-generated class? That could be interesting.

Although, another option is that you put your user-defined routes in a subclass of the guardrail-generated class. That should still work, and then we don't need to add this?

Edit: That's a bit less flexible, though... the user-defined class might already have a superclass with some common methods and such, so my alternative wouldn't work for that case.