Committed by
Gerrit Code Review
ONOS-3379: Config REST 500 errors on GET operations if keys don't exist
Change-Id: Ie32bdb70693c5571421840265b4be71d0706d797
Showing
2 changed files
with
86 additions
and
14 deletions
| ... | @@ -40,6 +40,7 @@ import java.util.Collection; | ... | @@ -40,6 +40,7 @@ import java.util.Collection; |
| 40 | import java.util.Dictionary; | 40 | import java.util.Dictionary; |
| 41 | import java.util.List; | 41 | import java.util.List; |
| 42 | import java.util.Random; | 42 | import java.util.Random; |
| 43 | +import java.util.Set; | ||
| 43 | import java.util.concurrent.CompletableFuture; | 44 | import java.util.concurrent.CompletableFuture; |
| 44 | import java.util.concurrent.ExecutionException; | 45 | import java.util.concurrent.ExecutionException; |
| 45 | import java.util.concurrent.Future; | 46 | import java.util.concurrent.Future; |
| ... | @@ -144,6 +145,23 @@ public abstract class Tools { | ... | @@ -144,6 +145,23 @@ public abstract class Tools { |
| 144 | } | 145 | } |
| 145 | 146 | ||
| 146 | /** | 147 | /** |
| 148 | + * Returns the specified set if the set is not null and not empty; | ||
| 149 | + * otherwise throws a not found exception. | ||
| 150 | + * | ||
| 151 | + * @param item set to check | ||
| 152 | + * @param message not found message | ||
| 153 | + * @param <T> Set item type | ||
| 154 | + * @return item if not null and not empty | ||
| 155 | + * @throws org.onlab.util.ItemNotFoundException if set is null or empty | ||
| 156 | + */ | ||
| 157 | + public static <T> Set<T> emptyIsNotFound(Set<T> item, String message) { | ||
| 158 | + if (item == null || item.isEmpty()) { | ||
| 159 | + throw new ItemNotFoundException(message); | ||
| 160 | + } | ||
| 161 | + return item; | ||
| 162 | + } | ||
| 163 | + | ||
| 164 | + /** | ||
| 147 | * Returns the specified item if that item is not null; otherwise throws | 165 | * Returns the specified item if that item is not null; otherwise throws |
| 148 | * bad argument exception. | 166 | * bad argument exception. |
| 149 | * | 167 | * | ... | ... |
| ... | @@ -15,10 +15,9 @@ | ... | @@ -15,10 +15,9 @@ |
| 15 | */ | 15 | */ |
| 16 | package org.onosproject.rest.resources; | 16 | package org.onosproject.rest.resources; |
| 17 | 17 | ||
| 18 | -import com.fasterxml.jackson.databind.node.ObjectNode; | 18 | +import java.io.IOException; |
| 19 | -import org.onosproject.net.config.NetworkConfigService; | 19 | +import java.io.InputStream; |
| 20 | -import org.onosproject.net.config.SubjectFactory; | 20 | +import java.util.Set; |
| 21 | -import org.onosproject.rest.AbstractWebResource; | ||
| 22 | 21 | ||
| 23 | import javax.ws.rs.Consumes; | 22 | import javax.ws.rs.Consumes; |
| 24 | import javax.ws.rs.DELETE; | 23 | import javax.ws.rs.DELETE; |
| ... | @@ -29,8 +28,16 @@ import javax.ws.rs.PathParam; | ... | @@ -29,8 +28,16 @@ import javax.ws.rs.PathParam; |
| 29 | import javax.ws.rs.Produces; | 28 | import javax.ws.rs.Produces; |
| 30 | import javax.ws.rs.core.MediaType; | 29 | import javax.ws.rs.core.MediaType; |
| 31 | import javax.ws.rs.core.Response; | 30 | import javax.ws.rs.core.Response; |
| 32 | -import java.io.IOException; | 31 | + |
| 33 | -import java.io.InputStream; | 32 | +import org.onosproject.net.config.Config; |
| 33 | +import org.onosproject.net.config.NetworkConfigService; | ||
| 34 | +import org.onosproject.net.config.SubjectFactory; | ||
| 35 | +import org.onosproject.rest.AbstractWebResource; | ||
| 36 | + | ||
| 37 | +import com.fasterxml.jackson.databind.node.ObjectNode; | ||
| 38 | + | ||
| 39 | +import static org.onlab.util.Tools.emptyIsNotFound; | ||
| 40 | +import static org.onlab.util.Tools.nullIsNotFound; | ||
| 34 | 41 | ||
| 35 | /** | 42 | /** |
| 36 | * Manage network configurations. | 43 | * Manage network configurations. |
| ... | @@ -38,8 +45,29 @@ import java.io.InputStream; | ... | @@ -38,8 +45,29 @@ import java.io.InputStream; |
| 38 | @Path("network/configuration") | 45 | @Path("network/configuration") |
| 39 | public class NetworkConfigWebResource extends AbstractWebResource { | 46 | public class NetworkConfigWebResource extends AbstractWebResource { |
| 40 | 47 | ||
| 48 | + | ||
| 49 | + private String subjectClassNotFoundErrorString(String subjectClassKey) { | ||
| 50 | + return "Config for '" + subjectClassKey + "' not found"; | ||
| 51 | + } | ||
| 52 | + | ||
| 53 | + private String subjectNotFoundErrorString(String subjectClassKey, | ||
| 54 | + String subjectKey) { | ||
| 55 | + return "Config for '" | ||
| 56 | + + subjectClassKey + "/" + subjectKey | ||
| 57 | + + "' not found"; | ||
| 58 | + } | ||
| 59 | + | ||
| 60 | + private String configKeyNotFoundErrorString(String subjectClassKey, | ||
| 61 | + String subjectKey, | ||
| 62 | + String configKey) { | ||
| 63 | + return "Config for '" | ||
| 64 | + + subjectClassKey + "/" + subjectKey + "/" + configKey | ||
| 65 | + + "' not found"; | ||
| 66 | + } | ||
| 67 | + | ||
| 41 | /** | 68 | /** |
| 42 | * Get entire network configuration base. | 69 | * Get entire network configuration base. |
| 70 | + * | ||
| 43 | * @rsModel NetCfgGet | 71 | * @rsModel NetCfgGet |
| 44 | * @return network configuration JSON | 72 | * @return network configuration JSON |
| 45 | */ | 73 | */ |
| ... | @@ -70,7 +98,9 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -70,7 +98,9 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 70 | public Response download(@PathParam("subjectClassKey") String subjectClassKey) { | 98 | public Response download(@PathParam("subjectClassKey") String subjectClassKey) { |
| 71 | NetworkConfigService service = get(NetworkConfigService.class); | 99 | NetworkConfigService service = get(NetworkConfigService.class); |
| 72 | ObjectNode root = mapper().createObjectNode(); | 100 | ObjectNode root = mapper().createObjectNode(); |
| 73 | - SubjectFactory subjectFactory = service.getSubjectFactory(subjectClassKey); | 101 | + SubjectFactory subjectFactory = |
| 102 | + nullIsNotFound(service.getSubjectFactory(subjectClassKey), | ||
| 103 | + subjectClassNotFoundErrorString(subjectClassKey)); | ||
| 74 | produceJson(service, root, subjectFactory, subjectFactory.subjectClass()); | 104 | produceJson(service, root, subjectFactory, subjectFactory.subjectClass()); |
| 75 | return ok(root).build(); | 105 | return ok(root).build(); |
| 76 | } | 106 | } |
| ... | @@ -90,8 +120,12 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -90,8 +120,12 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 90 | @PathParam("subjectKey") String subjectKey) { | 120 | @PathParam("subjectKey") String subjectKey) { |
| 91 | NetworkConfigService service = get(NetworkConfigService.class); | 121 | NetworkConfigService service = get(NetworkConfigService.class); |
| 92 | ObjectNode root = mapper().createObjectNode(); | 122 | ObjectNode root = mapper().createObjectNode(); |
| 93 | - SubjectFactory subjectFactory = service.getSubjectFactory(subjectClassKey); | 123 | + SubjectFactory subjectFactory = |
| 94 | - produceSubjectJson(service, root, subjectFactory.createSubject(subjectKey)); | 124 | + nullIsNotFound(service.getSubjectFactory(subjectClassKey), |
| 125 | + subjectClassNotFoundErrorString(subjectClassKey)); | ||
| 126 | + produceSubjectJson(service, root, subjectFactory.createSubject(subjectKey), | ||
| 127 | + true, | ||
| 128 | + subjectNotFoundErrorString(subjectClassKey, subjectKey)); | ||
| 95 | return ok(root).build(); | 129 | return ok(root).build(); |
| 96 | } | 130 | } |
| 97 | 131 | ||
| ... | @@ -111,20 +145,40 @@ public class NetworkConfigWebResource extends AbstractWebResource { | ... | @@ -111,20 +145,40 @@ public class NetworkConfigWebResource extends AbstractWebResource { |
| 111 | @PathParam("subjectKey") String subjectKey, | 145 | @PathParam("subjectKey") String subjectKey, |
| 112 | @PathParam("configKey") String configKey) { | 146 | @PathParam("configKey") String configKey) { |
| 113 | NetworkConfigService service = get(NetworkConfigService.class); | 147 | NetworkConfigService service = get(NetworkConfigService.class); |
| 114 | - return ok(service.getConfig(service.getSubjectFactory(subjectClassKey).createSubject(subjectKey), | 148 | + |
| 115 | - service.getConfigClass(subjectClassKey, configKey)).node()).build(); | 149 | + Object subject = |
| 150 | + nullIsNotFound(service.getSubjectFactory(subjectClassKey) | ||
| 151 | + .createSubject(subjectKey), | ||
| 152 | + subjectNotFoundErrorString(subjectClassKey, subjectKey)); | ||
| 153 | + | ||
| 154 | + Class configClass = | ||
| 155 | + nullIsNotFound(service.getConfigClass(subjectClassKey, configKey), | ||
| 156 | + configKeyNotFoundErrorString(subjectClassKey, subjectKey, configKey)); | ||
| 157 | + Config config = | ||
| 158 | + nullIsNotFound(service.getConfig(subject, configClass), | ||
| 159 | + configKeyNotFoundErrorString(subjectClassKey, | ||
| 160 | + subjectKey, | ||
| 161 | + configKey)); | ||
| 162 | + return ok(config.node()).build(); | ||
| 116 | } | 163 | } |
| 117 | 164 | ||
| 118 | @SuppressWarnings("unchecked") | 165 | @SuppressWarnings("unchecked") |
| 119 | private void produceJson(NetworkConfigService service, ObjectNode node, | 166 | private void produceJson(NetworkConfigService service, ObjectNode node, |
| 120 | SubjectFactory subjectFactory, Class subjectClass) { | 167 | SubjectFactory subjectFactory, Class subjectClass) { |
| 121 | service.getSubjects(subjectClass).forEach(s -> | 168 | service.getSubjects(subjectClass).forEach(s -> |
| 122 | - produceSubjectJson(service, newObject(node, subjectFactory.subjectKey(s)), s)); | 169 | + produceSubjectJson(service, newObject(node, subjectFactory.subjectKey(s)), s, false, "")); |
| 123 | } | 170 | } |
| 124 | 171 | ||
| 125 | private void produceSubjectJson(NetworkConfigService service, ObjectNode node, | 172 | private void produceSubjectJson(NetworkConfigService service, ObjectNode node, |
| 126 | - Object subject) { | 173 | + Object subject, |
| 127 | - service.getConfigs(subject).forEach(c -> node.set(c.key(), c.node())); | 174 | + boolean emptyIsError, |
| 175 | + String emptyErrorMessage) { | ||
| 176 | + Set<? extends Config<Object>> configs = service.getConfigs(subject); | ||
| 177 | + if (emptyIsError) { | ||
| 178 | + // caller wants an empty set to be a 404 | ||
| 179 | + configs = emptyIsNotFound(configs, emptyErrorMessage); | ||
| 180 | + } | ||
| 181 | + configs.forEach(c -> node.set(c.key(), c.node())); | ||
| 128 | } | 182 | } |
| 129 | 183 | ||
| 130 | 184 | ... | ... |
-
Please register or login to post a comment