From 0cb2752c8164913cf97d01c705a5faab54eb0288 Mon Sep 17 00:00:00 2001 From: Bryan Bende Date: Wed, 14 Mar 2018 10:44:27 -0400 Subject: [PATCH] NIFIREG-144 Fixing NPE when retrieving latest snapshot when no versions exist --- .../registry/service/RegistryService.java | 27 +------ .../registry/service/TestRegistryService.java | 74 +++++++++++++++++++ .../registry/web/api/BucketFlowResource.java | 17 +---- 3 files changed, 80 insertions(+), 38 deletions(-) diff --git a/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/RegistryService.java b/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/RegistryService.java index acc37be62..b03116251 100644 --- a/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/RegistryService.java +++ b/nifi-registry-framework/src/main/java/org/apache/nifi/registry/service/RegistryService.java @@ -150,7 +150,6 @@ public Bucket getBucket(final String bucketIdentifier) { final BucketEntity bucket = metadataService.getBucketById(bucketIdentifier); if (bucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -199,7 +198,6 @@ public Bucket updateBucket(final Bucket bucket) { final BucketEntity existingBucketById = metadataService.getBucketById(bucket.getIdentifier()); if (existingBucketById == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucket.getIdentifier()); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -244,7 +242,6 @@ public Bucket deleteBucket(final String bucketIdentifier) { final BucketEntity existingBucket = metadataService.getBucketById(bucketIdentifier); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -274,7 +271,6 @@ public List getBucketItems(final String bucketIdentifier) { final BucketEntity bucket = metadataService.getBucketById(bucketIdentifier); if (bucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -345,7 +341,6 @@ public VersionedFlow createFlow(final String bucketIdentifier, final VersionedFl final BucketEntity existingBucket = metadataService.getBucketById(bucketIdentifier); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -382,14 +377,12 @@ public VersionedFlow getFlow(final String bucketIdentifier, final String flowIde final BucketEntity existingBucket = metadataService.getBucketById(bucketIdentifier); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } final FlowEntity existingFlow = metadataService.getFlowByIdWithSnapshotCounts(flowIdentifier); if (existingFlow == null) { LOGGER.warn("The specified flow id [{}] does not exist.", flowIdentifier); - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); } @@ -413,7 +406,6 @@ public List getFlows(final String bucketId) { final BucketEntity existingBucket = metadataService.getBucketById(bucketId); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketId); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -448,14 +440,12 @@ public VersionedFlow updateFlow(final VersionedFlow versionedFlow) { final BucketEntity existingBucket = metadataService.getBucketById(versionedFlow.getBucketIdentifier()); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", versionedFlow.getBucketIdentifier()); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } final FlowEntity existingFlow = metadataService.getFlowByIdWithSnapshotCounts(versionedFlow.getIdentifier()); if (existingFlow == null) { LOGGER.warn("The specified flow id [{}] does not exist.", versionedFlow.getIdentifier()); - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); } @@ -507,7 +497,6 @@ public VersionedFlow deleteFlow(final String bucketIdentifier, final String flow final BucketEntity existingBucket = metadataService.getBucketById(bucketIdentifier); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -515,7 +504,6 @@ public VersionedFlow deleteFlow(final String bucketIdentifier, final String flow final FlowEntity existingFlow = metadataService.getFlowById(flowIdentifier); if (existingFlow == null) { LOGGER.warn("The specified flow id [{}] does not exist.", flowIdentifier); - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); } @@ -561,7 +549,6 @@ public VersionedFlowSnapshot createFlowSnapshot(final VersionedFlowSnapshot flow final BucketEntity existingBucket = metadataService.getBucketById(snapshotMetadata.getBucketIdentifier()); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", snapshotMetadata.getBucketIdentifier()); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -569,7 +556,6 @@ public VersionedFlowSnapshot createFlowSnapshot(final VersionedFlowSnapshot flow final FlowEntity existingFlow = metadataService.getFlowById(snapshotMetadata.getFlowIdentifier()); if (existingFlow == null) { LOGGER.warn("The specified flow id [{}] does not exist.", snapshotMetadata.getFlowIdentifier()); - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); } @@ -650,7 +636,6 @@ public VersionedFlowSnapshot getFlowSnapshot(final String bucketIdentifier, fina final BucketEntity existingBucket = metadataService.getBucketById(bucketIdentifier); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -658,7 +643,6 @@ public VersionedFlowSnapshot getFlowSnapshot(final String bucketIdentifier, fina final FlowEntity flowEntityWithCount = metadataService.getFlowByIdWithSnapshotCounts(flowIdentifier); if (flowEntityWithCount == null) { LOGGER.warn("The specified flow id [{}] does not exist.", flowIdentifier); - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); } @@ -670,7 +654,6 @@ public VersionedFlowSnapshot getFlowSnapshot(final String bucketIdentifier, fina final FlowSnapshotEntity snapshotEntity = metadataService.getFlowSnapshot(flowIdentifier, version); if (snapshotEntity == null) { LOGGER.warn("The specified flow snapshot id [{}] does not exist for version [{}].", flowIdentifier, version); - throw new ResourceNotFoundException("The specified versioned flow snapshot does not exist for this flow."); } @@ -725,7 +708,6 @@ public SortedSet getFlowSnapshots(final String bu final BucketEntity existingBucket = metadataService.getBucketById(bucketIdentifier); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -733,7 +715,6 @@ public SortedSet getFlowSnapshots(final String bu final FlowEntity existingFlow = metadataService.getFlowById(flowIdentifier); if (existingFlow == null) { LOGGER.warn("The specified flow id [{}] does not exist.", flowIdentifier); - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); } @@ -770,7 +751,6 @@ public VersionedFlowSnapshotMetadata getLatestFlowSnapshotMetadata(final String final BucketEntity existingBucket = metadataService.getBucketById(bucketIdentifier); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -778,7 +758,6 @@ public VersionedFlowSnapshotMetadata getLatestFlowSnapshotMetadata(final String final FlowEntity existingFlow = metadataService.getFlowById(flowIdentifier); if (existingFlow == null) { LOGGER.warn("The specified flow id [{}] does not exist.", flowIdentifier); - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); } @@ -788,6 +767,10 @@ public VersionedFlowSnapshotMetadata getLatestFlowSnapshotMetadata(final String // get latest snapshot for the flow final FlowSnapshotEntity latestSnapshot = metadataService.getLatestSnapshot(existingFlow.getId()); + if (latestSnapshot == null) { + throw new ResourceNotFoundException("The specified flow ID has no versions"); + } + return DataModelMapper.map(existingBucket, latestSnapshot); } finally { readLock.unlock(); @@ -813,7 +796,6 @@ public VersionedFlowSnapshotMetadata deleteFlowSnapshot(final String bucketIdent final BucketEntity existingBucket = metadataService.getBucketById(bucketIdentifier); if (existingBucket == null) { LOGGER.warn("The specified bucket id [{}] does not exist.", bucketIdentifier); - throw new ResourceNotFoundException("The specified bucket ID does not exist in this registry."); } @@ -821,7 +803,6 @@ public VersionedFlowSnapshotMetadata deleteFlowSnapshot(final String bucketIdent final FlowEntity existingFlow = metadataService.getFlowById(flowIdentifier); if (existingFlow == null) { LOGGER.warn("The specified flow id [{}] does not exist.", flowIdentifier); - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); } diff --git a/nifi-registry-framework/src/test/java/org/apache/nifi/registry/service/TestRegistryService.java b/nifi-registry-framework/src/test/java/org/apache/nifi/registry/service/TestRegistryService.java index 9b759d2f9..c35cda79a 100644 --- a/nifi-registry-framework/src/test/java/org/apache/nifi/registry/service/TestRegistryService.java +++ b/nifi-registry-framework/src/test/java/org/apache/nifi/registry/service/TestRegistryService.java @@ -32,6 +32,7 @@ import org.apache.nifi.registry.flow.VersionedProcessor; import org.apache.nifi.registry.serialization.Serializer; import org.apache.nifi.registry.serialization.VersionedProcessGroupSerializer; +import org.junit.Assert; import org.junit.Before; import org.junit.Test; import org.mockito.invocation.InvocationOnMock; @@ -862,6 +863,79 @@ public void testGetFlowSnapshotsWhenNoSnapshots() { assertEquals(0, retrievedSnapshots.size()); } + @Test + public void testGetLatestSnapshotMetadataWhenVersionsExist() { + final BucketEntity existingBucket = new BucketEntity(); + existingBucket.setId("b1"); + existingBucket.setName("My Bucket"); + existingBucket.setDescription("This is my bucket"); + existingBucket.setCreated(new Date()); + + when(metadataService.getBucketById(existingBucket.getId())).thenReturn(existingBucket); + + // return a flow with the existing snapshot when getFlowById is called + final FlowEntity existingFlow = new FlowEntity(); + existingFlow.setId("flow1"); + existingFlow.setName("My Flow"); + existingFlow.setDescription("This is my flow."); + existingFlow.setCreated(new Date()); + existingFlow.setModified(new Date()); + existingFlow.setBucketId(existingBucket.getId()); + + when(metadataService.getFlowById(existingFlow.getId())).thenReturn(existingFlow); + + final FlowSnapshotEntity existingSnapshot1 = new FlowSnapshotEntity(); + existingSnapshot1.setVersion(1); + existingSnapshot1.setFlowId(existingFlow.getId()); + existingSnapshot1.setCreatedBy("user1"); + existingSnapshot1.setCreated(new Date()); + existingSnapshot1.setComments("This is snapshot 1"); + + when(metadataService.getLatestSnapshot(existingFlow.getId())).thenReturn(existingSnapshot1); + + VersionedFlowSnapshotMetadata latestMetadata = registryService.getLatestFlowSnapshotMetadata(existingBucket.getId(), existingFlow.getId()); + assertNotNull(latestMetadata); + assertEquals(1, latestMetadata.getVersion()); + } + + @Test + public void testGetLatestSnapshotMetadataWhenNoVersionsExist() { + final BucketEntity existingBucket = new BucketEntity(); + existingBucket.setId("b1"); + existingBucket.setName("My Bucket"); + existingBucket.setDescription("This is my bucket"); + existingBucket.setCreated(new Date()); + + when(metadataService.getBucketById(existingBucket.getId())).thenReturn(existingBucket); + + // return a flow with the existing snapshot when getFlowById is called + final FlowEntity existingFlow = new FlowEntity(); + existingFlow.setId("flow1"); + existingFlow.setName("My Flow"); + existingFlow.setDescription("This is my flow."); + existingFlow.setCreated(new Date()); + existingFlow.setModified(new Date()); + existingFlow.setBucketId(existingBucket.getId()); + + when(metadataService.getFlowById(existingFlow.getId())).thenReturn(existingFlow); + + final FlowSnapshotEntity existingSnapshot1 = new FlowSnapshotEntity(); + existingSnapshot1.setVersion(1); + existingSnapshot1.setFlowId(existingFlow.getId()); + existingSnapshot1.setCreatedBy("user1"); + existingSnapshot1.setCreated(new Date()); + existingSnapshot1.setComments("This is snapshot 1"); + + when(metadataService.getLatestSnapshot(existingFlow.getId())).thenReturn(null); + + try { + registryService.getLatestFlowSnapshotMetadata(existingBucket.getId(), existingFlow.getId()); + Assert.fail("Should have thrown exception"); + } catch (ResourceNotFoundException e) { + assertEquals("The specified flow ID has no versions", e.getMessage()); + } + } + @Test(expected = ResourceNotFoundException.class) public void testGetSnapshotDoesNotExistInMetadataProvider() { final String bucketId = "b1"; diff --git a/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java b/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java index a53d6e248..64f183062 100644 --- a/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java +++ b/nifi-registry-web-api/src/main/java/org/apache/nifi/registry/web/api/BucketFlowResource.java @@ -27,7 +27,6 @@ import org.apache.commons.lang3.StringUtils; import org.apache.nifi.registry.bucket.BucketItem; import org.apache.nifi.registry.diff.VersionedFlowDifference; -import org.apache.nifi.registry.exception.ResourceNotFoundException; import org.apache.nifi.registry.flow.VersionedFlow; import org.apache.nifi.registry.flow.VersionedFlowSnapshot; import org.apache.nifi.registry.flow.VersionedFlowSnapshotMetadata; @@ -376,14 +375,8 @@ public Response getLatestFlowVersion( authorizeBucketAccess(RequestAction.READ, bucketId); - final VersionedFlowSnapshotMetadata latest = registryService.getLatestFlowSnapshotMetadata(bucketId, flowId); - if (latest == null) { - logger.warn("The specified flow id [{}] does not exist.", flowId); - - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); - } - - final VersionedFlowSnapshot lastSnapshot = registryService.getFlowSnapshot(bucketId, flowId, latest.getVersion()); + final VersionedFlowSnapshotMetadata latestMetadata = registryService.getLatestFlowSnapshotMetadata(bucketId, flowId); + final VersionedFlowSnapshot lastSnapshot = registryService.getFlowSnapshot(bucketId, flowId, latestMetadata.getVersion()); populateLinksAndPermissions(lastSnapshot); return Response.status(Response.Status.OK).entity(lastSnapshot).build(); @@ -418,12 +411,6 @@ public Response getLatestFlowVersionMetadata( authorizeBucketAccess(RequestAction.READ, bucketId); final VersionedFlowSnapshotMetadata latest = registryService.getLatestFlowSnapshotMetadata(bucketId, flowId); - if (latest == null) { - logger.warn("The specified flow id [{}] does not exist.", flowId); - - throw new ResourceNotFoundException("The specified flow ID does not exist in this bucket."); - } - linkService.populateSnapshotLinks(latest); return Response.status(Response.Status.OK).entity(latest).build();