From 31cbc922a7971a5f20d5790a1fbbd6b38a49af13 Mon Sep 17 00:00:00 2001 From: pkaras Date: Thu, 21 Mar 2019 11:10:12 +0100 Subject: [PATCH] Extract required(...) method Change-Id: I9a2a02aabd6b90c601f3bcd2cdd708e9af70d819 Issue-ID: DMAAP-1107 Signed-off-by: piotr.karas --- .../java/org/onap/dmaap/dbcapi/model/ApiError.java | 3 +- .../dmaap/dbcapi/resources/DR_NodeResource.java | 17 +++-- .../dmaap/dbcapi/resources/DR_PubResource.java | 23 +++--- .../dmaap/dbcapi/resources/DR_SubResource.java | 37 +++++----- .../onap/dmaap/dbcapi/resources/DmaapResource.java | 59 +++++++-------- .../onap/dmaap/dbcapi/resources/FeedResource.java | 19 ++--- .../dmaap/dbcapi/resources/MR_ClientResource.java | 37 +++++----- .../dmaap/dbcapi/resources/MR_ClusterResource.java | 21 +++--- .../dmaap/dbcapi/resources/RequiredChecker.java | 53 +++++++++++++ .../dbcapi/resources/RequiredFieldException.java | 28 +++++-- .../onap/dmaap/dbcapi/resources/TopicResource.java | 23 +++--- .../org/onap/dmaap/dbcapi/service/ApiService.java | 63 ++-------------- .../dbcapi/resources/RequiredCheckerTest.java | 86 ++++++++++++++++++++++ .../onap/dmaap/dbcapi/service/ApiServiceTest.java | 18 ----- 14 files changed, 290 insertions(+), 197 deletions(-) create mode 100644 src/main/java/org/onap/dmaap/dbcapi/resources/RequiredChecker.java create mode 100644 src/test/java/org/onap/dmaap/dbcapi/resources/RequiredCheckerTest.java diff --git a/src/main/java/org/onap/dmaap/dbcapi/model/ApiError.java b/src/main/java/org/onap/dmaap/dbcapi/model/ApiError.java index 2e05740..c67e55b 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/model/ApiError.java +++ b/src/main/java/org/onap/dmaap/dbcapi/model/ApiError.java @@ -21,10 +21,11 @@ package org.onap.dmaap.dbcapi.model; import javax.xml.bind.annotation.XmlRootElement; +import java.io.Serializable; import java.util.Objects; @XmlRootElement -public class ApiError { +public class ApiError implements Serializable { private int code; private String message; private String fields; diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/DR_NodeResource.java b/src/main/java/org/onap/dmaap/dbcapi/resources/DR_NodeResource.java index 029222e..49dc69a 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/resources/DR_NodeResource.java +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/DR_NodeResource.java @@ -57,6 +57,7 @@ public class DR_NodeResource extends BaseLoggingClass { private DR_NodeService dr_nodeService = new DR_NodeService(); private ResponseBuilder responseBuilder = new ResponseBuilder(); + private RequiredChecker checker = new RequiredChecker(); @GET @ApiOperation( value = "return DR_Node details", @@ -88,8 +89,8 @@ public class DR_NodeResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "dcaeLocation", node.getDcaeLocationName(), ""); - resp.required( "fqdn", node.getFqdn(), ""); + checker.required( "dcaeLocation", node.getDcaeLocationName()); + checker.required( "fqdn", node.getFqdn()); } catch ( RequiredFieldException rfe ) { return responseBuilder.error(new ApiError(BAD_REQUEST.getStatusCode(), "missing required field", "dcaeLocation, fqdn")); @@ -117,10 +118,10 @@ public class DR_NodeResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "dcaeLocation", name, ""); - resp.required( "fqdn", node.getFqdn(), ""); + checker.required( "dcaeLocation", name); + checker.required( "fqdn", node.getFqdn()); } catch ( RequiredFieldException rfe ) { - return responseBuilder.error(resp.getErr()); + return responseBuilder.error(rfe.getApiError()); } node.setFqdn(name); DR_Node nNode = dr_nodeService.updateDr_Node(node, resp.getErr()); @@ -146,10 +147,10 @@ public class DR_NodeResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "fqdn", name, ""); + checker.required( "fqdn", name); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } dr_nodeService.removeDr_Node(name, resp.getErr()); if ( resp.getErr().is2xx() ) { diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/DR_PubResource.java b/src/main/java/org/onap/dmaap/dbcapi/resources/DR_PubResource.java index 9c2ae21..1054b03 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/resources/DR_PubResource.java +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/DR_PubResource.java @@ -60,6 +60,7 @@ public class DR_PubResource extends BaseLoggingClass { private DR_PubService dr_pubService = new DR_PubService(); private ResponseBuilder responseBuilder = new ResponseBuilder(); + private RequiredChecker checker = new RequiredChecker(); @GET @ApiOperation( value = "return DR_Pub details", @@ -96,13 +97,13 @@ public class DR_PubResource extends BaseLoggingClass { logger.info( "Entry: POST /dr_pubs"); try { - resp.required( "feedId", pub.getFeedId(), ""); + checker.required( "feedId", pub.getFeedId()); } catch ( RequiredFieldException rfe ) { try { - resp.required( "feedName", pub.getFeedName(), ""); + checker.required( "feedName", pub.getFeedName()); }catch ( RequiredFieldException rfe2 ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe2.getApiError().toString() ); + return responseBuilder.error(rfe2.getApiError()); } // if we found a FeedName instead of a FeedId then try to look it up. List nfeeds = feeds.getAllFeeds( pub.getFeedName(), pub.getFeedVersion(), "equals"); @@ -113,10 +114,10 @@ public class DR_PubResource extends BaseLoggingClass { fnew = nfeeds.get(0); } try { - resp.required( "dcaeLocationName", pub.getDcaeLocationName(), ""); + checker.required( "dcaeLocationName", pub.getDcaeLocationName()); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.getErr().toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } @@ -189,9 +190,9 @@ public class DR_PubResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "pubId", id, ""); + checker.required( "pubId", id); } catch ( RequiredFieldException rfe ) { - return responseBuilder.error(resp.getErr()); + return responseBuilder.error(rfe.getApiError()); } DR_Pub pub = dr_pubService.getDr_Pub( id, resp.getErr() ); @@ -245,9 +246,9 @@ public class DR_PubResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "feedId", id, ""); + checker.required( "feedId", id); } catch ( RequiredFieldException rfe ) { - return responseBuilder.error(resp.getErr()); + return responseBuilder.error(rfe.getApiError()); } DR_Pub pub = dr_pubService.getDr_Pub( id, resp.getErr() ); diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/DR_SubResource.java b/src/main/java/org/onap/dmaap/dbcapi/resources/DR_SubResource.java index 01ac059..b74d5a1 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/resources/DR_SubResource.java +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/DR_SubResource.java @@ -62,6 +62,7 @@ import static javax.ws.rs.core.Response.Status.CREATED; public class DR_SubResource extends BaseLoggingClass { private ResponseBuilder responseBuilder = new ResponseBuilder(); + private RequiredChecker checker = new RequiredChecker(); @GET @ApiOperation( value = "return DR_Sub details", @@ -96,13 +97,13 @@ public class DR_SubResource extends BaseLoggingClass { FeedService feeds = new FeedService(); Feed fnew = null; try { - resp.required( "feedId", sub.getFeedId(), ""); + checker.required( "feedId", sub.getFeedId()); } catch ( RequiredFieldException rfe ) { try { - resp.required( "feedName", sub.getFeedName(), ""); + checker.required( "feedName", sub.getFeedName()); }catch ( RequiredFieldException rfe2 ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe2.getApiError().toString() ); + return responseBuilder.error(rfe2.getApiError()); } // if we found a FeedName instead of a FeedId then try to look it up. List nfeeds = feeds.getAllFeeds( sub.getFeedName(), sub.getFeedVersion(), "equals"); @@ -114,10 +115,10 @@ public class DR_SubResource extends BaseLoggingClass { } try { - resp.required( "dcaeLocationName", sub.getDcaeLocationName(), ""); + checker.required( "dcaeLocationName", sub.getDcaeLocationName()); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } // we may have fnew already if located by FeedName if ( fnew == null ) { @@ -161,13 +162,13 @@ public class DR_SubResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "subId", name, ""); - resp.required( "feedId", sub.getFeedId(), ""); - resp.required( "dcaeLocationName", sub.getDcaeLocationName(), ""); + checker.required( "subId", name); + checker.required( "feedId", sub.getFeedId()); + checker.required( "dcaeLocationName", sub.getDcaeLocationName()); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } FeedService feeds = new FeedService(); Feed fnew = feeds.getFeed( sub.getFeedId(), resp.getErr() ); @@ -201,10 +202,10 @@ public class DR_SubResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "subId", id, ""); + checker.required( "subId", id); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } DR_SubService dr_subService = new DR_SubService(); dr_subService.removeDr_Sub(id, resp.getErr() ); @@ -229,10 +230,10 @@ public class DR_SubResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "subId", id, ""); + checker.required( "subId", id); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } DR_SubService dr_subService = new DR_SubService(); DR_Sub sub = dr_subService.getDr_Sub( id, resp.getErr() ); diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/DmaapResource.java b/src/main/java/org/onap/dmaap/dbcapi/resources/DmaapResource.java index d7fb507..955cab7 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/resources/DmaapResource.java +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/DmaapResource.java @@ -7,9 +7,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -24,11 +24,14 @@ package org.onap.dmaap.dbcapi.resources; - import io.swagger.annotations.Api; import io.swagger.annotations.ApiOperation; import io.swagger.annotations.ApiResponse; import io.swagger.annotations.ApiResponses; +import org.onap.dmaap.dbcapi.logging.BaseLoggingClass; +import org.onap.dmaap.dbcapi.model.ApiError; +import org.onap.dmaap.dbcapi.model.Dmaap; +import org.onap.dmaap.dbcapi.service.DmaapService; import javax.ws.rs.Consumes; import javax.ws.rs.GET; @@ -41,13 +44,6 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.UriInfo; -import org.onap.dmaap.dbcapi.logging.BaseLoggingClass; -import org.onap.dmaap.dbcapi.model.ApiError; -import org.onap.dmaap.dbcapi.model.Dmaap; -import org.onap.dmaap.dbcapi.service.ApiService; -import org.onap.dmaap.dbcapi.service.DmaapService; - - @Path("/dmaap") @Api( value= "dmaap", description = "Endpoint for this instance of DMaaP object containing values for this OpenDCAE deployment" ) @@ -59,7 +55,8 @@ public class DmaapResource extends BaseLoggingClass { private DmaapService dmaapService = new DmaapService(); private ResponseBuilder responseBuilder = new ResponseBuilder(); - + private RequiredChecker checker = new RequiredChecker(); + @GET @ApiOperation( value = "return dmaap details", notes = "returns the `dmaap` object, which contains system wide configuration settings", response = Dmaap.class) @ApiResponses( value = { @@ -71,7 +68,7 @@ public class DmaapResource extends BaseLoggingClass { Dmaap d = dmaapService.getDmaap(); return responseBuilder.success(d); } - + @POST @ApiOperation( value = "return dmaap details", notes = "Create a new DMaaP set system wide configuration settings for the *dcaeEnvironment*. Deprecated with introduction of persistence in 1610.", response = Dmaap.class) @ApiResponses( value = { @@ -79,26 +76,22 @@ public class DmaapResource extends BaseLoggingClass { @ApiResponse( code = 400, message = "Error", response = ApiError.class ) }) public Response addDmaap( Dmaap obj ) { - ApiService check = new ApiService(); - try { //check for required fields - check.required( "dmaapName", obj.getDmaapName(), "^\\S+$" ); //no white space allowed in dmaapName - check.required( "dmaapProvUrl", obj.getDrProvUrl(), "" ); - check.required( "topicNsRoot", obj.getTopicNsRoot(), "" ); - check.required( "bridgeAdminTopic", obj.getBridgeAdminTopic(), "" ); + try { + validateRequiredFields(obj); } catch( RequiredFieldException rfe ) { - return responseBuilder.error(check.getErr()); + return responseBuilder.error(rfe.getApiError()); } - + Dmaap d = dmaapService.addDmaap(obj); if ( d == null ) { return responseBuilder.notFound(); - } + } return responseBuilder.success(d); } - + @PUT @ApiOperation( value = "return dmaap details", notes = "Update system settings for *dcaeEnvironment*.", response = Dmaap.class) @ApiResponses( value = { @@ -106,23 +99,25 @@ public class DmaapResource extends BaseLoggingClass { @ApiResponse( code = 400, message = "Error", response = ApiError.class ) }) public Response updateDmaap( Dmaap obj ) { - ApiService check = new ApiService(); - try { //check for required fields - check.required( "dmaapName", obj.getDmaapName(), "^\\S+$" ); //no white space allowed in dmaapName - check.required( "dmaapProvUrl", obj.getDrProvUrl(), "" ); - check.required( "topicNsRoot", obj.getTopicNsRoot(), "" ); - check.required( "bridgeAdminTopic", obj.getBridgeAdminTopic(), "" ); + try { + validateRequiredFields(obj); } catch( RequiredFieldException rfe ) { - return responseBuilder.error(check.getErr()); + return responseBuilder.error(rfe.getApiError()); } + Dmaap d = dmaapService.updateDmaap(obj); if ( d != null ) { return responseBuilder.success(d); } else { return responseBuilder.notFound(); - } + } + } + + private void validateRequiredFields(Dmaap obj) throws RequiredFieldException { + checker.required( "dmaapName", obj.getDmaapName(), "^\\S+$" ); //no white space allowed in dmaapName + checker.required( "dmaapProvUrl", obj.getDrProvUrl()); + checker.required( "topicNsRoot", obj.getTopicNsRoot()); + checker.required( "bridgeAdminTopic", obj.getBridgeAdminTopic()); } - - } diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/FeedResource.java b/src/main/java/org/onap/dmaap/dbcapi/resources/FeedResource.java index 382d88c..86a79f2 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/resources/FeedResource.java +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/FeedResource.java @@ -58,6 +58,7 @@ import org.onap.dmaap.dbcapi.service.FeedService; public class FeedResource extends BaseLoggingClass { private ResponseBuilder responseBuilder = new ResponseBuilder(); + private RequiredChecker checker = new RequiredChecker(); @GET @ApiOperation( value = "return Feed details", @@ -96,13 +97,13 @@ public class FeedResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "feedName", feed.getFeedName(), ""); - resp.required( "feedVersion", feed.getFeedVersion(), ""); - resp.required( "owner", feed.getOwner(), "" ); - resp.required( "asprClassification", feed.getAsprClassification(), "" ); + checker.required( "feedName", feed.getFeedName()); + checker.required( "feedVersion", feed.getFeedVersion()); + checker.required( "owner", feed.getOwner()); + checker.required( "asprClassification", feed.getAsprClassification()); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } @@ -152,10 +153,10 @@ public class FeedResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "feedId", id, ""); + checker.required( "feedId", id); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } Feed nfeed = feedService.getFeed( id, resp.getErr() ); diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/MR_ClientResource.java b/src/main/java/org/onap/dmaap/dbcapi/resources/MR_ClientResource.java index a67ac8f..a65007d 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/resources/MR_ClientResource.java +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/MR_ClientResource.java @@ -62,6 +62,7 @@ public class MR_ClientResource extends BaseLoggingClass { private MR_ClientService mr_clientService = new MR_ClientService(); private ResponseBuilder responseBuilder = new ResponseBuilder(); + private RequiredChecker checker = new RequiredChecker(); @GET @ApiOperation( value = "return MR_Client details", @@ -95,18 +96,18 @@ public class MR_ClientResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "fqtn", client.getFqtn(), ""); - resp.required( "dcaeLocationName", client.getDcaeLocationName(), ""); + checker.required( "fqtn", client.getFqtn()); + checker.required( "dcaeLocationName", client.getDcaeLocationName()); String s = client.getClientRole(); if ( s == null ) { s = client.getClientIdentity(); } - resp.required( "clientRole or clientIdentity", s, "" ); - resp.required( "action", client.getAction(), ""); + checker.required( "clientRole or clientIdentity", s); + checker.required( "action", client.getAction()); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } MR_ClusterService clusters = new MR_ClusterService(); @@ -161,14 +162,14 @@ public class MR_ClientResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "fqtn", client.getFqtn(), ""); - resp.required( "dcaeLocationName", client.getDcaeLocationName(), ""); - resp.required( "clientRole", client.getClientRole(), "" ); - resp.required( "action", client.getAction(), ""); + checker.required( "fqtn", client.getFqtn()); + checker.required( "dcaeLocationName", client.getDcaeLocationName()); + checker.required( "clientRole", client.getClientRole()); + checker.required( "action", client.getAction()); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } client.setMrClientId(clientId); MR_Client nClient = mr_clientService.updateMr_Client(client, resp.getErr() ); @@ -196,10 +197,10 @@ public class MR_ClientResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "clientId", id, ""); + checker.required( "clientId", id); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } mr_clientService.removeMr_Client(id, true, resp.getErr() ); if ( resp.getErr().is2xx()) { @@ -224,10 +225,10 @@ public class MR_ClientResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "clientId", id, ""); + checker.required( "clientId", id); } catch ( RequiredFieldException rfe ) { - logger.debug( resp.toString() ); - return responseBuilder.error(resp.getErr()); + logger.debug( rfe.getApiError().toString() ); + return responseBuilder.error(rfe.getApiError()); } MR_Client nClient = mr_clientService.getMr_Client( id, resp.getErr() ); if ( resp.getErr().is2xx()) { diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/MR_ClusterResource.java b/src/main/java/org/onap/dmaap/dbcapi/resources/MR_ClusterResource.java index 5d2d379..598fcc2 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/resources/MR_ClusterResource.java +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/MR_ClusterResource.java @@ -56,6 +56,7 @@ public class MR_ClusterResource extends BaseLoggingClass { private MR_ClusterService mr_clusterService = new MR_ClusterService(); private ResponseBuilder responseBuilder = new ResponseBuilder(); + private RequiredChecker checker = new RequiredChecker(); @GET @ApiOperation( value = "return MR_Cluster details", @@ -86,10 +87,10 @@ public class MR_ClusterResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "dcaeLocationName", cluster.getDcaeLocationName(), "" ); - resp.required( "fqdn", cluster.getFqdn(), "" ); + checker.required( "dcaeLocationName", cluster.getDcaeLocationName()); + checker.required( "fqdn", cluster.getFqdn()); } catch( RequiredFieldException rfe ) { - return responseBuilder.error(resp.getErr()); + return responseBuilder.error(rfe.getApiError()); } MR_Cluster mrc = mr_clusterService.addMr_Cluster(cluster, resp.getErr() ); if ( mrc != null && mrc.isStatusValid() ) { @@ -115,10 +116,10 @@ public class MR_ClusterResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "fqdn", clusterId, "" ); - resp.required( "dcaeLocationName", cluster.getDcaeLocationName(), "" ); + checker.required( "fqdn", clusterId); + checker.required( "dcaeLocationName", cluster.getDcaeLocationName()); } catch( RequiredFieldException rfe ) { - return responseBuilder.error(resp.getErr()); + return responseBuilder.error(rfe.getApiError()); } cluster.setDcaeLocationName(clusterId); MR_Cluster mrc = mr_clusterService.updateMr_Cluster(cluster, resp.getErr() ); @@ -143,9 +144,9 @@ public class MR_ClusterResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "fqdn", id, "" ); + checker.required( "fqdn", id); } catch( RequiredFieldException rfe ) { - return responseBuilder.error(resp.getErr()); + return responseBuilder.error(rfe.getApiError()); } mr_clusterService.removeMr_Cluster(id, resp.getErr() ); if ( resp.getErr().is2xx()) { @@ -169,9 +170,9 @@ public class MR_ClusterResource extends BaseLoggingClass { ApiService resp = new ApiService(); try { - resp.required( "dcaeLocationName", id, "" ); + checker.required( "dcaeLocationName", id); } catch( RequiredFieldException rfe ) { - return responseBuilder.error(resp.getErr()); + return responseBuilder.error(rfe.getApiError()); } MR_Cluster mrc = mr_clusterService.getMr_Cluster( id, resp.getErr() ); if ( mrc != null && mrc.isStatusValid() ) { diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/RequiredChecker.java b/src/main/java/org/onap/dmaap/dbcapi/resources/RequiredChecker.java new file mode 100644 index 0000000..36f0215 --- /dev/null +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/RequiredChecker.java @@ -0,0 +1,53 @@ +/* + * ============LICENSE_START======================================================= + * PNF-REGISTRATION-HANDLER + * ================================================================================ + * Copyright (C) 2019 NOKIA Intellectual Property. All rights reserved. + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ============LICENSE_END========================================================= + */ + +package org.onap.dmaap.dbcapi.resources; + +import org.onap.dmaap.dbcapi.model.ApiError; + +import java.util.regex.Matcher; +import java.util.regex.Pattern; + +import static javax.ws.rs.core.Response.Status.BAD_REQUEST; + +public class RequiredChecker { + + public void required(String name, Object val) throws RequiredFieldException { + if (val == null) { + throw new RequiredFieldException(new ApiError(BAD_REQUEST.getStatusCode(), + "missing required field", name)); + } + } + + public void required(String name, String val, String expr) throws RequiredFieldException { + + required(name, val); + + if (expr != null && !expr.isEmpty()) { + Pattern pattern = Pattern.compile(expr); + Matcher matcher = pattern.matcher(val); + if (!matcher.find()) { + throw new RequiredFieldException(new ApiError(BAD_REQUEST.getStatusCode(), + "value '" + val + "' violates regexp check '" + expr + "'", name)); + } + } + } + +} diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/RequiredFieldException.java b/src/main/java/org/onap/dmaap/dbcapi/resources/RequiredFieldException.java index 74af356..2968d18 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/resources/RequiredFieldException.java +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/RequiredFieldException.java @@ -7,9 +7,9 @@ * Licensed under the Apache License, Version 2.0 (the "License"); * you may not use this file except in compliance with the License. * You may obtain a copy of the License at - * + * * http://www.apache.org/licenses/LICENSE-2.0 - * + * * Unless required by applicable law or agreed to in writing, software * distributed under the License is distributed on an "AS IS" BASIS, * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. @@ -20,11 +20,27 @@ package org.onap.dmaap.dbcapi.resources; +import org.onap.dmaap.dbcapi.model.ApiError; + public class RequiredFieldException extends Exception { - /** - * - */ - private static final long serialVersionUID = 1L; + private static final long serialVersionUID = 2L; + + private final ApiError apiError; + + public RequiredFieldException(ApiError apiError) { + super(); + this.apiError = apiError; + } + + public ApiError getApiError() { + return apiError; + } + @Override + public String toString() { + return "RequiredFieldException{" + + "apiError=" + apiError + + '}'; + } } diff --git a/src/main/java/org/onap/dmaap/dbcapi/resources/TopicResource.java b/src/main/java/org/onap/dmaap/dbcapi/resources/TopicResource.java index 4f442c9..96b6a4a 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/resources/TopicResource.java +++ b/src/main/java/org/onap/dmaap/dbcapi/resources/TopicResource.java @@ -63,6 +63,7 @@ public class TopicResource extends BaseLoggingClass { private static String defaultReplicationCount; private TopicService mr_topicService = new TopicService(); private ResponseBuilder responseBuilder = new ResponseBuilder(); + private RequiredChecker checker = new RequiredChecker(); public TopicResource() { DmaapConfig p = (DmaapConfig)DmaapConfig.getConfig(); @@ -109,12 +110,12 @@ public class TopicResource extends BaseLoggingClass { ApiService check = new ApiService(); try { - check.required( "topicName", topic.getTopicName(), "^\\S+$" ); //no white space allowed in topicName - check.required( "topicDescription", topic.getTopicDescription(), "" ); - check.required( "owner", topic.getOwner(), "" ); + checker.required( "topicName", topic.getTopicName(), "^\\S+$" ); //no white space allowed in topicName + checker.required( "topicDescription", topic.getTopicDescription()); + checker.required( "owner", topic.getOwner()); } catch( RequiredFieldException rfe ) { - logger.error("Error", rfe); - return responseBuilder.error(check.getErr()); + logger.error("Error", rfe.getApiError()); + return responseBuilder.error(rfe.getApiError()); } ReplicationType t = topic.getReplicationCase(); @@ -182,10 +183,10 @@ public class TopicResource extends BaseLoggingClass { ApiService check = new ApiService(); try { - check.required( "fqtn", id, "" ); + checker.required( "fqtn", id); } catch( RequiredFieldException rfe ) { - logger.error("Error", rfe); - return responseBuilder.error(check.getErr()); + logger.error("Error", rfe.getApiError()); + return responseBuilder.error(rfe.getApiError()); } mr_topicService.removeTopic(id, check.getErr()); @@ -212,10 +213,10 @@ public class TopicResource extends BaseLoggingClass { ApiService check = new ApiService(); try { - check.required( "topicName", id, "^\\S+$" ); //no white space allowed in topicName + checker.required( "topicName", id, "^\\S+$" ); //no white space allowed in topicName } catch( RequiredFieldException rfe ) { - logger.error("Error", rfe); - return responseBuilder.error(check.getErr()); + logger.error("Error", rfe.getApiError()); + return responseBuilder.error(rfe.getApiError()); } Topic mrc = mr_topicService.getTopic( id, check.getErr() ); if ( mrc == null ) { diff --git a/src/main/java/org/onap/dmaap/dbcapi/service/ApiService.java b/src/main/java/org/onap/dmaap/dbcapi/service/ApiService.java index b2eee6f..35e61db 100644 --- a/src/main/java/org/onap/dmaap/dbcapi/service/ApiService.java +++ b/src/main/java/org/onap/dmaap/dbcapi/service/ApiService.java @@ -24,9 +24,6 @@ import static com.att.eelf.configuration.Configuration.MDC_KEY_REQUEST_ID; import static com.att.eelf.configuration.Configuration.MDC_PARTNER_NAME; import static com.att.eelf.configuration.Configuration.MDC_SERVICE_NAME; -import java.util.regex.Matcher; -import java.util.regex.Pattern; -import javax.ws.rs.core.Response.Status; import javax.xml.bind.DatatypeConverter; import org.onap.dmaap.dbcapi.aaf.DmaapPerm; import org.onap.dmaap.dbcapi.authentication.ApiPolicy; @@ -34,7 +31,6 @@ import org.onap.dmaap.dbcapi.authentication.AuthenticationErrorException; import org.onap.dmaap.dbcapi.logging.BaseLoggingClass; import org.onap.dmaap.dbcapi.model.ApiError; import org.onap.dmaap.dbcapi.model.Dmaap; -import org.onap.dmaap.dbcapi.resources.RequiredFieldException; import org.onap.dmaap.dbcapi.util.DmaapConfig; import org.onap.dmaap.dbcapi.util.RandomString; import org.slf4j.MDC; @@ -109,37 +105,9 @@ public class ApiService extends BaseLoggingClass { this.err = err; } - - // test for presence of a required field - public void required(String name, Object val, String expr) throws RequiredFieldException { - err.setCode(0); - if (val == null) { - err.setCode(Status.BAD_REQUEST.getStatusCode()); - err.setMessage("missing required field"); - err.setFields(name); - throw new RequiredFieldException(); - } - if (expr != null && !expr.isEmpty()) { - Pattern pattern = Pattern.compile(expr); - Matcher matcher = pattern.matcher((CharSequence) val); - if (!matcher.find()) { - err.setCode(Status.BAD_REQUEST.getStatusCode()); - err.setMessage("value '" + val + "' violates regexp check '" + expr + "'"); - err.setFields(name); - throw new RequiredFieldException(); - } - } - } - - // utility to serialize ApiErr object - public String toString() { - return String.format("code=%d msg=%s fields=%s", err.getCode(), err.getMessage(), err.getFields()); - } - - - public void setCode(int statusCode) { - err.setCode(statusCode); - } + public void setCode(int statusCode) { + err.setCode(statusCode); + } public void setMessage(String string) { @@ -147,21 +115,11 @@ public class ApiService extends BaseLoggingClass { } - public void setFields(String string) { - err.setFields(string); - } + public void setFields(String string) { + err.setFields(string); + } - public void checkAuthorization(String auth, String uriPath, String httpMethod) - throws AuthenticationErrorException, Exception { - authorization = auth; - setUriFromPath(uriPath); - method = httpMethod; - - checkAuthorization(); - } - - - public void checkAuthorization() throws AuthenticationErrorException, Exception { + public void checkAuthorization() throws Exception { MDC.put(MDC_KEY_REQUEST_ID, requestId); @@ -217,12 +175,6 @@ public class ApiService extends BaseLoggingClass { throw ae; } - - - } - - public String getRequestId() { - return requestId; } public ApiService setRequestId(String requestId) { @@ -236,3 +188,4 @@ public class ApiService extends BaseLoggingClass { return this; } } + diff --git a/src/test/java/org/onap/dmaap/dbcapi/resources/RequiredCheckerTest.java b/src/test/java/org/onap/dmaap/dbcapi/resources/RequiredCheckerTest.java new file mode 100644 index 0000000..f07058b --- /dev/null +++ b/src/test/java/org/onap/dmaap/dbcapi/resources/RequiredCheckerTest.java @@ -0,0 +1,86 @@ +/* + * ============LICENSE_START======================================================= + * PNF-REGISTRATION-HANDLER + * ================================================================================ + * Copyright (C) 2019 NOKIA Intellectual Property. All rights reserved. + * ================================================================================ + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + * ============LICENSE_END========================================================= + */ + +package org.onap.dmaap.dbcapi.resources; + +import org.hamcrest.BaseMatcher; +import org.hamcrest.Description; +import org.junit.Rule; +import org.junit.Test; +import org.junit.rules.ExpectedException; +import org.onap.dmaap.dbcapi.model.ApiError; + +import static javax.ws.rs.core.Response.Status.BAD_REQUEST; +import static org.junit.Assert.fail; + +public class RequiredCheckerTest { + + private static final String NAME = "field_name"; + @Rule + public ExpectedException thrown = ExpectedException.none(); + private RequiredChecker requiredChecker = new RequiredChecker(); + + + @Test + public void required_shouldThrowExceptionWhenObjectIsNull() throws RequiredFieldException { + thrown.expect(RequiredFieldException.class); + thrown.expect(new ApiErrorMatcher(new ApiError(BAD_REQUEST.getStatusCode(), + "missing required field", NAME))); + + requiredChecker.required(NAME, null); + } + + @Test + public void required_shouldThrowExceptionWhenRegexValidationFailed() throws RequiredFieldException { + thrown.expect(RequiredFieldException.class); + thrown.expect(new ApiErrorMatcher(new ApiError(BAD_REQUEST.getStatusCode(), + "value 'with white space' violates regexp check '^\\S+$'", NAME))); + + requiredChecker.required(NAME, "with white space", "^\\S+$"); + } + + @Test + public void required_shouldPassValidation() { + try { + requiredChecker.required(NAME, "value", "^\\S+$"); + } catch (RequiredFieldException e) { + fail("No exception should be thrown"); + } + } + + class ApiErrorMatcher extends BaseMatcher { + + private final ApiError expectedApiEror; + + ApiErrorMatcher(ApiError expectedApiEror) { + this.expectedApiEror = expectedApiEror; + } + + @Override + public boolean matches(Object exception) { + return expectedApiEror.equals(((RequiredFieldException) exception).getApiError()); + } + + @Override + public void describeTo(Description description) { + description.appendText("Following ApiError is expected: ").appendValue(expectedApiEror); + } + } +} \ No newline at end of file diff --git a/src/test/java/org/onap/dmaap/dbcapi/service/ApiServiceTest.java b/src/test/java/org/onap/dmaap/dbcapi/service/ApiServiceTest.java index 33cce9d..dccfadf 100644 --- a/src/test/java/org/onap/dmaap/dbcapi/service/ApiServiceTest.java +++ b/src/test/java/org/onap/dmaap/dbcapi/service/ApiServiceTest.java @@ -59,22 +59,4 @@ public class ApiServiceTest { rh.reflect( "org.onap.dmaap.dbcapi.service.ApiService", "set", v ); } - - @Test - public void test3() { - ApiService nd = new ApiService(); - nd.setAuth( "auth" ); - try { - nd.required( "aName", null, "anExpr" ); - } catch ( RequiredFieldException rfe ) { - } - try { - nd.checkAuthorization( "authval", "/uri/Path", "GET" ); - nd.checkAuthorization(); - } catch ( AuthenticationErrorException aee ) { - } catch ( Exception e ) { - } - } - - } -- 2.16.6