Repository: eclipse/microprofile-open-api
Pull request #235 (2018-07-12 14:43:58)

Change methods to access Operations in PathItems
See original discussion in issue #234 (point `1.`)

I was surprised to not find any tests for the model in the TCK.

I understand that it focus on App-Server stuff (equivalent of ` 	swagger-jaxrs2`) but you have a set of constraints on the model interfaces as well.

For example, With:

```
PathItem p;
Operation o;
```

You can do:
```java
p.readOperationsMap().put(HttpMethod.POST, o);
```
Or:
```
p.setPOST(o);
```

Does the API tells if they are equivalent or not?

If you take the swagger-core implementation it is not. 
`readOperationsMap()` is somehow a view map (it should be unmodifiable in my opinion, this is an other story).

On the other side, doing this on the Kaizen model seems to update the the POST operation.

I think that going over the setter should be the recommended way.
Forcing the `getOperations()` to be unmodifiable is a to restrictive, but allowing modification is also a problem for implementation that do not keep the information inside a Map.

arthurdm: thanks @jmini.  Have you looked at this TCK test? https://github.com/eclipse/microprofile-open-api/blob/master/tck/src/main/java/org/eclipse/microprofile/openapi/tck/ModelConstructionTest.java   It tests the methods of the model classes.

I agree about the lack of description on how these methods interact.  I think that making the methods that return a map as immutable is a good idea.  I will open an issue for it.  
jmini: @arthurdm: I do not see any `readOperationsMap()` call in `ModelConstructionTest.java`.

Patch:
diff --git a/api/src/main/java/org/eclipse/microprofile/openapi/annotations/media/Schema.java b/api/src/main/java/org/eclipse/microprofile/openapi/annotations/media/Schema.java
index 7d710db..f023c24 100644
--- a/api/src/main/java/org/eclipse/microprofile/openapi/annotations/media/Schema.java
+++ b/api/src/main/java/org/eclipse/microprofile/openapi/annotations/media/Schema.java
@@ -111,8 +111,8 @@ public @interface Schema {
     double multipleOf() default 0;
 
     /**
-     * Sets the maximum numeric value for a property.  Value must be a valid number. 
-     * Ignored if the value is an empty string or not a number.
+     * Sets the maximum numeric value for a property. 
+     * Ignored if the value is an empty string.
      * 
      * @return the maximum value for this schema
      **/
@@ -126,7 +126,7 @@ public @interface Schema {
     boolean exclusiveMaximum() default false;
 
     /**
-     * Sets the minimum numeric value for a property. Value must be a valid number.
+     * Sets the minimum numeric value for a property. 
      * Ignored if the value is an empty string or not a number.
      * 
      * @return the minimum value for this schema
diff --git a/api/src/main/java/org/eclipse/microprofile/openapi/models/PathItem.java b/api/src/main/java/org/eclipse/microprofile/openapi/models/PathItem.java
index c0bde3e..cfd62a0 100644
--- a/api/src/main/java/org/eclipse/microprofile/openapi/models/PathItem.java
+++ b/api/src/main/java/org/eclipse/microprofile/openapi/models/PathItem.java
@@ -266,16 +266,27 @@ public interface PathItem extends Constructible, Extensible, Reference<PathItem>
     /**
      * Returns a list of all the operations for this path item.
      * 
+     * @deprecated since 1.1, use @link {@link Map#values()} on {@link #getOperations()} instead
      * @return a list of all the operations for this path item
      **/
+    @Deprecated
     List<Operation> readOperations();
 
     /**
      * Returns a map with all the operations for this path where the keys are HttpMethods.
      * 
+     * @deprecated since 1.1, use {@link #getOperations()} instead
      * @return a map with all the operations for this path where the keys are HttpMethods
      **/
+    @Deprecated
     Map<PathItem.HttpMethod, Operation> readOperationsMap();
+    
+    /**
+     * Returns a map with all the operations for this path where the keys are {@link PathItem.HttpMethod} items
+     * 
+     * @return a map with all the operations for this path where the keys are HttpMethods
+     **/
+    Map<PathItem.HttpMethod, Operation> getOperations();
 
     /**
      * Returns the servers property from a PathItem instance.


Test patch:
diff --git a/tck/src/main/java/org/eclipse/microprofile/openapi/tck/ModelConstructionTest.java b/tck/src/main/java/org/eclipse/microprofile/openapi/tck/ModelConstructionTest.java
index b723f34..c439c0d 100644
--- a/tck/src/main/java/org/eclipse/microprofile/openapi/tck/ModelConstructionTest.java
+++ b/tck/src/main/java/org/eclipse/microprofile/openapi/tck/ModelConstructionTest.java
@@ -261,6 +261,47 @@ public class ModelConstructionTest extends Arquillian {
         final Server s = createConstructibleInstance(Server.class);
         checkSameObject(pi, pi.addServer(s));
         checkListEntry(pi.getServers(), s);
+        
+        final Operation o1 = createConstructibleInstance(Operation.class);
+        checkSameObject(pi, pi.GET(o1));
+        checkSameObject(o1, pi.getGET());
+        
+        final Operation o2 = createConstructibleInstance(Operation.class);
+        checkSameObject(pi, pi.PUT(o2));
+        checkSameObject(o2, pi.getPUT());
+        
+        final Operation o3 = createConstructibleInstance(Operation.class);
+        checkSameObject(pi, pi.POST(o3));
+        checkSameObject(o3, pi.getPOST());
+        
+        final Operation o4 = createConstructibleInstance(Operation.class);
+        checkSameObject(pi, pi.DELETE(o4));
+        checkSameObject(o4, pi.getDELETE());
+        
+        final Operation o5 = createConstructibleInstance(Operation.class);
+        checkSameObject(pi, pi.OPTIONS(o5));
+        checkSameObject(o5, pi.getOPTIONS());
+        
+        final Operation o6 = createConstructibleInstance(Operation.class);
+        checkSameObject(pi, pi.HEAD(o6));
+        checkSameObject(o6, pi.getHEAD());
+        
+        final Operation o7 = createConstructibleInstance(Operation.class);
+        checkSameObject(pi, pi.PATCH(o7));
+        checkSameObject(o7, pi.getPATCH());
+        
+        final Operation o8 = createConstructibleInstance(Operation.class);
+        checkSameObject(pi, pi.TRACE(o8));
+        checkSameObject(o8, pi.getTRACE());
+        
+        checkMapEntry(pi.getOperations(), PathItem.HttpMethod.GET, o1);
+        checkMapEntry(pi.getOperations(), PathItem.HttpMethod.PUT, o2);
+        checkMapEntry(pi.getOperations(), PathItem.HttpMethod.POST, o3);
+        checkMapEntry(pi.getOperations(), PathItem.HttpMethod.DELETE, o4);
+        checkMapEntry(pi.getOperations(), PathItem.HttpMethod.OPTIONS, o5);
+        checkMapEntry(pi.getOperations(), PathItem.HttpMethod.HEAD, o6);
+        checkMapEntry(pi.getOperations(), PathItem.HttpMethod.PATCH, o7);
+        checkMapEntry(pi.getOperations(), PathItem.HttpMethod.TRACE, o8);
     }
     
     @Test
@@ -814,7 +855,7 @@ public class ModelConstructionTest extends Arquillian {
         return properties;
     }
     
-    private <T> void checkMapEntry(Map<String,T> map, String key, T value) {
+    private <K, T> void checkMapEntry(Map<K,T> map, K key, T value) {
         assertNotNull(map, "The map must not be null.");
         assertTrue(map.containsKey(key), "The map is expected to contain the key: " + key);
         assertSame(map.get(key), value, "The value associated with the key: " + key + " is expected to be the same one that was added.");