From 75093a89a3c0dd71421320ac997b629d2ccace41 Mon Sep 17 00:00:00 2001 From: oscardssmith Date: Thu, 29 Jun 2017 10:17:28 -0400 Subject: [PATCH] Maybe fixes merge conflicts --- .../iq/dataverse/api/AbstractApiBean.java | 159 +++++++++--------- .../edu/harvard/iq/dataverse/api/Groups.java | 45 ++--- .../edu/harvard/iq/dataverse/api/Util.java | 37 ++-- .../iq/dataverse/api/AbstractApiBeanTest.java | 7 - 4 files changed, 116 insertions(+), 132 deletions(-) diff --git a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java index 6d3941d876e..662ef4b7394 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/AbstractApiBean.java @@ -60,13 +60,14 @@ import javax.ws.rs.core.MediaType; import javax.ws.rs.core.Response; import javax.ws.rs.core.Response.Status; +import static org.apache.commons.lang.StringUtils.isNumeric; /** * Base class for API beans * @author michael */ public abstract class AbstractApiBean { - + private static final Logger logger = Logger.getLogger(AbstractApiBean.class.getName()); private static final String DATAVERSE_KEY_HEADER_NAME = "X-Dataverse-key"; public static final String STATUS_ERROR = "ERROR"; @@ -77,20 +78,20 @@ public abstract class AbstractApiBean { */ public static class WrappedResponse extends Exception { private final Response response; - + public WrappedResponse(Response response) { this.response = response; } - + public WrappedResponse( Throwable cause, Response response ) { super( cause ); this.response = response; } - + public Response getResponse() { return response; } - + /** * Creates a new response, based on the original response and the passed message. * Typical use would be to add a better error message to the HTTP response. @@ -100,14 +101,14 @@ public Response getResponse() { public Response refineResponse( String message ) { final Status statusCode = Response.Status.fromStatusCode(response.getStatus()); String baseMessage = getWrappedMessageWhenJson(); - + if ( baseMessage == null ) { final Throwable cause = getCause(); baseMessage = (cause!=null ? cause.getMessage() : ""); } return error(statusCode, message+" "+baseMessage); } - + /** * In the common case of the wrapped response being of type JSON, * return the message field it has (if any). @@ -117,7 +118,7 @@ String getWrappedMessageWhenJson() { if ( response.getMediaType().equals(MediaType.APPLICATION_JSON_TYPE) ) { Object entity = response.getEntity(); if ( entity == null ) return null; - + String json = entity.toString(); try ( StringReader rdr = new StringReader(json) ){ JsonReader jrdr = Json.createReader(rdr); @@ -134,43 +135,43 @@ String getWrappedMessageWhenJson() { } } } - + @EJB protected EjbDataverseEngine engineSvc; - + @EJB protected DatasetServiceBean datasetSvc; - + @EJB protected DataverseServiceBean dataverseSvc; - - @EJB + + @EJB protected AuthenticationServiceBean authSvc; - + @EJB protected DatasetFieldServiceBean datasetFieldSvc; - + @EJB protected MetadataBlockServiceBean metadataBlockSvc; - + @EJB protected UserServiceBean userSvc; - + @EJB protected DataverseRoleServiceBean rolesSvc; - + @EJB protected SettingsServiceBean settingsSvc; - + @EJB protected RoleAssigneeServiceBean roleAssigneeSvc; - + @EJB protected PermissionServiceBean permissionSvc; - + @EJB protected GroupServiceBean groupSvc; - + @EJB protected ActionLogServiceBean actionLogSvc; @@ -203,7 +204,7 @@ String getWrappedMessageWhenJson() { @Context protected HttpServletRequest httpRequest; - + /** * For pretty printing (indenting) of JSON output. */ @@ -218,30 +219,26 @@ public JsonParser call() throws Exception { return new JsonParser(datasetFieldSvc, metadataBlockSvc,settingsSvc); } }); - + /** * Functional interface for handling HTTP requests in the APIs. - * + * * @see #response(edu.harvard.iq.dataverse.api.AbstractApiBean.DataverseRequestHandler) */ protected static interface DataverseRequestHandler { - Response handle( DataverseRequest u ) throws WrappedResponse; + Response handle( DataverseRequest u ) throws WrappedResponse; } - - + + /* ===================== *\ * Utility Methods * * Get that DSL feelin' * \* ===================== */ - + protected JsonParser jsonParser() { return jsonParserRef.get(); } - - protected boolean isNumeric( String str ) { - return Util.isNumeric(str); - } - + protected boolean parseBooleanOrDie( String input ) throws WrappedResponse { if (input == null ) throw new WrappedResponse( badRequest("Boolean value missing")); input = input.trim(); @@ -250,8 +247,8 @@ protected boolean parseBooleanOrDie( String input ) throws WrappedResponse { } else { throw new WrappedResponse( badRequest("Illegal boolean value '" + input + "'")); } - } - + } + /** * Returns the {@code key} query parameter from the current request, or {@code null} if * the request has no such parameter. @@ -261,13 +258,13 @@ protected boolean parseBooleanOrDie( String input ) throws WrappedResponse { protected String getRequestParameter( String key ) { return httpRequest.getParameter(key); } - + protected String getRequestApiKey() { String headerParamApiKey = httpRequest.getHeader(DATAVERSE_KEY_HEADER_NAME); String queryParamApiKey = httpRequest.getParameter("key"); return headerParamApiKey!=null ? headerParamApiKey : queryParamApiKey; } - + /* ========= *\ * Finders * \* ========= */ @@ -286,15 +283,15 @@ protected RoleAssignee findAssignee(String identifier) { } /** - * + * * @param apiKey the key to find the user with * @return the user, or null - * @see #findUserOrDie(java.lang.String) + * @see #findUserOrDie(java.lang.String) */ protected AuthenticatedUser findUserByApiToken( String apiKey ) { return authSvc.lookupUser(apiKey); } - + /** * Returns the user of pointed by the API key, or the guest user * @return a user, may be a guest user. @@ -311,35 +308,35 @@ protected User findUserOrDie() throws WrappedResponse { } return findAuthenticatedUserOrDie(requestApiKey); } - + /** * Finds the authenticated user, based on (in order): *
    *
  1. The key in the HTTP header {@link #DATAVERSE_KEY_HEADER_NAME}
    2. *
  2. The key in the query parameter {@code key} *
- * + * * If no user is found, throws a wrapped bad api key (HTTP UNAUTHORIZED) response. - * + * * @return The authenticated user which owns the passed api key * @throws edu.harvard.iq.dataverse.api.AbstractApiBean.WrappedResponse in case said user is not found. */ protected AuthenticatedUser findAuthenticatedUserOrDie() throws WrappedResponse { return findAuthenticatedUserOrDie(getRequestApiKey()); } - - + + private AuthenticatedUser findAuthenticatedUserOrDie( String key ) throws WrappedResponse { AuthenticatedUser authUser = authSvc.lookupUser(key); if ( authUser != null ) { System.out.println("Updating lastApiUseTime for authenticated user via abstractapibean"); authUser = userSvc.updateLastApiUseTime(authUser); - + return authUser; } throw new WrappedResponse( badApiKey(key) ); } - + protected Dataverse findDataverseOrDie( String dvIdtf ) throws WrappedResponse { Dataverse dv = findDataverse(dvIdtf); if ( dv == null ) { @@ -347,22 +344,22 @@ protected Dataverse findDataverseOrDie( String dvIdtf ) throws WrappedResponse { } return dv; } - + protected DataverseRequest createDataverseRequest( User u ) { return new DataverseRequest(u, httpRequest); } - + protected Dataverse findDataverse( String idtf ) { return isNumeric(idtf) ? dataverseSvc.find(Long.parseLong(idtf)) : dataverseSvc.findByAlias(idtf); } - + protected DvObject findDvo( Long id ) { return em.createNamedQuery("DvObject.findById", DvObject.class) .setParameter("id", id) .getSingleResult(); } - + /** * Tries to find a DvObject. If the passed id can be interpreted as a number, * it tries to get the DvObject by its id. Else, it tries to get a {@link Dataverse} @@ -377,61 +374,61 @@ protected DvObject findDvo( String id ) { Dataverse d = dataverseSvc.findByAlias(id); return ( d != null ) ? d : datasetSvc.findByGlobalId(id); - + } } - + protected T failIfNull( T t, String errorMessage ) throws WrappedResponse { if ( t != null ) return t; throw new WrappedResponse( error( Response.Status.BAD_REQUEST,errorMessage) ); } - + protected MetadataBlock findMetadataBlock(Long id) { return metadataBlockSvc.findById(id); } protected MetadataBlock findMetadataBlock(String idtf) throws NumberFormatException { return metadataBlockSvc.findByName(idtf); } - + protected DatasetFieldType findDatasetFieldType(String idtf) throws NumberFormatException { return isNumeric(idtf) ? datasetFieldSvc.find(Long.parseLong(idtf)) : datasetFieldSvc.findByNameOpt(idtf); - } - + } + /* =================== *\ * Command Execution * \* =================== */ - + /** * Executes a command, and returns the appropriate result/HTTP response. * @param Return type for the command * @param cmd The command to execute. * @return Value from the command * @throws edu.harvard.iq.dataverse.api.AbstractApiBean.WrappedResponse Unwrap and return. - * @see #response(java.util.concurrent.Callable) + * @see #response(java.util.concurrent.Callable) */ protected T execCommand( Command cmd ) throws WrappedResponse { try { return engineSvc.submit(cmd); - + } catch (IllegalCommandException ex) { throw new WrappedResponse( ex, error(Response.Status.FORBIDDEN, ex.getMessage() ) ); - + } catch (PermissionException ex) { /** * @todo Is there any harm in exposing ex.getLocalizedMessage()? * There's valuable information in there that can help people reason * about permissions! */ - throw new WrappedResponse(error(Response.Status.UNAUTHORIZED, + throw new WrappedResponse(error(Response.Status.UNAUTHORIZED, "User " + cmd.getRequest().getUser().getIdentifier() + " is not permitted to perform requested action.") ); - + } catch (CommandException ex) { Logger.getLogger(AbstractApiBean.class.getName()).log(Level.SEVERE, "Error while executing command " + cmd, ex); throw new WrappedResponse(ex, error(Status.INTERNAL_SERVER_ERROR, ex.getMessage())); } } - + /** * A syntactically nicer way of using {@link #execCommand(edu.harvard.iq.dataverse.engine.command.Command)}. * @param hdl The block to run. @@ -455,17 +452,17 @@ protected Response response( Callable hdl ) { .type("application/json").build(); } } - + /** * The preferred way of handling a request that requires a user. The system * looks for the user and, if found, handles it to the handler for doing the * actual work. - * + * * This is a relatively secure way to handle things, since if the user is not * found, the response is about the bad API key, rather than something else - * (say, 404 NOT FOUND which leaks information about the existence of the + * (say, 404 NOT FOUND which leaks information about the existence of the * sought object). - * + * * @param hdl handling code block. * @return HTTP Response appropriate for the way {@code hdl} executed. */ @@ -487,11 +484,11 @@ protected Response response( DataverseRequestHandler hdl ) { .type("application/json").build(); } } - + /* ====================== *\ * HTTP Response methods * \* ====================== */ - + protected Response ok( JsonArrayBuilder bld ) { return Response.ok(Json.createObjectBuilder() .add("status", STATUS_OK) @@ -537,26 +534,26 @@ protected Response created( String uri, JsonObjectBuilder bld ) { .type(MediaType.APPLICATION_JSON) .build(); } - + protected Response accepted() { return Response.accepted() .entity(Json.createObjectBuilder() .add("status", STATUS_OK).build() ).build(); } - + protected Response notFound( String msg ) { return error(Status.NOT_FOUND, msg); } - + protected Response badRequest( String msg ) { return error( Status.BAD_REQUEST, msg ); } - + protected Response badApiKey( String apiKey ) { return error(Status.UNAUTHORIZED, (apiKey != null ) ? "Bad api key '" + apiKey +"'" : "Please provide a key query parameter (?key=XXX) or via the HTTP header " + DATAVERSE_KEY_HEADER_NAME ); } - + protected Response permissionError( PermissionException pe ) { return permissionError( pe.getMessage() ); } @@ -564,7 +561,7 @@ protected Response permissionError( PermissionException pe ) { protected Response permissionError( String message ) { return error( Status.UNAUTHORIZED, message ); } - + protected static Response error( Status sts, String msg ) { return Response.status(sts) .entity( NullSafeJsonBuilder.jsonObjectBuilder() @@ -572,7 +569,7 @@ protected static Response error( Status sts, String msg ) { .add( "message", msg ).build() ).type(MediaType.APPLICATION_JSON_TYPE).build(); } - + protected Response allowCors( Response r ) { r.getHeaders().add("Access-Control-Allow-Origin", "*"); return r; @@ -583,9 +580,9 @@ class LazyRef { private interface Ref { T get(); } - + private Ref ref; - + public LazyRef( final Callable initer ) { ref = () -> { try { @@ -598,7 +595,7 @@ public LazyRef( final Callable initer ) { } }; } - + public T get() { return ref.get(); } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Groups.java b/src/main/java/edu/harvard/iq/dataverse/api/Groups.java index 2641e36939f..9e1b52be10b 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Groups.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Groups.java @@ -22,6 +22,7 @@ import javax.ws.rs.POST; import javax.ws.rs.PUT; import javax.ws.rs.PathParam; +import static org.apache.commons.lang.StringUtils.isNumeric; /** * @@ -31,22 +32,22 @@ @Stateless public class Groups extends AbstractApiBean { private static final Logger logger = Logger.getLogger(Groups.class.getName()); - + private IpGroupProvider ipGroupPrv; private ShibGroupProvider shibGroupPrv; - + Pattern legalGroupName = Pattern.compile("^[-_a-zA-Z0-9]+$"); - + @PostConstruct void postConstruct() { ipGroupPrv = groupSvc.getIpGroupProvider(); shibGroupPrv = groupSvc.getShibGroupProvider(); } - + /** - * Creates a new {@link IpGroup}. The name of the group is based on the + * Creates a new {@link IpGroup}. The name of the group is based on the * {@code alias:} field, but might be changed to ensure uniqueness. - * @param dto + * @param dto * @return Response describing the created group or the error that prevented * that group from being created. */ @@ -57,19 +58,19 @@ public Response postIpGroup( JsonObject dto ){ IpGroup grp = new JsonParser().parseIpGroup(dto); grp.setGroupProvider( ipGroupPrv ); grp.setPersistedGroupAlias( - ipGroupPrv.findAvailableName( + ipGroupPrv.findAvailableName( grp.getPersistedGroupAlias()==null ? "ipGroup" : grp.getPersistedGroupAlias())); - + grp = ipGroupPrv.store(grp); return created("/groups/ip/" + grp.getPersistedGroupAlias(), json(grp) ); - + } catch ( Exception e ) { logger.log( Level.WARNING, "Error while storing a new IP group: " + e.getMessage(), e); return error(Response.Status.INTERNAL_SERVER_ERROR, "Error: " + e.getMessage() ); - + } } - + /** * Creates or updates the {@link IpGroup} named {@code groupName}. * @param groupName Name of the group. @@ -92,21 +93,21 @@ public Response putIpGroups( @PathParam("groupName") String groupName, JsonObjec grp.setPersistedGroupAlias( groupName ); grp = ipGroupPrv.store(grp); return created("/groups/ip/" + grp.getPersistedGroupAlias(), json(grp) ); - + } catch ( Exception e ) { logger.log( Level.WARNING, "Error while storing a new IP group: " + e.getMessage(), e); return error(Response.Status.INTERNAL_SERVER_ERROR, "Error: " + e.getMessage() ); - + } } - + @GET @Path("ip") public Response listIpGroups() { return ok( ipGroupPrv.findGlobalGroups() .stream().map(g->json(g)).collect(toJsonArray()) ); } - + @GET @Path("ip/{groupIdtf}") public Response getIpGroup( @PathParam("groupIdtf") String groupIdtf ) { @@ -116,10 +117,10 @@ public Response getIpGroup( @PathParam("groupIdtf") String groupIdtf ) { } else { grp = ipGroupPrv.get(groupIdtf); } - + return (grp == null) ? notFound( "Group " + groupIdtf + " not found") : ok(json(grp)); } - + @DELETE @Path("ip/{groupIdtf}") public Response deleteIpGroup( @PathParam("groupIdtf") String groupIdtf ) { @@ -129,9 +130,9 @@ public Response deleteIpGroup( @PathParam("groupIdtf") String groupIdtf ) { } else { grp = ipGroupPrv.get(groupIdtf); } - + if (grp == null) return notFound( "Group " + groupIdtf + " not found"); - + try { ipGroupPrv.deleteGroup(grp); return ok("Group " + grp.getAlias() + " deleted."); @@ -141,7 +142,7 @@ public Response deleteIpGroup( @PathParam("groupIdtf") String groupIdtf ) { while ( e.getCause() != null ) { e = e.getCause(); } - + if ( e instanceof IllegalArgumentException ) { return error(Response.Status.BAD_REQUEST, e.getMessage()); } else { @@ -149,7 +150,7 @@ public Response deleteIpGroup( @PathParam("groupIdtf") String groupIdtf ) { } } } - + @GET @Path("shib") public Response listShibGroups() { @@ -207,5 +208,5 @@ public Response deleteShibGroup( @PathParam("primaryKey") String id ) { return error(Response.Status.BAD_REQUEST, "Could not find Shibboleth group with an id of " + id); } } - + } diff --git a/src/main/java/edu/harvard/iq/dataverse/api/Util.java b/src/main/java/edu/harvard/iq/dataverse/api/Util.java index ce7cb34a280..82adedc709f 100644 --- a/src/main/java/edu/harvard/iq/dataverse/api/Util.java +++ b/src/main/java/edu/harvard/iq/dataverse/api/Util.java @@ -15,13 +15,13 @@ public class Util { static final Set VALID_BOOLEAN_VALUES; static final Set BOOLEAN_TRUE_VALUES; - + static { BOOLEAN_TRUE_VALUES = new TreeSet<>(); BOOLEAN_TRUE_VALUES.add("true"); BOOLEAN_TRUE_VALUES.add("yes"); BOOLEAN_TRUE_VALUES.add("1"); - + VALID_BOOLEAN_VALUES = new TreeSet<>(); VALID_BOOLEAN_VALUES.addAll(BOOLEAN_TRUE_VALUES ); VALID_BOOLEAN_VALUES.add("no"); @@ -34,21 +34,14 @@ static JsonArray asJsonArray( String str ) { return rdr.readArray(); } } - + static boolean isBoolean( String s ) { return VALID_BOOLEAN_VALUES.contains(s.toLowerCase()); } - + static boolean isTrue( String s ) { return BOOLEAN_TRUE_VALUES.contains(s.toLowerCase()); } - - static boolean isNumeric( String s ) { - for ( char c : s.toCharArray() ) { - if ( ! Character.isDigit(c) ) return false; - } - return true; - } /** * @param date The Date object to convert. @@ -72,7 +65,7 @@ static boolean isNumeric( String s ) { */ private static final String DATE_TIME_FORMAT_STRING = "yyyy-MM-dd'T'HH:mm:ss'Z'"; private static final String DATE_FORMAT_STRING = "yyyy-MM-dd"; - + private static final ThreadLocal DATETIME_FORMAT_TL = new ThreadLocal(){ @Override protected SimpleDateFormat initialValue() @@ -82,7 +75,7 @@ protected SimpleDateFormat initialValue() return format; } }; - + private static final ThreadLocal DATE_FORMAT_TL = new ThreadLocal(){ @Override protected SimpleDateFormat initialValue() @@ -92,7 +85,7 @@ protected SimpleDateFormat initialValue() return format; } }; - + /** * Note: SimpleDateFormat is not thread-safe! Never retain the format returned by this method in a field. * @return The standard API format for date-and-time. @@ -100,7 +93,7 @@ protected SimpleDateFormat initialValue() public static SimpleDateFormat getDateTimeFormat() { return DATETIME_FORMAT_TL.get(); } - + /** * Note: SimpleDateFormat is not thread-safe! Never retain the format returned by this method in a field. * @return The standard API format for dates. @@ -108,25 +101,25 @@ public static SimpleDateFormat getDateTimeFormat() { public static SimpleDateFormat getDateFormat() { return DATE_FORMAT_TL.get(); } - + /** * Takes in a list of strings and returns a list stripped of nulls, empty strings and duplicates * @param stringsToCheck - * @return + * @return */ - + public static List removeDuplicatesNullsEmptyStrings(List stringsToCheck){ - + if (stringsToCheck == null){ throw new NullPointerException("stringsToCheck cannot be null"); } - + return stringsToCheck.stream() .filter(p -> p != null) // no nulls .map(String :: trim) // strip strings .filter(p -> p.length() > 0 ) // no empty strings .distinct() // distinct - .collect(Collectors.toList()); + .collect(Collectors.toList()); } - + } diff --git a/src/test/java/edu/harvard/iq/dataverse/api/AbstractApiBeanTest.java b/src/test/java/edu/harvard/iq/dataverse/api/AbstractApiBeanTest.java index 59bf0467ca9..8730bf724a6 100644 --- a/src/test/java/edu/harvard/iq/dataverse/api/AbstractApiBeanTest.java +++ b/src/test/java/edu/harvard/iq/dataverse/api/AbstractApiBeanTest.java @@ -31,13 +31,6 @@ public void before() { sut = new AbstractApiBeanImpl(); } - @Test - public void testIsNumeric() { - assertTrue(sut.isNumeric("1")); - assertTrue(sut.isNumeric("199999")); - assertFalse(sut.isNumeric("a")); - } - @Test public void testParseBooleanOrDie_ok() throws Exception { assertTrue(sut.parseBooleanOrDie("1"));