Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

REP-45 Add option to delete replication data through UI #60

Merged
merged 9 commits into from
Apr 3, 2019
Merged
Show file tree
Hide file tree
Changes from 1 commit
Commits
File filter

Filter by extension

Filter by extension


Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 3 additions & 3 deletions admin/query/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -90,17 +90,17 @@
<limit implementation="org.codice.jacoco.LenientLimit">
<counter>INSTRUCTION</counter>
<value>COVEREDRATIO</value>
<minimum>0.72</minimum>
<minimum>0.75</minimum>
</limit>
<limit implementation="org.codice.jacoco.LenientLimit">
<counter>BRANCH</counter>
<value>COVEREDRATIO</value>
<minimum>0.68</minimum>
<minimum>0.74</minimum>
</limit>
<limit implementation="org.codice.jacoco.LenientLimit">
<counter>COMPLEXITY</counter>
<value>COVEREDRATIO</value>
<minimum>0.58</minimum>
<minimum>0.61</minimum>
</limit>
</limits>
</rule>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -76,12 +76,7 @@ public boolean siteExists(String name) {
}

public boolean siteIdExists(String id) {
try {
siteManager.get(id);
return true;
} catch (NotFoundException e) {
return false;
}
return siteManager.exists(id);
}

public ReplicationSiteField updateSite(String id, String name, AddressField address) {
Expand Down Expand Up @@ -114,7 +109,7 @@ public boolean replicationConfigExists(String name) {
}

public boolean configExists(String id) {
return configManager.configExists(id);
return configManager.exists(id);
}

public ReplicationField createReplication(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -416,7 +416,7 @@ public void enableConfig() {

@Test
public void siteIdExists() {
when(siteManager.get(anyString())).thenReturn(new ReplicationSiteImpl());
when(siteManager.exists(anyString())).thenReturn(true);
assertThat(utils.siteIdExists("id"), is(true));
}

Expand Down
4 changes: 2 additions & 2 deletions replication-api-impl/pom.xml
Original file line number Diff line number Diff line change
Expand Up @@ -331,12 +331,12 @@
<limit implementation="org.codice.jacoco.LenientLimit">
<counter>INSTRUCTION</counter>
<value>COVEREDRATIO</value>
<minimum>0.56</minimum>
<minimum>0.57</minimum>
</limit>
<limit implementation="org.codice.jacoco.LenientLimit">
<counter>BRANCH</counter>
<value>COVEREDRATIO</value>
<minimum>0.48</minimum>
<minimum>0.49</minimum>
</limit>
<limit implementation="org.codice.jacoco.LenientLimit">
<counter>COMPLEXITY</counter>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -150,15 +150,17 @@ private void deleteBatch(String[] idsToDelete) throws SourceUnavailableException
try {
framework.delete(new DeleteRequestImpl(idsToDelete));
} catch (IngestException ie) {
// nothing in batch was deleted, so...
deleteSequentially(idsToDelete);
}
}

// One metacard failing to delete will cause the entire batch to not be deleted. So,
// if the batch fails, perform the deletes individually and just skip over the ones that fail.
for (String id : idsToDelete) {
try {
framework.delete(new DeleteRequestImpl(id));
} catch (IngestException e) {
LOGGER.debug("Failed to delete metacard with id: {}", id, e);
}
private void deleteSequentially(String[] ids) throws SourceUnavailableException {
for (String id : ids) {
try {
framework.delete(new DeleteRequestImpl(id));
} catch (IngestException e) {
LOGGER.debug("Failed to delete metacard with id: {}", id, e);
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@
import java.util.Map;
import java.util.Optional;
import java.util.stream.Collectors;
import org.apache.commons.collections4.CollectionUtils;
import org.codice.ddf.persistence.PersistenceException;
import org.codice.ddf.persistence.PersistentItem;
import org.codice.ddf.persistence.PersistentStore;
Expand Down Expand Up @@ -78,7 +79,7 @@ public Optional<ReplicationItem> getItem(String metacardId, String source, Strin
return Optional.empty();
}

if (matchingPersistentItems == null || matchingPersistentItems.isEmpty()) {
if (CollectionUtils.isEmpty(matchingPersistentItems)) {
LOGGER.debug(
"couldn't find persisted item with id: {}, source: {}, and destination: {}. This is expected during initial replication.",
metacardId,
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -52,8 +52,9 @@ public class ReplicatorConfigImpl extends AbstractPersistable implements Replica
* <li>0 (No version) - initial version of configs which were saved in the catalog framework
* <li>1 - The first version of configs to be saved in the replication persistent store
* <ul>
* <li>Add <b>suspended</b> field of type boolean
* <li>Add <b>deleted</b> field of type boolean
* <li>Add <b>suspended</b> field of type boolean with default of false
* <li>Add <b>deleted</b> field of type boolean with default of false
* <li>Add <b>deleteData</b> field of type boolean with default of false
* </ul>
* </ol>
*/
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -47,9 +47,9 @@ public void remove(String id) {
}

@Override
public boolean configExists(String configId) {
public boolean exists(String id) {
try {
persistentStore.get(ReplicatorConfigImpl.class, configId);
persistentStore.get(ReplicatorConfigImpl.class, id);
} catch (NotFoundException | ReplicationPersistenceException e) {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

we might want the ReplicationPersistenceException to bubble out, rather than just returning false when we actually don't know. Same goes for this method in the SiteManagerImpl

return false;
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -92,4 +92,14 @@ public void save(ReplicationSite site) {
public void remove(String id) {
persistentStore.delete(ReplicationSiteImpl.class, id);
}

@Override
public boolean exists(String id) {
try {
persistentStore.get(ReplicationSiteImpl.class, id);
} catch (NotFoundException | ReplicationPersistenceException e) {
return false;
}
return true;
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -13,6 +13,8 @@
*/
package org.codice.ditto.replication.api.impl.persistence;

import static org.hamcrest.CoreMatchers.is;
import static org.hamcrest.MatcherAssert.assertThat;
import static org.mockito.Matchers.any;
import static org.mockito.Matchers.anyString;
import static org.mockito.Matchers.eq;
Expand All @@ -21,6 +23,8 @@
import static org.mockito.Mockito.when;

import java.util.stream.Stream;
import javax.ws.rs.NotFoundException;
import org.codice.ditto.replication.api.ReplicationPersistenceException;
import org.codice.ditto.replication.api.data.ReplicatorConfig;
import org.codice.ditto.replication.api.impl.data.ReplicatorConfigImpl;
import org.junit.Before;
Expand All @@ -32,7 +36,7 @@
@RunWith(MockitoJUnitRunner.class)
public class ReplicatorConfigManagerImplTest {

ReplicatorConfigManagerImpl manager;
private ReplicatorConfigManagerImpl manager;

@Mock ReplicationPersistentStore persistentStore;

Expand Down Expand Up @@ -73,4 +77,22 @@ public void removeConfig() {
manager.remove("id");
verify(persistentStore).delete(eq(ReplicatorConfigImpl.class), anyString());
}

@Test
public void testExistsNotFound() {
when(persistentStore.get(ReplicatorConfigImpl.class, "id")).thenThrow(NotFoundException.class);
assertThat(manager.exists("id"), is(false));
}

@Test
public void testExistsErrorAccessingPersistenceStore() {
when(persistentStore.get(ReplicatorConfigImpl.class, "id"))
.thenThrow(ReplicationPersistenceException.class);
assertThat(manager.exists("id"), is(false));
}

@Test
public void testExistsConfigFound() {
assertThat(manager.exists("id"), is(true));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import javax.ws.rs.NotFoundException;
import org.codice.ddf.configuration.SystemBaseUrl;
import org.codice.ddf.configuration.SystemInfo;
import org.codice.ditto.replication.api.ReplicationPersistenceException;
import org.codice.ditto.replication.api.data.ReplicationSite;
import org.codice.ditto.replication.api.impl.data.ReplicationSiteImpl;
import org.junit.Before;
Expand All @@ -41,20 +42,20 @@ public class SiteManagerImplTest {

private static final String LOCAL_SITE_ID = "local-site-id-1234567890";

SiteManagerImpl store;
private SiteManagerImpl siteManager;

@Mock ReplicationPersistentStore persistentStore;

@Before
public void setUp() {
System.setProperty("org.codice.ddf.system.siteName", "testSite");
store = new SiteManagerImpl(persistentStore);
siteManager = new SiteManagerImpl(persistentStore);
}

@Test
public void init() {
when(persistentStore.get(any(Class.class), anyString())).thenThrow(new NotFoundException());
store.init();
siteManager.init();
ArgumentCaptor<ReplicationSiteImpl> captor = ArgumentCaptor.forClass(ReplicationSiteImpl.class);
verify(persistentStore).save(captor.capture());
ReplicationSite site = captor.getValue();
Expand All @@ -69,7 +70,7 @@ public void initUpdateName() {
orig.setName("oldName");
orig.setUrl(SystemBaseUrl.EXTERNAL.getBaseUrl());
when(persistentStore.get(eq(ReplicationSiteImpl.class), anyString())).thenReturn(orig);
store.init();
siteManager.init();
ArgumentCaptor<ReplicationSiteImpl> captor = ArgumentCaptor.forClass(ReplicationSiteImpl.class);
verify(persistentStore).save(captor.capture());
ReplicationSiteImpl site = captor.getValue();
Expand All @@ -84,7 +85,7 @@ public void initUpdateURL() {
orig.setName(SystemInfo.getSiteName());
orig.setUrl("https://asdf:1234");
when(persistentStore.get(eq(ReplicationSiteImpl.class), anyString())).thenReturn(orig);
store.init();
siteManager.init();
ArgumentCaptor<ReplicationSiteImpl> captor = ArgumentCaptor.forClass(ReplicationSiteImpl.class);
verify(persistentStore).save(captor.capture());
ReplicationSiteImpl site = captor.getValue();
Expand All @@ -99,7 +100,7 @@ public void initNoOp() {
orig.setName(SystemInfo.getSiteName());
orig.setUrl(SystemBaseUrl.EXTERNAL.getBaseUrl());
when(persistentStore.get(eq(ReplicationSiteImpl.class), anyString())).thenReturn(orig);
store.init();
siteManager.init();
verify(persistentStore, never()).save(any(ReplicationSiteImpl.class));
}

Expand All @@ -109,25 +110,43 @@ public void getSites() {
Stream<ReplicationSiteImpl> siteStream = Stream.of(site);
when(persistentStore.objects(eq(ReplicationSiteImpl.class))).thenReturn(siteStream);

assertThat(store.objects().anyMatch(site::equals), is(true));
assertThat(siteManager.objects().anyMatch(site::equals), is(true));
}

@Test
public void createSite() {
ReplicationSite site = store.createSite("name", "url");
ReplicationSite site = siteManager.createSite("name", "url");
assertThat(site.getName(), is("name"));
assertThat(site.getUrl(), is("url"));
}

@Test(expected = IllegalArgumentException.class)
public void saveSiteBadSite() {
ReplicationSite site = mock(ReplicationSite.class);
store.save(site);
siteManager.save(site);
}

@Test
public void deleteSite() {
store.remove("id");
siteManager.remove("id");
verify(persistentStore).delete(eq(ReplicationSiteImpl.class), eq("id"));
}

@Test
public void testExistsNotFound() {
when(persistentStore.get(ReplicationSiteImpl.class, "id")).thenThrow(NotFoundException.class);
assertThat(siteManager.exists("id"), is(false));
}

@Test
public void testExistsErrorAccessingPersistenceStore() {
when(persistentStore.get(ReplicationSiteImpl.class, "id"))
.thenThrow(ReplicationPersistenceException.class);
assertThat(siteManager.exists("id"), is(false));
}

@Test
public void testExistsConfigFound() {
assertThat(siteManager.exists("id"), is(true));
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -162,5 +162,11 @@ public interface ReplicatorConfig extends Persistable {
*/
boolean deleteData();
peterhuffer marked this conversation as resolved.
Show resolved Hide resolved

/**
* See {@link #deleteData()}.
*
* @param deleteData {@code true} if this {@code ReplicatorConfig}'s data should be deleted,
* otherwise false
*/
void setDeleteData(boolean deleteData);
}
Original file line number Diff line number Diff line change
Expand Up @@ -54,4 +54,10 @@ public interface DataManager<T extends Persistable> {
* @throws NotFoundException if an object with the given id cannot be found
*/
void remove(String id);

/**
* @param id unique id of the {@link Persistable}
* @return {@code true} if the {@link Persistable} exists, otherwise {@code false}.
*/
boolean exists(String id);
}
Original file line number Diff line number Diff line change
Expand Up @@ -15,12 +15,5 @@

import org.codice.ditto.replication.api.data.ReplicatorConfig;

/** Performs CRUD operations for {@link ReplicatorConfig}. */
public interface ReplicatorConfigManager extends DataManager<ReplicatorConfig> {

/**
* @param configId unique id of {@link ReplicatorConfig}
* @return {@code true} if the config exists, otherwise {@code false}.
*/
boolean configExists(String configId);
}
/** Performs CRUD operations for {@link ReplicatorConfig}s. */
public interface ReplicatorConfigManager extends DataManager<ReplicatorConfig> {}