GitHub PullRequestReviewCommentEvent at 2014-05-28 18:30:43
Repository: geoserver
Actor: smithkm
Payload: {"comment":{"url":"https://api.github.com/repos/geoserver/geoserver/pulls/comments/13147543","id":13147543,"diff_hunk":"@@ -815,256 +719,612 @@ File styleDir(boolean create, StyleInfo styleInfo) throws IOException {\n      * @param workspaceInfo Workspace used to access styles directory\n      * @return styles directory\n      * @throws IOException\n-     */\n-    File styleDir(boolean create, WorkspaceInfo workspaceInfo) throws IOException {\n-        Resource styles = get(pathStyles(workspaceInfo));\n-        if (create) {\n-            return styles.dir();\n-        } else {\n-            return Resources.directory(styles);\n-        }\n-    }\n-\n-    /**\n-     * Style directory for the provided workspace (or global styles directory if workspace not provided).\n-     * \n-     * @param workspace\n-     * @return Path to styles directory\n-     */\n-    private String pathStyles(StyleInfo style) {\n-        WorkspaceInfo workspace = style != null ? style.getWorkspace() : null;\n-        if (workspace == null) {\n-            return \"styles\";\n-        } else {\n-            return Paths.path(\"workspaces\", workspace.getName(), \"styles\");\n-        }\n-    }\n-\n-    /**\n-     * Style directory for the provided workspace (or global styles directory if workspace not provided).\n      * \n-     * @param workspace\n-     * @return Path to styles directory\n+     * @deprecated As of GeoServer 2.6, replaced by {@link #get(StyleInfo, String...)}\n      */\n-    private String pathStyles(WorkspaceInfo workspace) {\n-        if (workspace == null) {\n-            return \"styles\";\n-        } else {\n-            return Paths.path(\"workspaces\", workspace.getName(), \"styles\");\n-        }\n+    File styleDir(boolean create, WorkspaceInfo workspaceInfo) throws IOException {\n+        Resource styles = get(workspaceInfo, STYLE_DIR);\n+        return Resources.directory(styles, create);\n     }\n \n     /**\n      * Returns the configuration file for the specified style, if the file does not exist null is returned.\n+     * \n+     * @deprecated As of GeoServer 2.6, replaced by {@link #config(StyleInfo, String...)}\n      */\n     public File findStyleFile(StyleInfo s) throws IOException {\n-        Resource resource = get(pathStyleFile(s));\n+        Resource resource = config(s);\n         return Resources.file(resource);\n     }\n \n     /**\n      * Returns the SLD file for the specified style, if the file does not exist null is returned.\n+     * \n+     * @deprecated As of GeoServer 2.6, replaced by {@link #style(StyleInfo, String...)}\n      */\n     public File findStyleSldFile(StyleInfo s) throws IOException {\n-        Resource resource = get(pathSldFile(s));\n+        Resource resource = style(s);\n         return Resources.file(resource);\n     }\n \n     /**\n      * Returns the configuration file for the specified style, if the file does not exist a file object is still returned.\n+     * \n+     * @deprecated As of GeoServer 2.6, replaced by {@link #config(StyleInfo, String...)}\n      */\n     public File findOrCreateStyleFile(StyleInfo s) throws IOException {\n-        Resource resource = get(pathStyleFile(s));\n+        Resource resource = config(s);\n         return resource.file();\n     }\n \n     /**\n      * Returns the SLD file for the specified style, if the file does not exist a file object is still returned.\n+     * \n+     * @deprecated As of GeoServer 2.6, replaced by {@link #style(StyleInfo, String...)}\n      */\n     public File findOrCreateStyleSldFile(StyleInfo s) throws IOException {\n-        Resource resource = get(pathSldFile(s));\n+        Resource resource = style(s);\n         return resource.file();\n     }\n+    \n+    // Resource lookup methods\n+    static final String WORKSPACE_XML = \"workspace.xml\";\n+    static final String NAMESPACE_XML = \"namespace.xml\";\n+    static final String DATASTORE_XML = \"datastore.xml\";\n+    static final String COVERAGESTORE_XML = \"coveragestore.xml\";\n+    static final String WMSSTORE_XML = \"wmsstore.xml\";\n+    static final String FEATURETYPE_XML = \"featuretype.xml\";\n+    static final String COVERAGE_XML = \"coverage.xml\";\n+    static final String WMSLAYER_XML = \"wmslayer.xml\";\n+    static final String LAYER_XML = \"layer.xml\";\n+    static final String WORKSPACE_DIR = \"workspaces\";\n+    static final String LAYERGROUP_DIR = \"layergroups\";\n+    static final String STYLE_DIR = \"styles\";\n \n-    @Deprecated\n-    private File styleFile(boolean create, StyleInfo s) throws IOException {\n-        Resource resource = get(pathStyleFile(s));\n-        if (create) {\n-            return resource.file();\n+    \n+    /**\n+     * Retrieve a resource relative tot he root of the data directory. An empty path will retrieve\n+     * the directory itself.\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource getRoot(String... path) {\n+        Resource r = get(Paths.path(path));\n+        assert r!=null;\n+        return r;\n+    }\n+\n+    /**\n+     * Retrieve a resource in the the workspaces directory. An empty path will retrieve\n+     * the directory itself.\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource getWorkspaces(String... path) {\n+        Resource r = get(  Paths.path( WORKSPACE_DIR, Paths.path(path)));\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the configuration xml for the default workspace.\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource defaultWorkspaceConfig() {\n+        Resource r = getRoot(\"default.xml\");\n+        assert r!=null;\n+        return r;\n+    }\n+\n+    /**\n+     * Retrieve a resource in the the workspace configuration directory. An empty path will retrieve\n+     * the directory itself.\n+     * @param ws The workspace\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource get(WorkspaceInfo ws, String... path) {\n+        Resource r;\n+        if(ws==null) {\n+            return resourceLoader.get(\"\");\n         } else {\n-            return Resources.file(resource);\n+            r = getWorkspaces(  ws.getName(), Paths.path(path));\n         }\n+        assert r!=null;\n+        return r;\n     }\n-\n+    \n     /**\n-     * Path to generated style file, style directory used based on {@link StyleInfo#getWorkspace()}.\n-     * \n-     * @param style\n-     * @return\n+     * Retrieve the workspace configuration XML as a Resource\n+     * @param ws The workspace\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource config(WorkspaceInfo ws) {\n+        Resource r = get(ws, WORKSPACE_XML);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve a resource in the the configuration directory of the workspace associated with a \n+     * namespace.  An empty path will retrieve the directory itself.\n+     * @param ws The namespace\n+     * @return A {@link Resource}\n      */\n-    String pathStyleFile(StyleInfo style) {\n-        String configFileName = style.getName() + \".xml\";\n-        if (configFileName.equals(style.getFilename())) {\n-            configFileName = configFileName + \".xml\";\n+    public @Nonnull Resource get(NamespaceInfo ns, String... path) {\n+        Resource r = getWorkspaces(ns.getPrefix(), Paths.path(path));\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the namespace configuration XML as a Resource\n+     * @param ns The namespace\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource config(NamespaceInfo ns) {\n+        Resource r = get(ns, NAMESPACE_XML);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve a resource in the the configuration directory of a Store.  An empty path will \n+     * retrieve the directory itself.\n+     * @param store The store\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource get(StoreInfo store, String... path) {\n+        Resource r = get(store.getWorkspace(), store.getName(), Paths.path(path));\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the datastore configuration XML as a Resource\n+     * @param ds The datastore\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource config(DataStoreInfo ds) {\n+        Resource r = get(ds, DATASTORE_XML);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the coverage store configuration XML as a Resource\n+     * @param cs The coverage store\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource config(CoverageStoreInfo cs) {\n+        Resource r = get(cs, COVERAGESTORE_XML);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the WMS store configuration XML as a Resource\n+     * @param wmss The coverage store\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource config(WMSStoreInfo wmss) {\n+        Resource r = get(wmss, WMSSTORE_XML);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the WMS store configuration XML as a Resource\n+     * @param wmss The coverage store\n+     * @return A {@link Resource}\n+     */\n+    private @Nonnull Resource config(StoreInfo si) {\n+        final Resource r;\n+        if(si instanceof DataStoreInfo) {\n+            r=config((DataStoreInfo) si);\n+        } else if(si instanceof CoverageStoreInfo) {\n+            r=config((CoverageStoreInfo) si);\n+        } else if(si instanceof WMSStoreInfo) {\n+            r=config((WMSStoreInfo) si);\n+        } else {\n+            // It'd be nice if we could be generic and cover potential future StoreInfo types.\n+            throw new IllegalArgumentException(\n+                    \"Only DataStoreInfo, CoverageStoreInfo, and WMSStoreInfo are supported.\");\n         }\n-        WorkspaceInfo workspace = style != null ? style.getWorkspace() : null;\n-        if (workspace == null) {\n-            return Paths.convert(\"styles\", configFileName);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the resource configuration XML as a Resource\n+     * @param wmss The resource\n+     * @return A {@link Resource}\n+     */\n+    private @Nonnull Resource config(ResourceInfo si) {\n+        final Resource r;\n+        if(si instanceof FeatureTypeInfo) {\n+            r=config((FeatureTypeInfo) si);\n+        } else if(si instanceof CoverageInfo) {\n+            r=config((CoverageInfo) si);\n+        } else if(si instanceof WMSLayerInfo) {\n+            r=config((WMSLayerInfo) si);\n         } else {\n-            return Paths.convert(Paths.path(\"workspaces\", workspace.getName(), \"styles\"),\n-                    configFileName);\n+            // It'd be nice if we could be generic and cover potential future ResourceInfo types.\n+            throw new IllegalArgumentException(\n+                    \"Only FeatureTypeInfo, CoverageInfo, and WMSLayerInfo are supported.\");\n         }\n+        assert r!=null;\n+        return r;\n     }\n-\n+    \n     /**\n-     * Path to generated style file, style directory used based on {@link StyleInfo#getWorkspace()}.\n-     * \n-     * @param style\n-     * @return\n+     * Retrieve a resource in the the configuration directory of a Resource.  An empty path will \n+     * retrieve the directory itself.\n+     * @param ri The store\n+     * @return A {@link Resource}\n      */\n-    String pathSldFile(StyleInfo style) {\n-        WorkspaceInfo workspace = style != null ? style.getWorkspace() : null;\n-        if (workspace == null) {\n-            return Paths.convert(\"styles\", style.getFilename());\n-        } else {\n-            return Paths.convert(Paths.path(\"workspaces\", workspace.getName(), \"styles\"),\n-                    style.getFilename());\n+    public @Nonnull Resource get(ResourceInfo ri, String... path) {\n+        Resource r = get(ri.getStore(), ri.getName(), Paths.path(path));\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the feature type configuration XML as a Resource\n+     * @param fti The feature type\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource config(FeatureTypeInfo fti) {\n+        Resource r = get(fti, FEATURETYPE_XML);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the coverage configuration XML as a Resource\n+     * @param c The feature type\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource config(CoverageInfo c) {\n+        Resource r = get(c, COVERAGE_XML);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve the WMS layer configuration XML as a Resource\n+     * @param wmsl The feature type\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource config(WMSLayerInfo wmsl) {\n+        Resource r = get(wmsl, WMSLAYER_XML);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve a resource in the the configuration directory of a Layer.  An empty path will \n+     * retrieve the directory itself.\n+     * @param li The store\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource get(LayerInfo l, String... path ) {\n+        final Resource r;\n+        if ( l.getResource() instanceof FeatureTypeInfo) {\n+            r = get( (FeatureTypeInfo) l.getResource(), path );\n         }\n+        else if ( l.getResource() instanceof CoverageInfo ) {\n+            r = get( (CoverageInfo) l.getResource(), path );\n+        }\n+        else if ( l.getResource() instanceof WMSLayerInfo ) {\n+            r = get( (WMSLayerInfo) l.getResource(), path );\n+        }\n+        else {\n+            // It'd be nice if we could be generic and cover potential future ResourceInfo types.\n+            throw new IllegalArgumentException(\n+                    \"Only FeatureTypeInfo, CoverageInfo, and WMSLayerInfo are supported.\");\n+        }\n+        assert r!=null;\n+        return r;\n     }\n-\n-    @Deprecated\n-    private File styleSldFile(boolean create, StyleInfo s) throws IOException {\n-        if (create) {\n-            Resource resource = get(pathSldFile(s));\n-            return resource.file();\n+    \n+    /**\n+     * Retrieve the layer configuration XML as a Resource\n+     * @param li The feature type\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource config(LayerInfo li) {\n+        Resource r = get(li, LAYER_XML);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve a resource in the the layer groups directory. An empty path will retrieve\n+     * the directory itself.\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource getLayerGroups(String... path) {\n+        Resource r = get(  Paths.path( LAYERGROUP_DIR, Paths.path(path)));\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve a resource in the the layer groups directory of a workspace. An empty path will retrieve\n+     * the directory itself.\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource getLayerGroups(WorkspaceInfo wsi, String... path) {\n+        Resource r = get(wsi, Paths.path( LAYERGROUP_DIR, Paths.path(path)));\n+        assert r!=null;\n+        return r;\n+    }\n+    \n+    /**\n+     * Retrieve a resource in the the configuration directory of a LayerGroup.  An empty path will \n+     * retrieve the directory itself. This directory is shared by all Layer Groups in a Workspace.\n+     * @param lgi The store\n+     * @return A {@link Resource}\n+     */\n+    public @Nonnull Resource get(LayerGroupInfo lgi, String... path) {\n+        WorkspaceInfo wsi = lgi.getWorkspace();\n+        final Resource r;\n+        if(wsi==null) {\n+            r = getLayerGroups(path);\n         } else {\n-            Resource resource = get(pathSldFile(s));\n-            return Resources.file(resource);\n+            r = getLayerGroups(wsi, path);\n         }\n+        assert r!=null;\n+        return r;\n     }\n-\n+    \n     /**\n-     * Copies a file into the global style configuration directory.\n-     * <p>\n-     * If the resource directory does exist it will be created\n-     * </p>\n-     * \n-     * @deprecated use {@link #copyToStyleDir(File, StyleInfo)}\n+     * Retrieve the layer group configuration XML as a Resource\n+     * @param lgi The layer group\n+     * @return A {@link Resource}\n      */\n-    public void copyToStyleDir(File f) throws IOException {\n-        Resource styles = get(\"styles\");\n-        FileUtils.copyFileToDirectory(f, styles.dir());\n+    public @Nonnull Resource config(LayerGroupInfo lgi) {\n+        Resource r = get(lgi, String.format(\"%s.xml\", lgi.getName()));\n+        assert r!=null;\n+        return r;\n     }\n+    \n \n     /**\n-     * Copy file to styles directory (determined using {@link StyleInfo#getWorkspace()}).\n-     * \n-     * @param file\n-     * @param style\n-     * @throws IOException\n+     * Retrieve a resource in the the styles directory. An empty path will retrieve\n+     * the directory itself.\n+     * @return A {@link Resource}\n      */\n-    @Deprecated\n-    public void copyToStyleDir(File file, StyleInfo style) throws IOException {\n-        Resource styles = get(pathStyles(style));\n-        FileUtils.copyFileToDirectory(file, styles.dir());\n+    public @Nonnull Resource getStyles(String... path) {\n+        Resource r = get(  Paths.path( STYLE_DIR, Paths.path(path)));\n+        assert r!=null;\n+        return r;\n     }\n-\n+    \n     /**\n-     * Copies data into the global style directory.\n-     * <p>\n-     * If the style directory does exist it will be created\n-     * </p>\n+     * Retrieve a resource in the the styles directory of a workspace. An empty path will retrieve\n+     * the directory itself.\n+     * @return A {@link Resource}\n      */\n-    public void copyToStyleDir(InputStream data, String filename) throws IOException {\n-        Resource styles = get(\"styles\");\n-        copy(data, styles.dir(), filename);\n+    public @Nonnull Resource getStyles(WorkspaceInfo wsi, String... path) {\n+        Resource r = get(wsi, Paths.path( STYLE_DIR, Paths.path(path)));\n+        assert r!=null;\n+        return r;\n     }\n \n     /**\n-     * Style directory for the provided workspace (or global styles directory if workspace not provided).\n-     * \n-     * @param workspace\n-     * @return Path to styles directory\n+     * Retrieve a resource in the the configuration directory of a Resource.  An empty path will \n+     * retrieve the directory itself.\n+     * @param si The store\n+     * @return A {@link Resource}\n      */\n-    private String pathLayerGroup(LayerGroupInfo layerGroup) {\n-        WorkspaceInfo workspace = layerGroup.getWorkspace();\n+    public @Nonnull Resource get(StyleInfo si, String... path) {\n+        WorkspaceInfo workspace = si != null ? si.getWorkspace() : null;\n+        final Resource r;\n         if (workspace == null) {\n-            return \"layergroups\";\n+            r = getStyles(path);\n         } else {\n-            return Paths.path(\"workspaces\", workspace.getName(), \"layergroups\");\n+            r = getStyles(workspace, path);\n         }\n+        assert r!=null;\n+        return r;\n     }\n \n     /**\n-     * Style directory for the provided workspace (or global styles directory if workspace not provided).\n-     * \n-     * @param workspace\n-     * @return Path to styles directory\n+     * Retrieve the style configuration XML as a Resource\n+     * @param c The style\n+     * @return A {@link Resource}\n      */\n-    private String pathLayerGroup(WorkspaceInfo workspace) {\n-        if (workspace == null) {\n-            return \"layergroups\";\n+    public @Nonnull Resource config(StyleInfo s) {\n+        //special case for styles, if the file name (minus the suffix) matches the id of the style\n+        // and the suffix is xml (rather than sld) we need to avoid overwritting the actual \n+        // style file\n+        final String filename;\n+        if (s.getFilename() != null && s.getFilename().endsWith(\".xml\") \n+                && s.getFilename().startsWith(s.getName()+\".\")) {\n+            //append a second .xml suffix\n+            filename = s.getName() + \".xml.xml\";\n         } else {\n-            return Paths.path(\"workspaces\", workspace.getName(), \"layergroups\");\n+            filename = s.getName() + \".xml\";\n         }\n-    }\n \n+        Resource r = get(s, filename);\n+        assert r!=null;\n+        return r;\n+    }\n+    \n     /**\n-     * Returns the directory in which global layer groups are persisted, if the directory does not exist null is returned.\n+     * Retrieve the style definition as a Resource\n+     * @param c The style\n+     * @return A {@link Resource}\n      */\n-    public File findLayerGroupDir() throws IOException {\n-        Resource resource = get(\"layergroups\");\n-        return Resources.directory(resource);\n+    public @Nonnull Resource style(StyleInfo s) {\n+        // Must be a simple filename\n+        final String filename = s.getFilename();\n+        Resource r = get(s, filename);\n+        assert r!=null;\n+        return r;\n     }\n-\n+    \n     /**\n-     * Returns the directory in which global layer groups are persisted, if the directory does not exist it will be created.\n+     * Retrieve the StyleInfo as a GeoTools Style object. Note this is just\n+     * the data structure as written, the matching external graphics are unmodified\n+     * and may not be (yet) available on the local system.","path":"src/main/src/main/java/org/geoserver/config/GeoServerDataDirectory.java","position":1247,"original_position":1247,"commit_id":"2421716191e06994cc9f686c538f30060a0db4e2","original_commit_id":"2421716191e06994cc9f686c538f30060a0db4e2","user":{"login":"smithkm","id":2184425,"avatar_url":"https://avatars.githubusercontent.com/u/2184425?","gravatar_id":"6196eee831ad4213d18126cd6d25ae99","url":"https://api.github.com/users/smithkm","html_url":"https://github.com/smithkm","followers_url":"https://api.github.com/users/smithkm/followers","following_url":"https://api.github.com/users/smithkm/following{/other_user}","gists_url":"https://api.github.com/users/smithkm/gists{/gist_id}","starred_url":"https://api.github.com/users/smithkm/starred{/owner}{/repo}","subscriptions_url":"https://api.github.com/users/smithkm/subscriptions","organizations_url":"https://api.github.com/users/smithkm/orgs","repos_url":"https://api.github.com/users/smithkm/repos","events_url":"https://api.github.com/users/smithkm/events{/privacy}","received_events_url":"https://api.github.com/users/smithkm/received_events","type":"User","site_admin":false},"body":"Ah, I see where the ResourceLocator is lurking in the SLD 1.0 parser now. ","created_at":"2014-05-28T18:30:43Z","updated_at":"2014-05-28T18:30:43Z","html_url":"https://github.com/geoserver/geoserver/pull/591#discussion_r13147543","pull_request_url":"https://api.github.com/repos/geoserver/geoserver/pulls/591","_links":{"self":{"href":"https://api.github.com/repos/geoserver/geoserver/pulls/comments/13147543"},"html":{"href":"https://github.com/geoserver/geoserver/pull/591#discussion_r13147543"},"pull_request":{"href":"https://api.github.com/repos/geoserver/geoserver/pulls/591"}}}}