AMBARI-9230. Add getUpdateDirectives to org.apache.ambari.server.api.resources.ResourceDefinition and use when handling PUT requests (rlevas)

ambari-server/src/main/java/org/apache/ambari/server/api/predicate/PredicateCompiler.java

@@ -20,7 +20,7 @@ package org.apache.ambari.server.api.predicate;
 
 import org.apache.ambari.server.controller.spi.Predicate;
 
-import java.util.Set;
+import java.util.Collection;
 
 /**
  * Compiler which takes a query expression as input and produces a predicate instance as output.
@@ -53,12 +53,12 @@ public class PredicateCompiler {
    * Generate a predicate from a query expression.
    *
    * @param exp               query expression
-   * @param ignoredProperties  set of property names to ignore
+   * @param ignoredProperties  collection of property names to ignore
    *
    * @return a predicate instance
    * @throws InvalidQueryException if unable to compile the expression
    */
-  public Predicate compile(String exp, Set<String> ignoredProperties) throws InvalidQueryException {
+  public Predicate compile(String exp, Collection<String> ignoredProperties) throws InvalidQueryException {
     return parser.parse(lexer.tokens(exp, ignoredProperties));
   }
 }

ambari-server/src/main/java/org/apache/ambari/server/api/predicate/QueryLexer.java

@@ -125,7 +125,7 @@ public class QueryLexer {
    * @return an array of tokens
    * @throws InvalidQueryException if the query is invalid
    */
-  public Token[] tokens(String exp, Set<String> ignoreProperties) throws InvalidQueryException {
+  public Token[] tokens(String exp, Collection<String> ignoreProperties) throws InvalidQueryException {
     ScanContext ctx = new ScanContext();
     ctx.addPropertiesToIgnore(SET_IGNORE);
     ctx.addPropertiesToIgnore(ignoreProperties);
@@ -349,7 +349,7 @@ public class QueryLexer {
      *
      * @param ignoredProperties set of property names to ignore
      */
-    public void addPropertiesToIgnore(Set<String> ignoredProperties) {
+    public void addPropertiesToIgnore(Collection<String> ignoredProperties) {
       if (ignoredProperties != null) {
         m_propertiesToIgnore.addAll(ignoredProperties);
       }

ambari-server/src/main/java/org/apache/ambari/server/api/resources/BaseResourceDefinition.java

@@ -188,4 +188,12 @@ public abstract class BaseResourceDefinition implements ResourceDefinition {
       resultNode.setProperty("href", href);
     }
   }
+
+  /**
+   * Returns a collection which can be modified by sub resources
+   */
+  @Override
+  public Collection<String> getUpdateDirectives() {
+    return new HashSet<String>();
+  }
 }

ambari-server/src/main/java/org/apache/ambari/server/api/resources/ResourceDefinition.java

@@ -107,4 +107,9 @@ public interface ResourceDefinition {
   public interface PostProcessor {
     public void process(Request request, TreeNode<Resource> resultNode, String href);
   }
+
+  /**
+   * Retrieves directives from the URI
+   */
+  public Collection<String> getUpdateDirectives();
 }

ambari-server/src/main/java/org/apache/ambari/server/api/services/BaseRequest.java

@@ -39,13 +39,13 @@ import javax.ws.rs.core.UriInfo;
 import java.io.UnsupportedEncodingException;
 import java.net.URLDecoder;
 import java.util.ArrayList;
-import java.util.Arrays;
 import java.util.Collections;
 import java.util.HashMap;
 import java.util.HashSet;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import java.util.Collection;
 import java.util.regex.Matcher;
 import java.util.regex.Pattern;
 
@@ -303,15 +303,30 @@ public abstract class BaseRequest implements Request {
   private void parseQueryPredicate() throws InvalidQueryException {
     String queryString = m_body.getQueryString();
     if (queryString == null) {
-      String uri     = getURI();
-      int    qsBegin = uri.indexOf("?");
+      String uri = getURI();
+      int qsBegin = uri.indexOf("?");
 
       queryString = (qsBegin == -1) ? null : uri.substring(qsBegin + 1);
     }
 
     if (queryString != null) {
       try {
-        m_predicate = getPredicateCompiler().compile(URLDecoder.decode(queryString, "UTF-8"));
+        Collection<String> ignoredProperties = null;
+        switch (this.getRequestType()) {
+          case PUT:
+            ignoredProperties = m_resource.getResourceDefinition().getUpdateDirectives();
+            break;
+          case POST:
+            ignoredProperties = m_resource.getResourceDefinition().getCreateDirectives();
+            break;
+          default:
+            break;
+        }
+
+
+        m_predicate = (ignoredProperties == null)
+            ? getPredicateCompiler().compile(URLDecoder.decode(queryString, "UTF-8"))
+            : getPredicateCompiler().compile(URLDecoder.decode(queryString, "UTF-8"), ignoredProperties);
       } catch (UnsupportedEncodingException e) {
         throw new RuntimeException("Unable to decode URI: " + e, e);
       }

ambari-server/src/main/java/org/apache/ambari/server/api/services/RequestFactory.java

@@ -48,7 +48,7 @@ public class RequestFactory {
       case GET:
         return new GetRequest(headers, body, uriInfo, resource);
       case PUT:
-        return new PutRequest(headers, body, uriInfo, resource);
+        return createPutRequest(headers, body, uriInfo, resource);
       case DELETE:
         return new DeleteRequest(headers, body, uriInfo, resource);
       case POST:
@@ -70,27 +70,22 @@ public class RequestFactory {
    * @return new post request
    */
   private Request createPostRequest(HttpHeaders headers, RequestBody body, UriInfo uriInfo, ResourceInstance resource) {
-    boolean batchCreate = false;
-    Map<String, String> queryParameters = getQueryParameters(uriInfo, body);
-    if (! queryParameters.isEmpty()) {
-      ResourceDefinition resourceDefinition = resource.getResourceDefinition();
-      Collection<String> directives = resourceDefinition.getCreateDirectives();
-
-      Map<String, String> requestInfoProperties = body.getRequestInfoProperties();
-      for (Map.Entry<String, String> entry : queryParameters.entrySet()) {
-        if (directives.contains(entry.getKey())) {
-          requestInfoProperties.put(entry.getKey(), entry.getValue());
-        } else {
-          batchCreate = true;
-        }
-      }
-    }
+    boolean batchCreate = !applyDirectives(Request.Type.POST, body, uriInfo, resource);;
 
     return (batchCreate) ?
         new QueryPostRequest(headers, body, uriInfo, resource) :
         new PostRequest(headers, body, uriInfo, resource);
   }
 
+  /**
+   * Creates a PUT request. It will apply any eligible directives supplied in the URI
+   */
+  private Request createPutRequest(HttpHeaders headers, RequestBody body, UriInfo uriInfo, ResourceInstance resource) {
+    applyDirectives(Request.Type.PUT, body, uriInfo, resource);
+    return new PutRequest(headers, body, uriInfo, resource);
+  }
+
+
   /**
    * Gather query parameters from uri and body query string.
    *
@@ -116,4 +111,54 @@ public class RequestFactory {
     return queryParameters;
   }
 
+  /**
+   * Applies directives and determines if a query predicate exists or not.
+   * <p/>
+   * Depending on the request type (POST, PUT, etc...), retrieves the appropriate set of directives:
+   * <ul>
+   * <li><code>POST</code> - {@link org.apache.ambari.server.api.resources.ResourceDefinition#getCreateDirectives()}</li>
+   * <li><code>PUT</code> - {@link org.apache.ambari.server.api.resources.ResourceDefinition#getUpdateDirectives()}</li>
+   * </ul>
+   * <p/>
+   * Note: Only <code>POST</code> and <code>PUT</code> are supported.
+   * <p/>
+   * Iterates through the query parameters adding those that are known to be directives to the map
+   * of request info properties from {@link RequestBody#getRequestInfoProperties()}.
+   * <p/>
+   * If a query property is found that is not a directive, a query predicate exists and
+   * <code>false</code> is returned; else all query parameters are directives, leaving no query predicate,
+   * and <code>true</code> is returned.
+   *
+   * @return true if all the directives are supported; false if at least one directive is not supported
+   */
+  private boolean applyDirectives(Request.Type requestType, RequestBody body, UriInfo uriInfo, ResourceInstance resource) {
+    Map<String, String> queryParameters = getQueryParameters(uriInfo, body);
+    Map<String, String> requestInfoProperties;
+    boolean allDirectivesApplicable = true;
+    if (!queryParameters.isEmpty()) {
+      ResourceDefinition resourceDefinition = resource.getResourceDefinition();
+      Collection<String> directives;
+      switch (requestType) {
+        case PUT:
+          directives = resourceDefinition.getUpdateDirectives();
+          break;
+        case POST:
+          directives = resourceDefinition.getCreateDirectives();
+          break;
+        default:
+          // not yet implemented for other types
+          return false;
+      }
+      requestInfoProperties = body.getRequestInfoProperties();
+      for (Map.Entry<String, String> entry : queryParameters.entrySet()) {
+        if (directives.contains(entry.getKey())) {
+          requestInfoProperties.put(entry.getKey(), entry.getValue());
+        }
+        else {
+          allDirectivesApplicable = false;
+        }
+      }
+    }
+    return allDirectivesApplicable;
+  }
 }

ambari-server/src/test/java/org/apache/ambari/server/api/services/BaseRequestTest.java

@@ -124,6 +124,66 @@ public abstract class BaseRequestTest {
     assertNull(request.getQueryPredicate());
   }
 
+  @Test
+  public void testProcess_withDirectives() throws Exception {
+    HttpHeaders headers = createNiceMock(HttpHeaders.class);
+    String path = URLEncoder.encode("http://localhost.com:8080/api/v1/clusters/c1", "UTF-8");
+    String query = URLEncoder.encode("foo=foo-value&bar=bar-value", "UTF-8");
+    URI uri = new URI(path + "?" + query);
+    PredicateCompiler compiler = createStrictMock(PredicateCompiler.class);
+    Predicate predicate = createNiceMock(Predicate.class);
+    UriInfo uriInfo = createMock(UriInfo.class);
+    @SuppressWarnings("unchecked")
+    MultivaluedMap<String, String> queryParams = createMock(MultivaluedMap.class);
+    RequestHandler handler = createStrictMock(RequestHandler.class);
+    Result result = createMock(Result.class);
+    ResultStatus resultStatus = createMock(ResultStatus.class);
+    ResultPostProcessor processor = createStrictMock(ResultPostProcessor.class);
+    RequestBody body = createNiceMock(RequestBody.class);
+    ResourceInstance resource = createNiceMock(ResourceInstance.class);
+    ResourceDefinition resourceDefinition = createNiceMock(ResourceDefinition.class);
+    Set<String> directives = new HashSet<String>();
+    directives.add("my_directive");
+    Renderer renderer = new DefaultRenderer();
+
+    Request request = getTestRequest(headers, body, uriInfo, compiler, handler, processor, resource);
+
+    //expectations
+    expect(uriInfo.getQueryParameters()).andReturn(queryParams).anyTimes();
+    expect(queryParams.getFirst(QueryLexer.QUERY_MINIMAL)).andReturn(null);
+    expect(queryParams.getFirst(QueryLexer.QUERY_FORMAT)).andReturn(null);
+    expect(resource.getResourceDefinition()).andReturn(resourceDefinition).anyTimes();
+    expect(resourceDefinition.getUpdateDirectives()).andReturn(directives).anyTimes(); // for PUT implementation
+    expect(resourceDefinition.getCreateDirectives()).andReturn(directives).anyTimes(); // for POST implementation
+    expect(resourceDefinition.getRenderer(null)).andReturn(renderer);
+    expect(uriInfo.getRequestUri()).andReturn(uri).anyTimes();
+    expect(body.getQueryString()).andReturn(null);
+    if (request.getRequestType().equals(Request.Type.POST) || request.getRequestType().equals(Request.Type.PUT))
+    {
+      expect(compiler.compile("foo=foo-value&bar=bar-value", directives)).andReturn(predicate);
+    }
+    else
+    {
+      expect(compiler.compile("foo=foo-value&bar=bar-value")).andReturn(predicate); // default case
+    }
+    expect(handler.handleRequest(request)).andReturn(result);
+    expect(result.getStatus()).andReturn(resultStatus).anyTimes();
+    expect(resultStatus.isErrorState()).andReturn(false).anyTimes();
+
+    processor.process(result);
+
+    replay(headers, compiler, uriInfo, handler, queryParams, resource,
+      resourceDefinition, result, resultStatus, processor, predicate, body);
+
+    Result processResult = request.process();
+
+    verify(headers, compiler, uriInfo, handler, queryParams, resource,
+      resourceDefinition, result, resultStatus, processor, predicate, body);
+
+    assertSame(processResult, result);
+    assertSame(predicate, request.getQueryPredicate());
+  }
+
   @Test
   public void testProcess_WithBody() throws Exception {
     String uriString = "http://localhost.com:8080/api/v1/clusters/c1";
@@ -194,7 +254,7 @@ public abstract class BaseRequestTest {
     expect(uriInfo.getQueryParameters()).andReturn(queryParams).anyTimes();
     expect(queryParams.getFirst(QueryLexer.QUERY_MINIMAL)).andReturn(null);
     expect(queryParams.getFirst(QueryLexer.QUERY_FORMAT)).andReturn(null);
-    expect(resource.getResourceDefinition()).andReturn(resourceDefinition);
+    expect(resource.getResourceDefinition()).andReturn(resourceDefinition).anyTimes();
     expect(resourceDefinition.getRenderer(null)).andReturn(renderer);
     expect(uriInfo.getRequestUri()).andReturn(uri).anyTimes();
     expect(body.getQueryString()).andReturn(null);
@@ -241,7 +301,7 @@ public abstract class BaseRequestTest {
     expect(uriInfo.getQueryParameters()).andReturn(queryParams).anyTimes();
     expect(queryParams.getFirst(QueryLexer.QUERY_MINIMAL)).andReturn(null);
     expect(queryParams.getFirst(QueryLexer.QUERY_FORMAT)).andReturn(null);
-    expect(resource.getResourceDefinition()).andReturn(resourceDefinition);
+    expect(resource.getResourceDefinition()).andReturn(resourceDefinition).anyTimes();
     expect(resourceDefinition.getRenderer(null)).andReturn(renderer);
     expect(uriInfo.getRequestUri()).andReturn(uri).anyTimes();
     expect(body.getQueryString()).andReturn("foo=bar");
@@ -288,7 +348,7 @@ public abstract class BaseRequestTest {
     expect(uriInfo.getQueryParameters()).andReturn(queryParams).anyTimes();
     expect(queryParams.getFirst(QueryLexer.QUERY_MINIMAL)).andReturn(null);
     expect(queryParams.getFirst(QueryLexer.QUERY_FORMAT)).andReturn(null);
-    expect(resource.getResourceDefinition()).andReturn(resourceDefinition);
+    expect(resource.getResourceDefinition()).andReturn(resourceDefinition).anyTimes();
     expect(resourceDefinition.getRenderer(null)).andReturn(renderer);
     expect(uriInfo.getRequestUri()).andReturn(uri).anyTimes();
     expect(body.getQueryString()).andReturn("foo=bar");
@@ -330,7 +390,7 @@ public abstract class BaseRequestTest {
     expect(uriInfo.getQueryParameters()).andReturn(queryParams).anyTimes();
     expect(queryParams.getFirst(QueryLexer.QUERY_MINIMAL)).andReturn(null);
     expect(queryParams.getFirst(QueryLexer.QUERY_FORMAT)).andReturn(null);
-    expect(resource.getResourceDefinition()).andReturn(resourceDefinition);
+    expect(resource.getResourceDefinition()).andReturn(resourceDefinition).anyTimes();
     expect(resourceDefinition.getRenderer(null)).andReturn(renderer);
     expect(uriInfo.getRequestUri()).andReturn(uri).anyTimes();
     expect(body.getQueryString()).andReturn("blahblahblah");
@@ -372,7 +432,7 @@ public abstract class BaseRequestTest {
     expect(uriInfo.getQueryParameters()).andReturn(queryParams).anyTimes();
     expect(queryParams.getFirst(QueryLexer.QUERY_MINIMAL)).andReturn(null);
     expect(queryParams.getFirst(QueryLexer.QUERY_FORMAT)).andReturn(null);
-    expect(resource.getResourceDefinition()).andReturn(resourceDefinition);
+    expect(resource.getResourceDefinition()).andReturn(resourceDefinition).anyTimes();
     expect(resourceDefinition.getRenderer(null)).andReturn(renderer);
     expect(uriInfo.getRequestUri()).andReturn(uri).anyTimes();
     expect(handler.handleRequest(request)).andReturn(result);
@@ -463,7 +523,7 @@ public abstract class BaseRequestTest {
     //expectations
     expect(uriInfo.getQueryParameters()).andReturn(queryParams).anyTimes();
     expect(queryParams.getFirst(QueryLexer.QUERY_MINIMAL)).andReturn("true");
-    expect(resource.getResourceDefinition()).andReturn(resourceDefinition);
+    expect(resource.getResourceDefinition()).andReturn(resourceDefinition).anyTimes();
     expect(resourceDefinition.getRenderer("minimal")).andReturn(renderer);
     expect(uriInfo.getRequestUri()).andReturn(uri).anyTimes();
     expect(handler.handleRequest(request)).andReturn(result);
@@ -505,7 +565,7 @@ public abstract class BaseRequestTest {
     expect(uriInfo.getQueryParameters()).andReturn(queryParams).anyTimes();
     expect(queryParams.getFirst(QueryLexer.QUERY_MINIMAL)).andReturn(null);
     expect(queryParams.getFirst(QueryLexer.QUERY_FORMAT)).andReturn("default");
-    expect(resource.getResourceDefinition()).andReturn(resourceDefinition);
+    expect(resource.getResourceDefinition()).andReturn(resourceDefinition).anyTimes();
     expect(resourceDefinition.getRenderer("default")).andReturn(renderer);
     expect(uriInfo.getRequestUri()).andReturn(uri).anyTimes();
     expect(handler.handleRequest(request)).andReturn(result);

ambari-server/src/test/java/org/apache/ambari/server/api/services/RequestFactoryTest.java

@@ -143,6 +143,44 @@ public class RequestFactoryTest {
     verify(headers, uriInfo, body, resource, mapQueryParams, resourceDefinition);
   }
 
+  @Test
+  // put with update directive in URI
+  public void testCreate_Put__WithUriDirective() {
+    HttpHeaders headers = createNiceMock(HttpHeaders.class);
+    UriInfo uriInfo = createNiceMock(UriInfo.class);
+    RequestBody body = createNiceMock(RequestBody.class);
+    ResourceInstance resource = createNiceMock(ResourceInstance.class);
+    ResourceDefinition resourceDefinition = createNiceMock(ResourceDefinition.class);
+
+    @SuppressWarnings("unchecked")
+    MultivaluedMap<String, String> mapQueryParams = createMock(MultivaluedMap.class);
+    Map<String, List<String>> mapProps = new HashMap<String, List<String>>();
+    mapProps.put("foo", Collections.singletonList("bar"));
+
+    Map<String, String> requestInfoMap = new HashMap<String, String>();
+
+    //expectations
+    expect(uriInfo.getQueryParameters()).andReturn(mapQueryParams).anyTimes();
+    expect(mapQueryParams.entrySet()).andReturn(mapProps.entrySet()).anyTimes();
+    expect(resource.getResourceDefinition()).andReturn(resourceDefinition).anyTimes();
+    expect(resourceDefinition.getUpdateDirectives()).andReturn(Collections.singleton("foo"));
+    expect(body.getQueryString()).andReturn(null);
+    expect(body.getRequestInfoProperties()).andReturn(requestInfoMap).anyTimes();
+
+    replay(headers, uriInfo, body, resource, mapQueryParams, resourceDefinition);
+
+    //test
+    RequestFactory factory = new RequestFactory();
+    Request request = factory.createRequest(headers, body, uriInfo, Request.Type.PUT, resource);
+
+    assertEquals(resource, request.getResource());
+    assertEquals(body, request.getBody());
+    assertEquals(Request.Type.PUT, request.getRequestType());
+    assertEquals("bar", requestInfoMap.get("foo"));
+
+    verify(headers, uriInfo, body, resource, mapQueryParams, resourceDefinition);
+  }
+
   @Test
   // query post : body contains query string
   public void testCreate_Post__BodyQueryParams() {