[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]

Re: [Pki-devel] [PATCH] misc fixes for Profile REST API and CLI



Fixed in attached patch.  Apply on top of previous patches.
(137 and 138).

Ade

On Tue, 2013-07-30 at 10:37 -0500, Endi Sukma Dewata wrote:
> On 7/29/2013 12:16 PM, Ade Lee wrote:
> >      Fix various issues with Profile Interface
> >
> >      1. Fixed REST API as per review.
> >      2. Add output for profile-show and profile-find
> >
> > Please review,
> > Ade
> 
> Some comments:
> 
> 1. The URL in profile-show output is missing a '/rest':
> 
>    https://hostname:8443/ca/profiles/caManualRenewal
> 
> 2. When calling profile-show as an agent/admin, the visibleOnly in 
> retrieveProfile() is set to true, so it fails showing invisible profiles.
> 
>    if (visibleOnly && !profile.isVisible()) {
>        throw new ProfileNotFoundException(profileId);
>    }
> 
> The visibleOnly should be set to true by default, and set to false if 
> it's an privileged user, same thing as in listProfiles().
> 
> 3. The output labels can be simplified up a little bit:
> 
>    Profile ID: ...
>    Name: ...
>    Description: ...
> 
> 4. There is a double space between "Profile ID:" and the value.
> 
> 5. In general the profile URL is not needed by CLI users. It may only be 
> useful for advanced users so it doesn't need to be displayed by default. 
> In user-find the user URL will only appear in verbose mode.
> 
> 6. This line probably can be removed since the profile ID is already 
> displayed earlier.
> 
>    Profile Inputs: <profile ID>
> 
> 7. The inputs probably can be simplified as follows:
> 
>    Input ID: i1
>    Name: Serial Number of Certificate to Renew
>    Class: serialNumRenewInputImpl
>    Attribute Name: serial_num
>    Attribute Description: Serial Number of Certificate to Renew
>    Attribute Syntax: string
> 
> If there are multiple inputs they can be separated by blank lines. Same 
> thing for outputs and policy sets.
> 
> Another possibility is to use separate commands such as 
> profile-input-find/show/add/del to manage the inputs.
> 
> 8. In the XML output the profile element can be simplified as follows:
> 
>    <Profile id="caManualRenewal">
>      ...
>    </Profile>
> 
> "Profile" is more user-friendly than "ProfileData".
> 
> 9. The input element can also be simplified as follows:
> 
>    <Input id="i1">
>      <attribute name="...">
>      </attribute>
>    </Input>
> 
> 10. The unused code in createProfileDataInfo() can be removed.
> 

>From 18c594a4b9b562d74986ba8da259a0b56849f88c Mon Sep 17 00:00:00 2001
From: Ade Lee <alee redhat com>
Date: Wed, 31 Jul 2013 13:38:33 -0400
Subject: [PATCH] Fixes for profile REST interface from code review.

Simplified the inputs, outputs for ProfileData
---
 .../src/com/netscape/cms/servlet/test/CATest.java  |  10 +-
 .../com/netscape/certsrv/profile/ProfileData.java  | 113 +++++----------------
 .../com/netscape/certsrv/profile/ProfileInput.java |  15 ++-
 .../netscape/certsrv/profile/ProfileOutput.java    |  15 ++-
 .../cms/servlet/profile/ProfileService.java        |  29 +++---
 .../com/netscape/cmstools/profile/ProfileCLI.java  |  56 +++++-----
 6 files changed, 95 insertions(+), 143 deletions(-)

diff --git a/base/ca/functional/src/com/netscape/cms/servlet/test/CATest.java b/base/ca/functional/src/com/netscape/cms/servlet/test/CATest.java
index 0268c2d7d851cb94db67991a6f1baf163f69b3d1..fd0db892a23fbe54c75d364d9af35e022d0b6e85 100644
--- a/base/ca/functional/src/com/netscape/cms/servlet/test/CATest.java
+++ b/base/ca/functional/src/com/netscape/cms/servlet/test/CATest.java
@@ -18,7 +18,7 @@
 package com.netscape.cms.servlet.test;
 
 import java.util.Collection;
-import java.util.Map;
+import java.util.List;
 
 import org.apache.commons.cli.CommandLine;
 import org.apache.commons.cli.CommandLineParser;
@@ -434,10 +434,10 @@ public class CATest {
 
         log("Profile Input Information: \n");
 
-        Map<String, ProfileInput> inputs = info.getInputs();
-        for (Map.Entry<String, ProfileInput> entry : inputs.entrySet()) {
-            log("Input Id: " + entry.getKey());
-            for (ProfileAttribute attr: entry.getValue().getAttrs()) {
+        List<ProfileInput> inputs = info.getInputs();
+        for (ProfileInput input : inputs) {
+            log("Input Id: " + input.getId());
+            for (ProfileAttribute attr: input.getAttrs()) {
                 log("Input Attribute Name: " + attr.getName() + "\n");
                 log("Input Attribute Value: " + attr.getValue() + "\n");
             }
diff --git a/base/common/src/com/netscape/certsrv/profile/ProfileData.java b/base/common/src/com/netscape/certsrv/profile/ProfileData.java
index b1eca788776104cf89d221280e253eba0355015f..2985a976423b54cb8572ab50b04cba43c939a3a9 100644
--- a/base/common/src/com/netscape/certsrv/profile/ProfileData.java
+++ b/base/common/src/com/netscape/certsrv/profile/ProfileData.java
@@ -29,6 +29,7 @@ import java.util.Vector;
 
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
+import javax.xml.bind.annotation.XmlAttribute;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 import javax.xml.bind.annotation.adapters.XmlAdapter;
@@ -39,11 +40,11 @@ import javax.xml.bind.annotation.adapters.XmlJavaTypeAdapter;
  *
  */
 
- XmlRootElement(name = "ProfileData")
+ XmlRootElement(name = "Profile")
 @XmlAccessorType(XmlAccessType.FIELD)
 public class ProfileData {
 
-    @XmlElement
+    @XmlAttribute
     protected String id;
 
     @XmlElement
@@ -76,13 +77,11 @@ public class ProfileData {
     @XmlElement
     protected boolean xmlOutput;
 
-    @XmlElement(name = "Inputs")
-    @XmlJavaTypeAdapter(InputAdapter.class)
-    protected Map<String, ProfileInput> inputs = new LinkedHashMap<String, ProfileInput>();
+    @XmlElement(name = "Input")
+    protected List<ProfileInput> inputs = new ArrayList<ProfileInput>();
 
-    @XmlElement(name = "Outputs")
-    @XmlJavaTypeAdapter(OutputAdapter.class)
-    protected Map<String, ProfileOutput> outputs = new LinkedHashMap<String, ProfileOutput>();
+    @XmlElement(name = "Output")
+    protected List<ProfileOutput> outputs = new ArrayList<ProfileOutput>();
 
     @XmlElement(name = "PolicySets")
     @XmlJavaTypeAdapter(PolicySetAdapter.class)
@@ -177,27 +176,30 @@ public class ProfileData {
         this.classId = classId;
     }
 
-    public void addProfileInput(String id, ProfileInput input) {
-        inputs.put(id, input);
+    public void addProfileInput(ProfileInput input) {
+        inputs.add(input);
     }
 
     public ProfileInput getProfileInput(String id) {
-        return inputs.get(id);
+        for (ProfileInput input: inputs) {
+            if (input.getId().equals(id)) return input;
+        }
+        return null;
     }
 
-    public Map<String, ProfileInput> getInputs() {
+    public List<ProfileInput> getInputs() {
         return inputs;
     }
 
-    public void setInputs(Map<String, ProfileInput> inputs) {
+    public void setInputs(List<ProfileInput> inputs) {
         this.inputs = inputs;
     }
 
-    public Map<String, ProfileOutput> getOutputs() {
+    public List<ProfileOutput> getOutputs() {
         return outputs;
     }
 
-    public void setOutputs(Map<String, ProfileOutput> outputs) {
+    public void setOutputs(List<ProfileOutput> outputs) {
         this.outputs = outputs;
     }
 
@@ -213,84 +215,15 @@ public class ProfileData {
         this.policySets.put(id, policySet);
     }
 
-    public void addProfileOutput(String id, ProfileOutput output) {
-        outputs.put(id, output);
+    public void addProfileOutput(ProfileOutput output) {
+        outputs.add(output);
     }
 
     public ProfileOutput getProfileOutput(String id) {
-        return outputs.get(id);
-    }
-
-    public static class InputAdapter extends XmlAdapter<InputList, Map<String, ProfileInput>> {
-
-        public InputList marshal(Map<String,ProfileInput> map) {
-            InputList list = new InputList();
-            for (Map.Entry<String, ProfileInput> entry : map.entrySet()) {
-                Input input = new Input();
-                input.name = entry.getKey();
-                input.value = entry.getValue();
-                list.inputs.add(input);
-            }
-            return list;
+        for (ProfileOutput output: outputs) {
+            if (output.getId().equals(id)) return output;
         }
-
-        public Map<String, ProfileInput> unmarshal(InputList list) {
-            Map<String, ProfileInput> map = new LinkedHashMap<String, ProfileInput>();
-            for (Input input : list.inputs) {
-                map.put(input.name, input.value);
-            }
-            return map;
-        }
-    }
-
-    public static class InputList {
-        @XmlElement(name="input")
-        public List<Input> inputs = new ArrayList<Input>();
-    }
-
-    public static class Input {
-
-        @XmlElement(name="id")
-        public String name;
-
-        @XmlElement
-        public ProfileInput value;
-    }
-
-    public static class OutputAdapter extends XmlAdapter<OutputList, Map<String, ProfileOutput>> {
-
-        public OutputList marshal(Map<String,ProfileOutput> map) {
-            OutputList list = new OutputList();
-            for (Map.Entry<String, ProfileOutput> entry : map.entrySet()) {
-                Output output = new Output();
-                output.name = entry.getKey();
-                output.value = entry.getValue();
-                list.outputs.add(output);
-            }
-            return list;
-        }
-
-        public Map<String, ProfileOutput> unmarshal(OutputList list) {
-            Map<String, ProfileOutput> map = new LinkedHashMap<String, ProfileOutput>();
-            for (Output output : list.outputs) {
-                map.put(output.name, output.value);
-            }
-            return map;
-        }
-    }
-
-    public static class OutputList {
-        @XmlElement(name="output")
-        public List<Output> outputs = new ArrayList<Output>();
-    }
-
-    public static class Output {
-
-        @XmlElement(name="id")
-        public String name;
-
-        @XmlElement
-        public ProfileOutput value;
+        return null;
     }
 
     public static class PolicySetAdapter extends XmlAdapter<PolicySetList, Map<String, Vector<ProfilePolicy>>> {
@@ -330,7 +263,7 @@ public class ProfileData {
     }
 
     public static void main(String args[]) throws Exception {
-        Map<String, ProfileInput> inputs = new LinkedHashMap<String, ProfileInput>();
+        List<ProfileInput> inputs = new ArrayList<ProfileInput>();
         //ProfileInput input = new ProfileInput();
         //input.setClassId(classId);
         //input.setInputId(inputId);
diff --git a/base/common/src/com/netscape/certsrv/profile/ProfileInput.java b/base/common/src/com/netscape/certsrv/profile/ProfileInput.java
index 3a3aefe4c136a15f2aadbace4ae64c0e00812655..a9b9507cbbaffe6cd49d1f902188ff8002f85020 100644
--- a/base/common/src/com/netscape/certsrv/profile/ProfileInput.java
+++ b/base/common/src/com/netscape/certsrv/profile/ProfileInput.java
@@ -22,12 +22,13 @@ import java.util.Enumeration;
 import java.util.List;
 import java.util.Locale;
 
+import javax.xml.bind.annotation.XmlAttribute;
 import javax.xml.bind.annotation.XmlElement;
 
 import com.netscape.certsrv.property.Descriptor;
 
 public class ProfileInput {
-
+    private String id;
     private String classId;
     private String name;
     private String text;
@@ -38,8 +39,9 @@ public class ProfileInput {
         // required for jaxb
     }
 
-    public ProfileInput(IProfileInput input, String classId, Locale locale) {
+    public ProfileInput(IProfileInput input, String id, String classId, Locale locale) {
         this.name = input.getName(locale);
+        this.id = id;
         this.classId = classId;
         Enumeration<String> names = input.getValueNames();
         while (names.hasMoreElements()) {
@@ -68,6 +70,15 @@ public class ProfileInput {
         this.classId = classId;
     }
 
+    @XmlAttribute
+    public String getId() {
+        return id;
+    }
+
+    public void setId(String id) {
+        this.id = id;
+    }
+
     public void setName(String name) {
         this.name = name;
     }
diff --git a/base/common/src/com/netscape/certsrv/profile/ProfileOutput.java b/base/common/src/com/netscape/certsrv/profile/ProfileOutput.java
index 492fdb46a4b0de2a92d7dbe7e8fca2488f0b689e..759b65ce72cbe250a672b4c780d615c3cca10c50 100644
--- a/base/common/src/com/netscape/certsrv/profile/ProfileOutput.java
+++ b/base/common/src/com/netscape/certsrv/profile/ProfileOutput.java
@@ -24,6 +24,7 @@ import java.util.Locale;
 
 import javax.xml.bind.annotation.XmlAccessType;
 import javax.xml.bind.annotation.XmlAccessorType;
+import javax.xml.bind.annotation.XmlAttribute;
 import javax.xml.bind.annotation.XmlElement;
 import javax.xml.bind.annotation.XmlRootElement;
 
@@ -33,12 +34,23 @@ import com.netscape.certsrv.property.Descriptor;
 @XmlAccessorType(XmlAccessType.FIELD)
 public class ProfileOutput {
 
+    @XmlAttribute
+    private String  id;
+
     @XmlElement
     private String name;
 
     @XmlElement
     private String text;
 
+    public String getId() {
+        return id;
+    }
+
+    public void setId(String id) {
+        this.id = id;
+    }
+
     @XmlElement
     private String classId;
 
@@ -50,8 +62,9 @@ public class ProfileOutput {
         // required for jaxb
     }
 
-    public ProfileOutput(IProfileOutput output, String classId, Locale locale) {
+    public ProfileOutput(IProfileOutput output, String id, String classId, Locale locale) {
         this.name = output.getName(locale);
+        this.id = id;
         this.classId = classId;
         Enumeration<String> names = output.getValueNames();
         while (names.hasMoreElements()) {
diff --git a/base/common/src/com/netscape/cms/servlet/profile/ProfileService.java b/base/common/src/com/netscape/cms/servlet/profile/ProfileService.java
index 0feaa126f18650cc559885ab7d524444017d23ff..a848579a5d1e609d23fab059cd791b5858bd2849 100644
--- a/base/common/src/com/netscape/cms/servlet/profile/ProfileService.java
+++ b/base/common/src/com/netscape/cms/servlet/profile/ProfileService.java
@@ -27,7 +27,6 @@ import java.util.Enumeration;
 import java.util.LinkedHashMap;
 import java.util.List;
 import java.util.Map;
-import java.util.Map.Entry;
 import java.util.Vector;
 
 import javax.ws.rs.PathParam;
@@ -113,7 +112,7 @@ public class ProfileService extends PKIService implements ProfileResource {
 
     public ProfileData retrieveProfile(String profileId) throws ProfileNotFoundException {
         ProfileData data = null;
-        boolean visibleOnly = false;
+        boolean visibleOnly = true;
 
         if (ps == null) {
             return null;
@@ -123,7 +122,7 @@ public class ProfileService extends PKIService implements ProfileResource {
         if ((principal != null) &&
             (principal.hasRole("Certificate Manager Agents") ||
              principal.hasRole("Certificate Manager Administrators"))) {
-            visibleOnly = true;
+            visibleOnly = false;
         }
 
         Enumeration<String> profileIds = ps.getProfileIds();
@@ -202,8 +201,8 @@ public class ProfileService extends PKIService implements ProfileResource {
 
                 String classId = inputStore.getString(inputId + ".class_id");
 
-                ProfileInput input = new ProfileInput(profileInput, classId, getLocale());
-                data.addProfileInput(inputId, input);
+                ProfileInput input = new ProfileInput(profileInput, inputId, classId, getLocale());
+                data.addProfileInput(input);
             }
         }
 
@@ -221,8 +220,8 @@ public class ProfileService extends PKIService implements ProfileResource {
 
                 String classId = outputStore.getString(outputId + ".class_id");
 
-                ProfileOutput output = new ProfileOutput(profileOutput, classId, getLocale());
-                data.addProfileOutput(outputId, output);
+                ProfileOutput output = new ProfileOutput(profileOutput, outputId, classId, getLocale());
+                data.addProfileOutput(output);
             }
         }
 
@@ -537,10 +536,10 @@ public class ProfileService extends PKIService implements ProfileResource {
 
     private void populateProfileOutputs(ProfileData data, IProfile profile) throws EProfileException {
         profile.deleteAllProfileOutputs();
-        Map<String, ProfileOutput> outputs = data.getOutputs();
-        for (Entry<String, ProfileOutput> entry: outputs.entrySet()) {
-            String id = entry.getKey();
-            String classId = entry.getValue().getClassId();
+        List<ProfileOutput> outputs = data.getOutputs();
+        for (ProfileOutput output: outputs) {
+            String id = output.getId();
+            String classId = output.getClassId();
 
             NameValuePairs nvp = new NameValuePairs();
             // TODO - add a field for params in ProfileOuput
@@ -551,10 +550,10 @@ public class ProfileService extends PKIService implements ProfileResource {
 
     private void populateProfileInputs(ProfileData data, IProfile profile) throws EProfileException {
         profile.deleteAllProfileInputs();
-       Map<String, ProfileInput> inputs = data.getInputs();
-        for (Entry<String, ProfileInput> entry: inputs.entrySet()) {
-            String id = entry.getKey();
-            String classId = entry.getValue().getClassId();
+       List<ProfileInput> inputs = data.getInputs();
+        for (ProfileInput input: inputs) {
+            String id = input.getId();
+            String classId = input.getClassId();
 
             NameValuePairs nvp = new NameValuePairs();
             // TODO - add a field for params in ProfileInput.
diff --git a/base/java-tools/src/com/netscape/cmstools/profile/ProfileCLI.java b/base/java-tools/src/com/netscape/cmstools/profile/ProfileCLI.java
index 1463b096eb572b43d8985da7b86d0863f5b20ac0..bab5d08ce3231c455eec5eba8f73c787709d1e78 100644
--- a/base/java-tools/src/com/netscape/cmstools/profile/ProfileCLI.java
+++ b/base/java-tools/src/com/netscape/cmstools/profile/ProfileCLI.java
@@ -6,7 +6,6 @@ import java.io.FileOutputStream;
 import java.net.URI;
 import java.util.Arrays;
 import java.util.Locale;
-import java.util.Map;
 
 import javax.ws.rs.core.UriBuilder;
 import javax.xml.bind.JAXBContext;
@@ -94,53 +93,50 @@ public class ProfileCLI extends CLI {
 
     public static void printProfileDataInfo(ProfileDataInfo info) {
         System.out.println("Profile ID:  " + info.getProfileId());
-        System.out.println("Profile URL: " + info.getProfileURL());
-        System.out.println("Profile Name: " + info.getProfileName());
-        System.out.println("Profile Description: " + info.getProfileDescription());
+        System.out.println("URL: " + info.getProfileURL());
+        System.out.println("Name: " + info.getProfileName());
+        System.out.println("Description: " + info.getProfileDescription());
     }
 
     public static void printProfile(ProfileData data, URI baseUri) {
 
         UriBuilder profileBuilder = UriBuilder.fromUri(baseUri);
-        URI uri = profileBuilder.path(ProfileResource.class).path("{id}").
+        URI uri = profileBuilder.path("rest").path(ProfileResource.class).path("{id}").
                 build(data.getId());
 
-        System.out.println("Profile ID:  " + data.getId());
-        System.out.println("Profile URL: " + uri.toString());
-        System.out.println("Profile Name: " + data.getName());
-        System.out.println("Profile Description: " + data.getDescription() + "\n");
+        System.out.println("Profile ID: " + data.getId());
+        if (verbose) {
+            System.out.println("URL: " + uri.toString());
+        }
+        System.out.println("Name: " + data.getName());
+        System.out.println("Description: " + data.getDescription());
 
-        System.out.println("Profile Inputs:  " + data.getId());
-        int count =0;
-        for (Map.Entry<String, ProfileInput> entry: data.getInputs().entrySet()) {
-            ProfileInput input = entry.getValue();
-            System.out.println("Input " + count + " Id: " + entry.getKey());
-            System.out.println("Input " + count + " Name: " + input.getName());
-            System.out.println("Input " + count + " Class: " + input.getClassId());
+        for (ProfileInput input: data.getInputs()) {
+            System.out.println();
+            System.out.println("Input Id: " + input.getId());
+            System.out.println("Name: " + input.getName());
+            System.out.println("Class: " + input.getClassId());
             for (ProfileAttribute attr: input.getAttrs()) {
-                System.out.println("Input " + count + " Attribute Name: " + attr.getName());
-                System.out.println("Input " + count + " Attribute Description: " +
+                System.out.println("Attribute Name: " + attr.getName());
+                System.out.println("Attribute Description: " +
                     attr.getDescriptor().getDescription(Locale.getDefault()));
-                System.out.println("Input " + count + " Attribute Syntax: " +
+                System.out.println("Attribute Syntax: " +
                     attr.getDescriptor().getSyntax());
             }
-            count ++;
         }
 
-        count = 0;
-        for (Map.Entry<String, ProfileOutput> entry: data.getOutputs().entrySet()) {
-            ProfileOutput output = entry.getValue();
-            System.out.println("Output " + count + " Id: " + entry.getKey());
-            System.out.println("Output " + count + " Name: " + output.getName());
-            System.out.println("Output " + count + " Class: " + output.getClassId());
+        for (ProfileOutput output: data.getOutputs()) {
+            System.out.println();
+            System.out.println("Output Id: " + output.getId());
+            System.out.println("Name: " + output.getName());
+            System.out.println("Class: " + output.getClassId());
             for (ProfileAttribute attr: output.getAttrs()) {
-                System.out.println("Output " + count + " Attribute Name: " + attr.getName());
-                System.out.println("Output " + count + " Attribute Description: " +
+                System.out.println("Attribute Name: " + attr.getName());
+                System.out.println("Attribute Description: " +
                     attr.getDescriptor().getDescription(Locale.getDefault()));
-                System.out.println("Output " + count + " Attribute Syntax: " +
+                System.out.println("Attribute Syntax: " +
                     attr.getDescriptor().getSyntax());
             }
-            count ++;
         }
     }
 
-- 
1.8.1.4


[Date Prev][Date Next]   [Thread Prev][Thread Next]   [Thread Index] [Date Index] [Author Index]