SLIDER-50 change URL for published key-val pairs to .filetype, not /filetype

git-svn-id: https://svn.apache.org/repos/asf/incubator/slider/trunk@1594532 13f79535-47bb-0310-9956-ffa450edef68
diff --git a/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfigSet.java b/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfigSet.java
index 685ec9d..7db56f4 100644
--- a/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfigSet.java
+++ b/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfigSet.java
@@ -18,10 +18,12 @@
 
 package org.apache.slider.core.registry.docstore;
 
+import org.apache.slider.server.appmaster.web.rest.RestPaths;
 import org.codehaus.jackson.annotate.JsonIgnoreProperties;
 import org.codehaus.jackson.map.annotate.JsonSerialize;
 
 import java.util.HashMap;
+import java.util.Locale;
 import java.util.Map;
 import java.util.Set;
 import java.util.TreeSet;
@@ -35,7 +37,7 @@
 @JsonSerialize(include = JsonSerialize.Inclusion.NON_NULL)
 public class PublishedConfigSet {
 
-  public static final String VALID_NAME_PATTERN = "[A-Za-z0-9_-]+";
+  public static final String VALID_NAME_PATTERN = RestPaths.PUBLISHED_CONFIGURATION_REGEXP;
   public static final String E_INVALID_NAME =
       "Invalid configuration name -it must match the pattern " +
       VALID_NAME_PATTERN;
@@ -44,16 +46,25 @@
   public Map<String, PublishedConfiguration> configurations =
       new HashMap<>();
 
+  /**
+   * Put a name -it will be converted to lower case before insertion.
+   * Any existing entry will be overwritten (that includes an entry
+   * with a different case in the original name)
+   * @param name name of entry
+   * @param conf configuration
+   * @throws IllegalArgumentException if not a valid name
+   */
   public void put(String name, PublishedConfiguration conf) {
-    validateName(name);
-    configurations.put(name, conf);
+    String name1 = name.toLowerCase(Locale.ENGLISH);
+    validateName(name1);
+    configurations.put(name1, conf);
   }
 
   /**
    * Validate the name -restricting it to the set defined in 
    * {@link #VALID_NAME_PATTERN}
    * @param name name to validate
-   * @throws IllegalArgumentException if not
+   * @throws IllegalArgumentException if not a valid name
    */
   public static void validateName(String name) {
     if (!validNames.matcher(name).matches()) {
diff --git a/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfiguration.java b/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfiguration.java
index df32430..26bb3ef 100644
--- a/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfiguration.java
+++ b/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfiguration.java
@@ -158,4 +158,8 @@
     sb.append('}');
     return sb.toString();
   }
+  
+  public PublishedConfigurationOutputter creatOutputter(ConfigFormat format) {
+    return PublishedConfigurationOutputter.createOutputter(format, this);
+  }
 }
diff --git a/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfigurationOutputter.java b/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfigurationOutputter.java
index f4ffe34..929b8ef 100644
--- a/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfigurationOutputter.java
+++ b/slider-core/src/main/java/org/apache/slider/core/registry/docstore/PublishedConfigurationOutputter.java
@@ -21,14 +21,20 @@
 import com.google.common.base.Charsets;
 import com.google.common.base.Preconditions;
 import org.apache.commons.io.FileUtils;
+import org.apache.commons.io.IOUtils;
 import org.apache.hadoop.conf.Configuration;
 import org.apache.slider.common.tools.ConfigHelper;
 
 import java.io.File;
 import java.io.FileOutputStream;
 import java.io.IOException;
+import java.io.OutputStream;
+import java.io.StringWriter;
 import java.util.Properties;
 
+/**
+ * Output a published configuration
+ */
 public abstract class PublishedConfigurationOutputter {
 
   protected final PublishedConfiguration owner;
@@ -37,14 +43,41 @@
     this.owner = owner;
   }
 
+  /**
+   * Save the config to a destination file, in the format of this outputter
+   * @param dest destination file
+   * @throws IOException
+   */
   public void save(File dest) throws IOException {
-    
+    try(FileOutputStream out = new FileOutputStream(dest)) {
+      save(out);
+      out.close();
+    }
   }
 
-  public String asString() throws IOException {
-    return "";
+  /**
+   * Save the content. The default saves the asString() value
+   * to the output stream
+   * @param out output stream
+   * @throws IOException
+   */
+  public void save(OutputStream out) throws IOException {
+    IOUtils.write(asString(), out, Charsets.UTF_8);
   }
+  /**
+   * Convert to a string
+   * @return
+   * @throws IOException
+   */
+  public abstract String asString() throws IOException;
 
+  /**
+   * Create an outputter for the chosen format
+   * @param format format enumeration
+   * @param owner owning config
+   * @return the outputter
+   */
+  
   public static PublishedConfigurationOutputter createOutputter(ConfigFormat format,
       PublishedConfiguration owner) {
     Preconditions.checkNotNull(owner);
@@ -71,13 +104,8 @@
     }
 
     @Override
-    public void save(File dest) throws IOException {
-      FileOutputStream out = new FileOutputStream(dest);
-      try {
-        configuration.writeXml(out);
-      } finally {
-        out.close();
-      }
+    public void save(OutputStream out) throws IOException {
+      configuration.writeXml(out);
     }
 
     @Override
@@ -100,19 +128,15 @@
     }
 
     @Override
-    public void save(File dest) throws IOException {
-      FileOutputStream out = new FileOutputStream(dest);
-      try {
-        properties.store(out, "");
-      } finally {
-        out.close();
-      }
-
+    public void save(OutputStream out) throws IOException {
+      properties.store(out, "");
     }
 
-    @Override
+    
     public String asString() throws IOException {
-      return properties.toString();
+      StringWriter sw = new StringWriter();
+      properties.store(sw, "");
+      return sw.toString();
     }
   }
     
diff --git a/slider-core/src/main/java/org/apache/slider/core/registry/retrieve/RegistryRetriever.java b/slider-core/src/main/java/org/apache/slider/core/registry/retrieve/RegistryRetriever.java
index e4cf428..257f8d1 100644
--- a/slider-core/src/main/java/org/apache/slider/core/registry/retrieve/RegistryRetriever.java
+++ b/slider-core/src/main/java/org/apache/slider/core/registry/retrieve/RegistryRetriever.java
@@ -122,10 +122,10 @@
 
   /**
    * Get a complete configuration, with all values
-   * @param name
-   * @param external
-   * @return
-   * @throws IOException
+   * @param name name of the configuration
+   * @param external flag to indicate that it is an external configuration
+   * @return the retrieved config
+   * @throws IOException IO problems
    */
   public PublishedConfiguration retrieveConfiguration(String name,
       boolean external) throws IOException {
diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/web/SliderAmIpFilter.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/web/SliderAmIpFilter.java
index 3893699..1888891 100644
--- a/slider-core/src/main/java/org/apache/slider/server/appmaster/web/SliderAmIpFilter.java
+++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/web/SliderAmIpFilter.java
@@ -18,11 +18,11 @@
 
 package org.apache.slider.server.appmaster.web;
 
-import org.apache.commons.logging.Log;
-import org.apache.commons.logging.LogFactory;
 import org.apache.hadoop.yarn.server.webproxy.WebAppProxyServlet;
 import org.apache.hadoop.yarn.server.webproxy.amfilter.AmIpPrincipal;
 import org.apache.hadoop.yarn.server.webproxy.amfilter.AmIpServletRequestWrapper;
+import org.slf4j.Logger;
+import org.slf4j.LoggerFactory;
 
 import javax.servlet.Filter;
 import javax.servlet.FilterChain;
@@ -40,7 +40,8 @@
 import java.util.Set;
 
 public class SliderAmIpFilter implements Filter {
-  private static final Log LOG = LogFactory.getLog(SliderAmIpFilter.class);
+  protected static final Logger log =
+      LoggerFactory.getLogger(SliderAmIpFilter.class);
   
   public static final String PROXY_HOST = "PROXY_HOST";
   public static final String PROXY_URI_BASE = "PROXY_URI_BASE";
@@ -68,8 +69,8 @@
         try {
           proxyAddresses = new HashSet<String>();
           for(InetAddress add : InetAddress.getAllByName(proxyHost)) {
-            if (LOG.isDebugEnabled()) {
-              LOG.debug("proxy address is: " + add.getHostAddress());
+            if (log.isDebugEnabled()) {
+              log.debug("proxy address is: " + add.getHostAddress());
             }
             proxyAddresses.add(add.getHostAddress());
           }
@@ -96,13 +97,14 @@
     
     HttpServletRequest httpReq = (HttpServletRequest)req;
     HttpServletResponse httpResp = (HttpServletResponse)resp;
-    if (LOG.isDebugEnabled()) {
-      LOG.debug("Remote address for request is: " + httpReq.getRemoteAddr());
+    if (log.isDebugEnabled()) {
+      log.debug("Remote address for request is: " + httpReq.getRemoteAddr());
     }
-    if(!httpReq.getRequestURI().startsWith(wsContextRoot) &&
+    String requestURI = httpReq.getRequestURI();
+    if(!requestURI.startsWith(wsContextRoot) &&
        !getProxyAddresses().contains(httpReq.getRemoteAddr())) {
-      String redirectUrl = httpResp.encodeRedirectURL(proxyUriBase + 
-          httpReq.getRequestURI());
+      String redirectUrl = httpResp.encodeRedirectURL(proxyUriBase +
+                                                      requestURI);
       httpResp.sendRedirect(redirectUrl);
       return;
     }
@@ -117,15 +119,20 @@
         }
       }
     }
-    if(user == null) {
-      LOG.warn("Could not find "+WebAppProxyServlet.PROXY_USER_COOKIE_NAME
-          +" cookie, so user will not be set");
-      chain.doFilter(req, resp);
-    } else {
-      final AmIpPrincipal principal = new AmIpPrincipal(user);
-      ServletRequest requestWrapper = new AmIpServletRequestWrapper(httpReq,
-          principal);
-      chain.doFilter(requestWrapper, resp);
+    try {
+      if (user == null) {
+        log.warn("Could not find " + WebAppProxyServlet.PROXY_USER_COOKIE_NAME
+                 + " cookie, so user will not be set");
+        chain.doFilter(req, resp);
+      } else {
+        final AmIpPrincipal principal = new AmIpPrincipal(user);
+        ServletRequest requestWrapper = new AmIpServletRequestWrapper(httpReq,
+            principal);
+        chain.doFilter(requestWrapper, resp);
+      }
+    } catch (IOException | ServletException e) {
+      log.warn("When fetching {}: {}", requestURI, e);
+      throw e;
     }
   }
 }
diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/web/rest/RestPaths.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/web/rest/RestPaths.java
index 6d02e60..e2ca42f 100644
--- a/slider-core/src/main/java/org/apache/slider/server/appmaster/web/rest/RestPaths.java
+++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/web/rest/RestPaths.java
@@ -44,4 +44,13 @@
 
   public static final String REGISTRY_SERVICE = "v1/service";
   public static final String REGISTRY_ANYSERVICE = "v1/anyservice";
+
+  /**
+   * The regular expressions used to define valid configuration names/url path
+   * fragments: {@value}
+   */
+  public static final String PUBLISHED_CONFIGURATION_REGEXP
+      ="[a-z0-9][a-z0-9_\\+-]*";
+  
+  
 }
diff --git a/slider-core/src/main/java/org/apache/slider/server/appmaster/web/rest/publisher/PublisherResource.java b/slider-core/src/main/java/org/apache/slider/server/appmaster/web/rest/publisher/PublisherResource.java
index d18a08d..7722eea 100644
--- a/slider-core/src/main/java/org/apache/slider/server/appmaster/web/rest/publisher/PublisherResource.java
+++ b/slider-core/src/main/java/org/apache/slider/server/appmaster/web/rest/publisher/PublisherResource.java
@@ -19,8 +19,10 @@
 package org.apache.slider.server.appmaster.web.rest.publisher;
 
 import org.apache.hadoop.yarn.webapp.NotFoundException;
+import org.apache.slider.core.registry.docstore.ConfigFormat;
 import org.apache.slider.core.registry.docstore.PublishedConfigSet;
 import org.apache.slider.core.registry.docstore.PublishedConfiguration;
+import org.apache.slider.core.registry.docstore.PublishedConfigurationOutputter;
 import org.apache.slider.server.appmaster.web.WebAppApi;
 import org.slf4j.Logger;
 import org.slf4j.LoggerFactory;
@@ -34,6 +36,7 @@
 import javax.ws.rs.core.MediaType;
 import javax.ws.rs.core.UriInfo;
 import java.io.IOException;
+import static  org.apache.slider.server.appmaster.web.rest.RestPaths.*;
 
 /**
  * This publishes configuration sets
@@ -42,7 +45,9 @@
   protected static final Logger log =
       LoggerFactory.getLogger(PublisherResource.class);
   private final WebAppApi slider;
-
+  private static final String CONFIG =
+      "{config: " + PUBLISHED_CONFIGURATION_REGEXP + "}";
+  
   public PublisherResource(WebAppApi slider) {
     this.slider = slider;
   }
@@ -90,36 +95,50 @@
     return publishedConfig;
   }
   
-  
   @GET
-  @Path("/{config}/json")
+  @Path("/{config}.json")
   @Produces({MediaType.APPLICATION_JSON})
   public String getConfigurationContentJson(
       @PathParam("config") String config,
       @Context UriInfo uriInfo,
       @Context HttpServletResponse res) throws IOException {
-    
-    // delegate (including init)
-    PublishedConfiguration publishedConfig =
-        getConfigurationInstance(config, uriInfo, res);
-    return publishedConfig.asJson();
+    return getStringRepresentation(config, uriInfo, res,
+        ConfigFormat.JSON);
   }
 
-  
   @GET
-  @Path("/{config}/xml")
+  @Path("/{config}.xml")
   @Produces({MediaType.APPLICATION_XML})
   public String getConfigurationContentXML(
       @PathParam("config") String config,
       @Context UriInfo uriInfo,
       @Context HttpServletResponse res) throws IOException {
-    
+    return getStringRepresentation(config, uriInfo, res,
+        ConfigFormat.XML);
+  }
+  
+  @GET
+  @Path("/{config}.properties")
+  @Produces({MediaType.APPLICATION_XML})
+  public String getConfigurationContentProperties(
+      @PathParam("config") String config,
+      @Context UriInfo uriInfo,
+      @Context HttpServletResponse res) throws IOException {
+
+    return getStringRepresentation(config, uriInfo, res,
+        ConfigFormat.PROPERTIES);
+  }
+
+  public String getStringRepresentation(String config,
+      UriInfo uriInfo,
+      HttpServletResponse res, ConfigFormat format) throws IOException {
     // delegate (including init)
     PublishedConfiguration publishedConfig =
         getConfigurationInstance(config, uriInfo, res);
-    return publishedConfig.asConfigurationXML();
+    PublishedConfigurationOutputter outputter =
+        publishedConfig.creatOutputter(format);
+    return outputter.asString();
   }
 
-  
-  
+
 }
diff --git a/slider-core/src/test/groovy/org/apache/slider/agent/standalone/TestStandaloneRegistryAM.groovy b/slider-core/src/test/groovy/org/apache/slider/agent/standalone/TestStandaloneRegistryAM.groovy
index 7cbdaea..cd82aa0 100644
--- a/slider-core/src/test/groovy/org/apache/slider/agent/standalone/TestStandaloneRegistryAM.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/agent/standalone/TestStandaloneRegistryAM.groovy
@@ -172,12 +172,23 @@
 
 
     //get the XML
-    def yarnSiteXML = appendToURL(yarnSitePublisher, "xml")
+    def yarnSiteXML = yarnSitePublisher + ".xml"
 
 
     String confXML = GET(yarnSiteXML)
     log.info("Conf XML at $yarnSiteXML = \n $confXML")
 
+    String properties = GET(yarnSitePublisher + ".properties")
+    Properties parsedProps = new Properties()
+    parsedProps.load(new StringReader(properties))
+    assert parsedProps.size() > 0
+    def rmAddrFromDownloadedProperties = parsedProps.get(YarnConfiguration.RM_ADDRESS)
+    def rmHostnameFromDownloadedProperties = parsedProps.get(YarnConfiguration.RM_HOSTNAME)
+    assert rmAddrFromDownloadedProperties
+    assert rmHostnameFromDownloadedProperties
+
+    String json = GET(yarnSitePublisher + ".json")
+
 
 
     describe("Registry List")
@@ -202,10 +213,14 @@
     def yarnSite = retriever.retrieveConfiguration(ARTIFACT_NAME, true)
     assert !yarnSite.empty
     def siteXML = yarnSite.asConfiguration()
-    def rmAddr = siteXML.get(YarnConfiguration.RM_ADDRESS)
-    assert rmAddr
-    
-    
+    def rmHostnameViaClientSideXML = parsedProps.get(
+        YarnConfiguration.RM_HOSTNAME)
+    assert rmHostnameViaClientSideXML == rmHostnameFromDownloadedProperties
+    def rmAddrViaClientSideXML = siteXML.get(YarnConfiguration.RM_ADDRESS)
+
+  /* TODO SLIDER-52 PublishedConfiguration XML conf values are not resolved until client-side
+   assert rmAddrViaClientSideXML == rmAddrFromDownloadedProperties
+  */  
     describe "Internal configurations"
     assert !retriever.hasConfigurations(false)
     try {
@@ -233,9 +248,6 @@
     
     assert 0 == client.actionRegistry(registryArgs)
 
-  
-    
-    
     // listconf --internal
     registryArgs.list = false;
     registryArgs.listConf = true
diff --git a/slider-core/src/test/groovy/org/apache/slider/registry/TestConfigSetNaming.groovy b/slider-core/src/test/groovy/org/apache/slider/registry/TestConfigSetNaming.groovy
index b04ffe1..7cbe4c3 100644
--- a/slider-core/src/test/groovy/org/apache/slider/registry/TestConfigSetNaming.groovy
+++ b/slider-core/src/test/groovy/org/apache/slider/registry/TestConfigSetNaming.groovy
@@ -29,6 +29,7 @@
   def assertValid(String name) {
     PublishedConfigSet.validateName(name)
   }
+  
   def assertInvalid(String name) {
     try {
       PublishedConfigSet.validateName(name)
@@ -44,8 +45,8 @@
   }
   
   @Test
-  public void testUpperCase() throws Throwable {
-    assertValid("ABCDEFGHIJKLMNOPQRSTUVWXYZ")
+  public void testUpperCaseInvalid() throws Throwable {
+    assertInvalid("ABCDEFGHIJKLMNOPQRSTUVWXYZ")
   }
   
   @Test
@@ -55,7 +56,7 @@
   
   @Test
   public void testChars() throws Throwable {
-    assertValid("-_")
+    assertValid("a-_+")
   }
 
   @Test
@@ -70,7 +71,11 @@
         "'",
         "\u0000",
         "\u0f00",
-        ""
+        "key.value",
+        "-",
+        "+",
+        "_",
+        "?",
     ].each {String s -> assertInvalid(s)}
   }
 }