Remove duplicate entity get from update by name/id paths. Requires caller to pass in entity to em.updateProperties. Check if debug is enabled in many placesbefore calling logger.debug.
Project: http://git-wip-us.apache.org/repos/asf/usergrid/repo Commit: http://git-wip-us.apache.org/repos/asf/usergrid/commit/dc0c6d39 Tree: http://git-wip-us.apache.org/repos/asf/usergrid/tree/dc0c6d39 Diff: http://git-wip-us.apache.org/repos/asf/usergrid/diff/dc0c6d39 Branch: refs/heads/USERGRID-872 Commit: dc0c6d3999207d301ed4872b91b0b9e8dd25d011 Parents: 1cdefdb Author: Michael Russo <michaelaru...@gmail.com> Authored: Tue Dec 29 17:41:47 2015 -0800 Committer: Michael Russo <michaelaru...@gmail.com> Committed: Tue Dec 29 17:41:47 2015 -0800 ---------------------------------------------------------------------- .../corepersistence/CpEntityManager.java | 30 ++++++---- .../rest/applications/ServiceResource.java | 59 ++++++++++++++------ .../rest/applications/users/UsersResource.java | 19 +++++-- .../rest/test/resource/AbstractRestIT.java | 10 +++- .../cassandra/ManagementServiceImpl.java | 55 +++++++++++++----- 5 files changed, 125 insertions(+), 48 deletions(-) ---------------------------------------------------------------------- http://git-wip-us.apache.org/repos/asf/usergrid/blob/dc0c6d39/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpEntityManager.java ---------------------------------------------------------------------- diff --git a/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpEntityManager.java b/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpEntityManager.java index d652293..cbb06ed 100644 --- a/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpEntityManager.java +++ b/stack/core/src/main/java/org/apache/usergrid/corepersistence/CpEntityManager.java @@ -733,7 +733,8 @@ public class CpEntityManager implements EntityManager { @Override public void updateApplication( Map<String, Object> properties ) throws Exception { - this.updateProperties(new SimpleEntityRef(Application.ENTITY_TYPE, applicationId), properties); + Entity entity = this.get(applicationId, Application.class); + this.updateProperties(entity, properties); this.application = get( applicationId, Application.class ); } @@ -1013,7 +1014,9 @@ public class CpEntityManager implements EntityManager { entityRef = cref.getItemRef(); } - Entity entity = get( entityRef ); + // removing the nested em.get and requiring the caller to pass in the full entity entity + //Entity entity = get( entityRef ); + Entity entity = ( Entity )entityRef; properties.put(PROPERTY_MODIFIED, UUIDUtils.getTimestampInMillis(UUIDUtils.newTimeUUID())); @@ -1060,16 +1063,20 @@ public class CpEntityManager implements EntityManager { cpEntity.removeField( propertyName ); - logger.debug( "About to Write {}:{} version {}", new Object[] { + if(logger.isDebugEnabled()){ + logger.debug( "About to Write {}:{} version {}", new Object[] { cpEntity.getId().getType(), cpEntity.getId().getUuid(), cpEntity.getVersion() - } ); + } ); + } //TODO: does this call and others like it need a graphite reporter? cpEntity = ecm.write( cpEntity ).toBlocking().last(); - logger.debug("Wrote {}:{} version {}", new Object[]{ - cpEntity.getId().getType(), cpEntity.getId().getUuid(), cpEntity.getVersion() - }); + if(logger.isDebugEnabled()){ + logger.debug("Wrote {}:{} version {}", new Object[]{ + cpEntity.getId().getType(), cpEntity.getId().getUuid(), cpEntity.getVersion() + }); + } //Adding graphite metrics @@ -2134,10 +2141,14 @@ public class CpEntityManager implements EntityManager { public EntityRef getUserByIdentifier( Identifier identifier ) throws Exception { if ( identifier == null ) { - logger.debug( "getUserByIdentifier: returning null for null identifier" ); + if(logger.isDebugEnabled()){ + logger.debug( "getUserByIdentifier: returning null for null identifier" ); + } return null; } - logger.debug( "getUserByIdentifier {}:{}", identifier.getType(), identifier.toString() ); + if(logger.isDebugEnabled()){ + logger.debug( "getUserByIdentifier {}:{}", identifier.getType(), identifier.toString() ); + } if ( identifier.isUUID() ) { return new SimpleEntityRef( "user", identifier.getUUID() ); @@ -2172,7 +2183,6 @@ public class CpEntityManager implements EntityManager { // } // else { // look-aside as it might be an email in the name field - logger.debug( "return alias" ); return this.getAlias( new SimpleEntityRef( Application.ENTITY_TYPE, applicationId ), "user", identifier.getEmail() ); // } http://git-wip-us.apache.org/repos/asf/usergrid/blob/dc0c6d39/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java ---------------------------------------------------------------------- diff --git a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java index e98f366..9956ff7 100644 --- a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java +++ b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/ServiceResource.java @@ -179,7 +179,11 @@ public class ServiceResource extends AbstractContextResource { @Path("file") public AbstractContextResource getFileResource( @Context UriInfo ui ) throws Exception { - logger.debug( "in assets in ServiceResource" ); + + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.getFileResource" ); + } + ServiceParameter.addParameter( getServiceParameters(), "assets" ); PathSegment ps = getFirstPathSegment( "assets" ); @@ -195,7 +199,9 @@ public class ServiceResource extends AbstractContextResource { public AbstractContextResource addIdParameter( @Context UriInfo ui, @PathParam("entityId") PathSegment entityId ) throws Exception { - logger.debug( "ServiceResource.addIdParameter" ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.addIdParameter" ); + } UUID itemId = UUID.fromString( entityId.getPath() ); @@ -210,10 +216,11 @@ public class ServiceResource extends AbstractContextResource { @Path("{itemName}") public AbstractContextResource addNameParameter( @Context UriInfo ui, @PathParam("itemName") PathSegment itemName ) throws Exception { + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.addNameParameter" ); + logger.debug( "Current segment is {}", itemName.getPath() ); + } - logger.debug( "ServiceResource.addNameParameter" ); - - logger.debug( "Current segment is {}", itemName.getPath() ); if ( itemName.getPath().startsWith( "{" ) ) { Query query = Query.fromJsonString( itemName.getPath() ); @@ -233,8 +240,10 @@ public class ServiceResource extends AbstractContextResource { public ServiceResults executeServiceRequest( UriInfo ui, ApiResponse response, ServiceAction action, ServicePayload payload ) throws Exception { + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.executeServiceRequest" ); + } - logger.debug( "ServiceResource.executeServiceRequest" ); boolean tree = "true".equalsIgnoreCase( ui.getQueryParameters().getFirst( "tree" ) ); @@ -321,7 +330,9 @@ public class ServiceResource extends AbstractContextResource { @QueryParam("callback") @DefaultValue("callback") String callback ) throws Exception { - logger.debug( "ServiceResource.executeGet" ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.executeGet" ); + } ApiResponse response = createApiResponse(); @@ -370,7 +381,9 @@ public class ServiceResource extends AbstractContextResource { public ApiResponse executePostWithObject( @Context UriInfo ui, Object json, @QueryParam("callback") @DefaultValue("callback") String callback ) throws Exception { - logger.debug( "ServiceResource.executePostWithMap" ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.executePostWithMap" ); + } ApiResponse response = createApiResponse(); @@ -417,7 +430,9 @@ public class ServiceResource extends AbstractContextResource { public ApiResponse executePost( @Context UriInfo ui, String body, @QueryParam("callback") @DefaultValue("callback") String callback ) throws Exception { - logger.debug( "ServiceResource.executePost: body = " + body ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.executePost: body = " + body ); + } Object json; if ( StringUtils.isEmpty( body ) ) { @@ -451,7 +466,9 @@ public class ServiceResource extends AbstractContextResource { @QueryParam("callback") @DefaultValue("callback") String callback ) throws Exception { - logger.debug( "ServiceResource.executePut" ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.executePut" ); + } ObjectMapper mapper = new ObjectMapper(); Map<String, Object> json = mapper.readValue( body, mapTypeReference ); @@ -470,7 +487,9 @@ public class ServiceResource extends AbstractContextResource { @QueryParam("app_delete_confirm") String confirmAppDelete ) throws Exception { - logger.debug( "ServiceResource.executeDelete" ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.executeDelete" ); + } ApiResponse response = createApiResponse(); response.setAction( "delete" ); @@ -587,7 +606,9 @@ public class ServiceResource extends AbstractContextResource { @QueryParam("callback") @DefaultValue("callback") String callback, FormDataMultiPart multiPart ) throws Exception { - logger.debug( "ServiceResource.executeMultiPartPost" ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.executeMultiPartPost" ); + } return executeMultiPart( ui, callback, multiPart, ServiceAction.POST ); } @@ -601,7 +622,9 @@ public class ServiceResource extends AbstractContextResource { @QueryParam("callback") @DefaultValue("callback") String callback, FormDataMultiPart multiPart ) throws Exception { - logger.debug( "ServiceResource.executeMultiPartPut" ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.executeMultiPartPut" ); + } return executeMultiPart( ui, callback, multiPart, ServiceAction.PUT ); } @@ -723,7 +746,9 @@ public class ServiceResource extends AbstractContextResource { @HeaderParam("range") String rangeHeader, @HeaderParam("if-modified-since") String modifiedSince ) throws Exception { - logger.debug( "ServiceResource.executeStreamGet" ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.executeStreamGet" ); + } //Needed for testing if(properties.getProperty( PROPERTIES_USERGRID_BINARY_UPLOADER ).equals( "local" )){ @@ -740,8 +765,10 @@ public class ServiceResource extends AbstractContextResource { ServiceResults serviceResults = executeServiceRequest( ui, response, ServiceAction.GET, null ); Entity entity = serviceResults.getEntity(); - logger.info( "In ServiceResource.executeStreamGet with id: {}, range: {}, modifiedSince: {}", - new Object[] { entityId, rangeHeader, modifiedSince } ); + if(logger.isDebugEnabled()){ + logger.debug( "In ServiceResource.executeStreamGet with id: {}, range: {}, modifiedSince: {}", + new Object[] { entityId, rangeHeader, modifiedSince } ); + } Map<String, Object> fileMetadata = AssetUtils.getFileMetadata( entity ); http://git-wip-us.apache.org/repos/asf/usergrid/blob/dc0c6d39/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java ---------------------------------------------------------------------- diff --git a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java index 3769449..6b1aba4 100644 --- a/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java +++ b/stack/rest/src/main/java/org/apache/usergrid/rest/applications/users/UsersResource.java @@ -71,7 +71,9 @@ public class UsersResource extends ServiceResource { public AbstractContextResource addIdParameter( @Context UriInfo ui, @PathParam("entityId") PathSegment entityId ) throws Exception { - logger.info( "ServiceResource.addIdParameter" ); + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.addIdParameter" ); + } UUID itemId = UUID.fromString( entityId.getPath() ); @@ -87,10 +89,11 @@ public class UsersResource extends ServiceResource { @Path("{itemName}") public AbstractContextResource addNameParameter( @Context UriInfo ui, @PathParam("itemName") PathSegment itemName ) throws Exception { + if(logger.isDebugEnabled()){ + logger.debug( "ServiceResource.addNameParameter" ); + logger.debug( "Current segment is " + itemName.getPath() ); + } - logger.info( "ServiceResource.addNameParameter" ); - - logger.info( "Current segment is " + itemName.getPath() ); if ( itemName.getPath().startsWith( "{" ) ) { Query query = Query.fromJsonString( itemName.getPath() ); @@ -209,7 +212,9 @@ public class UsersResource extends ServiceResource { @QueryParam("callback") @DefaultValue("callback") String callback ) throws Exception { - logger.debug( "UsersResource.executePost: body = " + body); + if(logger.isDebugEnabled()){ + logger.debug( "UsersResource.executePost: body = " + body); + } Object json = readJsonToObject( body ); @@ -221,7 +226,9 @@ public class UsersResource extends ServiceResource { boolean activated = !( ( confRequred != null ) && confRequred ); - logger.debug("Confirmation required: {} Activated: {}", confRequred, activated ); + if(logger.isDebugEnabled()){ + logger.debug("Confirmation required: {} Activated: {}", confRequred, activated ); + } if ( json instanceof Map ) { @SuppressWarnings("unchecked") Map<String, Object> map = ( Map<String, Object> ) json; http://git-wip-us.apache.org/repos/asf/usergrid/blob/dc0c6d39/stack/rest/src/test/java/org/apache/usergrid/rest/test/resource/AbstractRestIT.java ---------------------------------------------------------------------- diff --git a/stack/rest/src/test/java/org/apache/usergrid/rest/test/resource/AbstractRestIT.java b/stack/rest/src/test/java/org/apache/usergrid/rest/test/resource/AbstractRestIT.java index 4282853..b253543 100644 --- a/stack/rest/src/test/java/org/apache/usergrid/rest/test/resource/AbstractRestIT.java +++ b/stack/rest/src/test/java/org/apache/usergrid/rest/test/resource/AbstractRestIT.java @@ -173,8 +173,14 @@ public class AbstractRestIT extends JerseyTest { } public void refreshIndex() { - //TODO: add error checking and logging - clientSetup.refreshIndex(); + //TODO see how we can refresh index (not async) for tests so sleep may not be needed + try { + clientSetup.refreshIndex(); + Thread.sleep(100); + } catch (InterruptedException e) { + System.out.println("Error refreshing index"); + e.printStackTrace(); + } } http://git-wip-us.apache.org/repos/asf/usergrid/blob/dc0c6d39/stack/services/src/main/java/org/apache/usergrid/management/cassandra/ManagementServiceImpl.java ---------------------------------------------------------------------- diff --git a/stack/services/src/main/java/org/apache/usergrid/management/cassandra/ManagementServiceImpl.java b/stack/services/src/main/java/org/apache/usergrid/management/cassandra/ManagementServiceImpl.java index 5ffe830..af70966 100644 --- a/stack/services/src/main/java/org/apache/usergrid/management/cassandra/ManagementServiceImpl.java +++ b/stack/services/src/main/java/org/apache/usergrid/management/cassandra/ManagementServiceImpl.java @@ -269,7 +269,9 @@ public class ManagementServiceImpl implements ManagementService { try { createApplication( organization.getUuid(), test_app_name ); }catch(ApplicationAlreadyExistsException aaee){ - logger.debug("The database setup already found an existing application"); + if(logger.isDebugEnabled()){ + logger.debug("The database setup already found an existing application"); + } } } } @@ -410,7 +412,9 @@ public class ManagementServiceImpl implements ManagementService { public OrganizationOwnerInfo createOwnerAndOrganization( String organizationName, String username, String name, String email, String password ) throws Exception { - logger.debug("createOwnerAndOrganization1"); + if(logger.isDebugEnabled()){ + logger.debug("createOwnerAndOrganization1"); + } boolean activated = !newAdminUsersNeedSysAdminApproval() && !newOrganizationsNeedSysAdminApproval(); boolean disabled = newAdminUsersRequireConfirmation(); // if we are active and enabled, skip the send email step @@ -424,7 +428,9 @@ public class ManagementServiceImpl implements ManagementService { public OrganizationOwnerInfo createOwnerAndOrganization( String organizationName, String username, String name, String email, String password, boolean activated, boolean disabled ) throws Exception { - logger.debug("createOwnerAndOrganization2"); + if(logger.isDebugEnabled()){ + logger.debug("createOwnerAndOrganization2"); + } return createOwnerAndOrganization( organizationName, username, name, email, password, activated, disabled, null, null ); } @@ -437,7 +443,9 @@ public class ManagementServiceImpl implements ManagementService { Map<String, Object> organizationProperties ) throws Exception { - logger.debug("createOwnerAndOrganization3"); + if(logger.isDebugEnabled()){ + logger.debug("createOwnerAndOrganization3"); + } /** * Only lock on the target values. We don't want lock contention if another @@ -472,7 +480,9 @@ public class ManagementServiceImpl implements ManagementService { user = createAdminUserInternal( username, name, email, password, activated, disabled, userProperties ); } - logger.debug("User created"); + if(logger.isDebugEnabled()){ + logger.debug("User created"); + } organization = createOrganizationInternal( null, organizationName, user, true, organizationProperties ); } finally { @@ -487,7 +497,9 @@ public class ManagementServiceImpl implements ManagementService { private OrganizationInfo createOrganizationInternal( UUID orgUuid, String organizationName, UserInfo user, boolean activated ) throws Exception { - logger.debug("createOrganizationInternal1"); + if(logger.isDebugEnabled()){ + logger.debug("createOrganizationInternal1"); + } return createOrganizationInternal( orgUuid, organizationName, user, activated, null ); } @@ -495,14 +507,20 @@ public class ManagementServiceImpl implements ManagementService { private OrganizationInfo createOrganizationInternal( UUID orgUuid, String organizationName, UserInfo user, boolean activated, Map<String, Object> properties ) throws Exception { - logger.info( "createOrganizationInternal2: {}", organizationName ); + if(logger.isDebugEnabled()){ + logger.debug( "createOrganizationInternal2: {}", organizationName ); + } if ( organizationName == null ) { - logger.debug("organizationName = null"); + if(logger.isDebugEnabled()){ + logger.debug("organizationName = null"); + } return null; } if ( user == null ) { - logger.debug("user = null"); + if(logger.isDebugEnabled()){ + logger.debug("user = null"); + } return null; } @@ -995,7 +1013,8 @@ public class ManagementServiceImpl implements ManagementService { json.remove( "newpassword" ); Map<String, Object> userProperties = user.getProperties(); userProperties.putAll( json ); - em.updateProperties( entityRef, userProperties ); + Entity entity = em.get( entityRef, User.class); + em.updateProperties( entity, userProperties ); } user = getAdminUserByUuid( user.getUuid() ); @@ -1208,7 +1227,9 @@ public class ManagementServiceImpl implements ManagementService { public int calculatePasswordHistorySizeForUser( UUID userId ) throws Exception { - logger.debug( "calculatePasswordHistorySizeForUser " + userId.toString() ); + if(logger.isDebugEnabled()){ + logger.debug( "calculatePasswordHistorySizeForUser " + userId.toString() ); + } int size = 0; EntityManager em = emf.getEntityManager( smf.getManagementAppId() ); @@ -1216,13 +1237,17 @@ public class ManagementServiceImpl implements ManagementService { Results orgResults = em.getCollection( new SimpleEntityRef( User.ENTITY_TYPE, userId ), Schema.COLLECTION_GROUPS, null, 10000, Level.REFS, false ); - logger.debug(" orgResults.size() = " + orgResults.size()); + if(logger.isDebugEnabled()){ + logger.debug(" orgResults.size() = " + orgResults.size()); + } for ( EntityRef orgRef : orgResults.getRefs() ) { Map properties = em.getDictionaryAsMap( orgRef, ORGANIZATION_PROPERTIES_DICTIONARY ); if ( properties != null ) { OrganizationInfo orgInfo = new OrganizationInfo( null, null, properties ); - logger.debug( " orgInfo.getPasswordHistorySize() = " + orgInfo.getPasswordHistorySize() ); + if(logger.isDebugEnabled()){ + logger.debug( " orgInfo.getPasswordHistorySize() = " + orgInfo.getPasswordHistorySize() ); + } size = Math.max( orgInfo.getPasswordHistorySize(), size ); } } @@ -1246,7 +1271,9 @@ public class ManagementServiceImpl implements ManagementService { public UserInfo verifyAdminUserPasswordCredentials( String name, String password ) throws Exception { UserInfo userInfo = null; - logger.debug("verifyAdminUserPasswordCredentials for {}/{}", name, password); + if(logger.isDebugEnabled()){ + logger.debug("verifyAdminUserPasswordCredentials for {}/{}", name, password); + } User user = findUserEntity( smf.getManagementAppId(), name ); if ( user == null ) {
