Skip to content

Commit

Permalink
4xx HTTP response codes should come with a ClientError status.
Browse files Browse the repository at this point in the history
This commit changes a few exception hierarchies, especially making
classes implement HasStatus, so that the status code can be correctly
reported by ExceptionRepresentation.

RestfulGraphDatabaseTest has had a lot of assertions added, confirming
that the correct status codes are returned

Fixes #4145.
  • Loading branch information
apcj committed Apr 2, 2015
1 parent 96685d3 commit 43f3662
Show file tree
Hide file tree
Showing 44 changed files with 312 additions and 206 deletions.
Expand Up @@ -19,13 +19,15 @@
*/ */
package org.neo4j.graphdb; package org.neo4j.graphdb;


import org.neo4j.kernel.api.exceptions.Status;

/** /**
* Thrown when the database is asked to modify data in a way that violates one or more * Thrown when the database is asked to modify data in a way that violates one or more
* constraints that it is expected to uphold. * constraints that it is expected to uphold.
* *
* For instance, if removing a node that still has relationships. * For instance, if removing a node that still has relationships.
*/ */
public class ConstraintViolationException extends RuntimeException public class ConstraintViolationException extends RuntimeException implements Status.HasStatus
{ {
public ConstraintViolationException( String msg ) public ConstraintViolationException( String msg )
{ {
Expand All @@ -36,4 +38,10 @@ public ConstraintViolationException( String msg, Throwable cause )
{ {
super(msg, cause); super(msg, cause);
} }

@Override
public Status status()
{
return Status.Schema.ConstraintViolation;
}
} }
Expand Up @@ -62,6 +62,7 @@
import org.neo4j.io.fs.FileSystemAbstraction; import org.neo4j.io.fs.FileSystemAbstraction;
import org.neo4j.io.pagecache.PageCache; import org.neo4j.io.pagecache.PageCache;
import org.neo4j.io.pagecache.monitoring.PageCacheMonitor; import org.neo4j.io.pagecache.monitoring.PageCacheMonitor;
import org.neo4j.kernel.api.EntityType;
import org.neo4j.kernel.api.KernelAPI; import org.neo4j.kernel.api.KernelAPI;
import org.neo4j.kernel.api.KernelTransaction; import org.neo4j.kernel.api.KernelTransaction;
import org.neo4j.kernel.api.ReadOperations; import org.neo4j.kernel.api.ReadOperations;
Expand Down Expand Up @@ -1056,13 +1057,15 @@ public Node getNodeById( long id )
{ {
if ( id < 0 || id > MAX_NODE_ID ) if ( id < 0 || id > MAX_NODE_ID )
{ {
throw new NotFoundException( format( "Node %d not found", id ) ); throw new NotFoundException( format( "Node %d not found", id ),
new EntityNotFoundException( EntityType.NODE, id ) );
} }
try ( Statement statement = threadToTransactionBridge.instance() ) try ( Statement statement = threadToTransactionBridge.instance() )
{ {
if ( !statement.readOperations().nodeExists( id ) ) if ( !statement.readOperations().nodeExists( id ) )
{ {
throw new NotFoundException( format( "Node %d not found", id ) ); throw new NotFoundException( format( "Node %d not found", id ),
new EntityNotFoundException( EntityType.NODE, id ) );
} }


return nodeManager.newNodeProxyById( id ); return nodeManager.newNodeProxyById( id );
Expand All @@ -1074,13 +1077,15 @@ public Relationship getRelationshipById( long id )
{ {
if ( id < 0 || id > MAX_RELATIONSHIP_ID ) if ( id < 0 || id > MAX_RELATIONSHIP_ID )
{ {
throw new NotFoundException( format( "Relationship %d not found", id ) ); throw new NotFoundException( format( "Relationship %d not found", id ),
new EntityNotFoundException( EntityType.RELATIONSHIP, id ));
} }
try ( Statement statement = threadToTransactionBridge.instance() ) try ( Statement statement = threadToTransactionBridge.instance() )
{ {
if ( !statement.readOperations().relationshipExists( id ) ) if ( !statement.readOperations().relationshipExists( id ) )
{ {
throw new NotFoundException( format( "Relationship %d not found", id ) ); throw new NotFoundException( format( "Relationship %d not found", id ),
new EntityNotFoundException( EntityType.RELATIONSHIP, id ));
} }


return nodeManager.newRelationshipProxy( id ); return nodeManager.newRelationshipProxy( id );
Expand Down
Expand Up @@ -19,7 +19,9 @@
*/ */
package org.neo4j.server.rest.repr; package org.neo4j.server.rest.repr;


public class BadInputException extends Exception import org.neo4j.kernel.api.exceptions.Status;

public class BadInputException extends Exception implements Status.HasStatus
{ {
public BadInputException( Throwable cause ) public BadInputException( Throwable cause )
{ {
Expand All @@ -35,4 +37,10 @@ public BadInputException( String message, Throwable cause )
{ {
super( message, cause ); super( message, cause );
} }

@Override
public Status status()
{
return Status.Request.InvalidFormat;
}
} }
Expand Up @@ -17,15 +17,20 @@
* You should have received a copy of the GNU General Public License * You should have received a copy of the GNU General Public License
* along with this program. If not, see <http://www.gnu.org/licenses/>. * along with this program. If not, see <http://www.gnu.org/licenses/>.
*/ */
package org.neo4j.server.rest.web; package org.neo4j.server.rest.repr;


public class OperationFailureException extends Exception import org.neo4j.kernel.api.exceptions.Status;

public class InvalidArgumentsException extends BadInputException
{ {
public OperationFailureException( String message ) public InvalidArgumentsException( String message )
{ {
super( message ); super( message );
} }


private static final long serialVersionUID = -4594462038185850546L; @Override

public Status status()
{
return Status.Statement.InvalidArguments;
}
} }
Expand Up @@ -297,7 +297,7 @@ protected Relationship getRelationship( GraphDatabaseAPI graphDb, URI uri )
} }
catch ( NotFoundException e ) catch ( NotFoundException e )
{ {
throw new RelationshipNotFoundException(); throw new RelationshipNotFoundException( e );
} }
} }


Expand Down
Expand Up @@ -23,14 +23,14 @@


public class NodeNotFoundException extends Exception public class NodeNotFoundException extends Exception
{ {
public NodeNotFoundException( String message ) public NodeNotFoundException( String message, NotFoundException e )
{ {
super(message); super( message, e );
} }


public NodeNotFoundException( NotFoundException e ) public NodeNotFoundException( NotFoundException e )
{ {
super(e); super( e );
} }


private static final long serialVersionUID = 7292603734007524712L; private static final long serialVersionUID = 7292603734007524712L;
Expand Down
Expand Up @@ -24,4 +24,8 @@ public class RelationshipNotFoundException extends Exception
{ {
private static final long serialVersionUID = -1177555212368703516L; private static final long serialVersionUID = -1177555212368703516L;


public RelationshipNotFoundException( Throwable cause )
{
super( cause );
}
} }
Expand Up @@ -19,8 +19,13 @@
*/ */
package org.neo4j.server.rest.domain; package org.neo4j.server.rest.domain;


import org.neo4j.server.rest.web.NodeNotFoundException;

@SuppressWarnings( "serial" ) @SuppressWarnings( "serial" )
public class EndNodeNotFoundException extends Exception public class EndNodeNotFoundException extends Exception
{ {

public EndNodeNotFoundException( NodeNotFoundException e )
{
super( e );
}
} }
Expand Up @@ -78,25 +78,21 @@ public static Object readJson( String json ) throws JsonParseException
} }
} }


public static Object jsonToSingleValue( String json ) throws org.neo4j.server.rest.web.PropertyValueException public static Object assertSupportedPropertyValue( Object jsonObject ) throws PropertyValueException
{
Object jsonObject = readJson( json );
return jsonObject instanceof Collection<?> ? jsonObject : assertSupportedPropertyValue( jsonObject );
}

private static Object assertSupportedPropertyValue( Object jsonObject ) throws PropertyValueException
{ {
if ( jsonObject instanceof Collection<?> )
{
return jsonObject;
}
if ( jsonObject == null ) if ( jsonObject == null )
{ {
throw new org.neo4j.server.rest.web.PropertyValueException( "null value not supported" ); throw new PropertyValueException( "null value not supported" );

} }

if ( !(jsonObject instanceof String || if ( !(jsonObject instanceof String ||
jsonObject instanceof Number || jsonObject instanceof Number ||
jsonObject instanceof Boolean) ) jsonObject instanceof Boolean) )
{ {
throw new org.neo4j.server.rest.web.PropertyValueException( throw new PropertyValueException(
"Unsupported value type " + jsonObject.getClass() + "." "Unsupported value type " + jsonObject.getClass() + "."
+ " Supported value types are all java primitives (byte, char, short, int, " + " Supported value types are all java primitives (byte, char, short, int, "
+ "long, float, double) and String, as well as arrays of all those types" ); + "long, float, double) and String, as well as arrays of all those types" );
Expand Down
Expand Up @@ -19,22 +19,24 @@
*/ */
package org.neo4j.server.rest.domain; package org.neo4j.server.rest.domain;


import org.neo4j.kernel.api.exceptions.Status;

@SuppressWarnings( "serial" ) @SuppressWarnings( "serial" )
public class JsonParseException extends org.neo4j.server.rest.web.PropertyValueException public class JsonParseException extends Exception implements Status.HasStatus
{ {

public JsonParseException( String message, Throwable cause ) public JsonParseException( String message, Throwable cause )
{ {
super( message, cause ); super( message, cause );
} }


public JsonParseException( String message ) public JsonParseException( Throwable cause )
{ {
super( message ); super( cause );
} }


public JsonParseException( Throwable cause ) @Override
public Status status()
{ {
super( cause ); return Status.Request.InvalidFormat;
} }
} }
Expand Up @@ -138,7 +138,7 @@ public void setProperty(PropertyContainer entity, String key, Object value) thro
} }
catch ( IllegalArgumentException e ) catch ( IllegalArgumentException e )
{ {
throw new PropertyValueException( key, value ); throw new PropertyValueException( "Could not set property \"" + key + "\", unsupported type: " + value );
} }
} }


Expand Down
Expand Up @@ -19,8 +19,13 @@
*/ */
package org.neo4j.server.rest.domain; package org.neo4j.server.rest.domain;


import org.neo4j.server.rest.web.NodeNotFoundException;

@SuppressWarnings( "serial" ) @SuppressWarnings( "serial" )
public class StartNodeNotFoundException extends Exception public class StartNodeNotFoundException extends Exception
{ {

public StartNodeNotFoundException( NodeNotFoundException e )
{
super( e );
}
} }
Expand Up @@ -44,7 +44,7 @@
import org.neo4j.jmx.impl.JmxKernelExtension; import org.neo4j.jmx.impl.JmxKernelExtension;
import org.neo4j.server.database.Database; import org.neo4j.server.database.Database;
import org.neo4j.server.rest.domain.JsonHelper; import org.neo4j.server.rest.domain.JsonHelper;
import org.neo4j.server.rest.repr.BadInputException; import org.neo4j.server.rest.domain.JsonParseException;
import org.neo4j.server.rest.repr.InputFormat; import org.neo4j.server.rest.repr.InputFormat;
import org.neo4j.server.rest.repr.ListRepresentation; import org.neo4j.server.rest.repr.ListRepresentation;
import org.neo4j.server.rest.repr.OutputFormat; import org.neo4j.server.rest.repr.OutputFormat;
Expand Down Expand Up @@ -153,7 +153,7 @@ public Response queryBeans( String query )
MBeanServer server = ManagementFactory.getPlatformMBeanServer(); MBeanServer server = ManagementFactory.getPlatformMBeanServer();


String json = dodgeStartingUnicodeMarker( query ); String json = dodgeStartingUnicodeMarker( query );
Collection<Object> queries = (Collection<Object>) JsonHelper.jsonToSingleValue( json ); Collection<Object> queries = (Collection<Object>) JsonHelper.readJson( json );


ArrayList<JmxMBeanRepresentation> beans = new ArrayList<JmxMBeanRepresentation>(); ArrayList<JmxMBeanRepresentation> beans = new ArrayList<JmxMBeanRepresentation>();
for ( Object queryObj : queries ) for ( Object queryObj : queries )
Expand All @@ -167,15 +167,14 @@ public Response queryBeans( String query )


return output.ok( new ListRepresentation( "jmxBean", beans ) ); return output.ok( new ListRepresentation( "jmxBean", beans ) );
} }
catch ( MalformedObjectNameException e ) catch ( JsonParseException e )
{ {
return output.badRequest( e ); return output.badRequest( e );
} }
catch ( BadInputException e ) catch ( MalformedObjectNameException e )
{ {
return output.badRequest( e ); return output.badRequest( e );
} }

} }


@POST @POST
Expand Down
Expand Up @@ -150,11 +150,11 @@ public static <T> Map<String, T> validateKeys( Map<String, T> map, String... req
{ {
if ( missing.size() == 1 ) if ( missing.size() == 1 )
{ {
throw new BadInputException( "Missing required key: \"" + missing.iterator().next() + "\"" ); throw new InvalidArgumentsException( "Missing required key: \"" + missing.iterator().next() + "\"" );
} }
else else
{ {
throw new BadInputException( "Missing required keys: " + missing ); throw new InvalidArgumentsException( "Missing required keys: " + missing );
} }
} }
return map; return map;
Expand Down
Expand Up @@ -42,6 +42,9 @@
import org.neo4j.server.rest.repr.Representation; import org.neo4j.server.rest.repr.Representation;
import org.neo4j.server.rest.repr.RepresentationFormat; import org.neo4j.server.rest.repr.RepresentationFormat;


import static org.neo4j.server.rest.domain.JsonHelper.assertSupportedPropertyValue;
import static org.neo4j.server.rest.domain.JsonHelper.readJson;

@Service.Implementation( RepresentationFormat.class ) @Service.Implementation( RepresentationFormat.class )
public class CompactJsonFormat extends RepresentationFormat public class CompactJsonFormat extends RepresentationFormat
{ {
Expand Down Expand Up @@ -219,7 +222,7 @@ public Object readValue( String input ) throws BadInputException
if ( empty( input ) ) return Collections.emptyMap(); if ( empty( input ) ) return Collections.emptyMap();
try try
{ {
return JsonHelper.jsonToSingleValue( stripByteOrderMark( input ) ); return assertSupportedPropertyValue( readJson( stripByteOrderMark( input ) ) );
} }
catch ( JsonParseException ex ) catch ( JsonParseException ex )
{ {
Expand Down
Expand Up @@ -26,7 +26,6 @@
import java.util.LinkedHashMap; import java.util.LinkedHashMap;
import java.util.List; import java.util.List;
import java.util.Map; import java.util.Map;

import javax.ws.rs.core.MediaType; import javax.ws.rs.core.MediaType;


import org.neo4j.server.rest.domain.JsonHelper; import org.neo4j.server.rest.domain.JsonHelper;
Expand All @@ -37,6 +36,9 @@
import org.neo4j.server.rest.repr.MappingWriter; import org.neo4j.server.rest.repr.MappingWriter;
import org.neo4j.server.rest.repr.RepresentationFormat; import org.neo4j.server.rest.repr.RepresentationFormat;


import static org.neo4j.server.rest.domain.JsonHelper.assertSupportedPropertyValue;
import static org.neo4j.server.rest.domain.JsonHelper.readJson;

public class JsonFormat extends RepresentationFormat public class JsonFormat extends RepresentationFormat
{ {
public JsonFormat() public JsonFormat()
Expand Down Expand Up @@ -117,7 +119,7 @@ public Object readValue( String input ) throws BadInputException
if ( empty( input ) ) return Collections.emptyMap(); if ( empty( input ) ) return Collections.emptyMap();
try try
{ {
return JsonHelper.jsonToSingleValue( stripByteOrderMark( input ) ); return assertSupportedPropertyValue( readJson( stripByteOrderMark( input ) ) );
} }
catch ( JsonParseException ex ) catch ( JsonParseException ex )
{ {
Expand Down
Expand Up @@ -30,6 +30,7 @@
import javax.ws.rs.core.Response; import javax.ws.rs.core.Response;


import org.neo4j.server.rest.repr.BadInputException; import org.neo4j.server.rest.repr.BadInputException;
import org.neo4j.server.rest.repr.InvalidArgumentsException;
import org.neo4j.server.rest.repr.ListWriter; import org.neo4j.server.rest.repr.ListWriter;
import org.neo4j.server.rest.repr.MappingWriter; import org.neo4j.server.rest.repr.MappingWriter;
import org.neo4j.server.rest.repr.MediaTypeNotSupportedException; import org.neo4j.server.rest.repr.MediaTypeNotSupportedException;
Expand Down Expand Up @@ -75,7 +76,7 @@ public Map<String, Object> readMap( String input, String... requiredKeys ) throw
if ( requiredKeys.length != 0 ) if ( requiredKeys.length != 0 )
{ {
String missingKeys = Arrays.toString( requiredKeys ); String missingKeys = Arrays.toString( requiredKeys );
throw new BadInputException( "Missing required keys: " + missingKeys ); throw new InvalidArgumentsException( "Missing required keys: " + missingKeys );
} }
return Collections.emptyMap(); return Collections.emptyMap();
} }
Expand Down
Expand Up @@ -46,6 +46,9 @@
import org.neo4j.server.rest.repr.RepresentationFormat; import org.neo4j.server.rest.repr.RepresentationFormat;
import org.neo4j.server.rest.repr.StreamingFormat; import org.neo4j.server.rest.repr.StreamingFormat;


import static org.neo4j.server.rest.domain.JsonHelper.assertSupportedPropertyValue;
import static org.neo4j.server.rest.domain.JsonHelper.readJson;

@Service.Implementation(RepresentationFormat.class) @Service.Implementation(RepresentationFormat.class)
public class StreamingJsonFormat extends RepresentationFormat implements StreamingFormat public class StreamingJsonFormat extends RepresentationFormat implements StreamingFormat
{ {
Expand Down Expand Up @@ -163,7 +166,7 @@ public Object readValue( String input ) throws BadInputException
} }
try try
{ {
return JsonHelper.jsonToSingleValue( stripByteOrderMark( input ) ); return assertSupportedPropertyValue( readJson( stripByteOrderMark( input ) ) );
} }
catch ( JsonParseException ex ) catch ( JsonParseException ex )
{ {
Expand Down

0 comments on commit 43f3662

Please sign in to comment.