Skip to content

Commit

Permalink
Address comments
Browse files Browse the repository at this point in the history
  • Loading branch information
peterhuffer committed Apr 2, 2019
1 parent dcc7c17 commit fe6316b
Show file tree
Hide file tree
Showing 15 changed files with 46 additions and 55 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ public class DeleteReplication extends BaseFunctionField<BooleanField> {
public static final String FIELD_NAME = "deleteReplication";

public static final String DESCRIPTION =
"Deletes a Replication. Optionally delete the data of the Replication. Deleting data will delete "
"Deletes a Replication and its history (statistics). Optionally delete the data of the Replication. Deleting data will delete "
+ "any local resources and metadata that were replicated by this Replication, but not any resources replicated to a remote Node.";

public static final BooleanField RETURN_TYPE = new BooleanField();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -301,7 +301,7 @@ public void testMarkConfigDeleted() {
when(configManager.get("id")).thenReturn(config);

assertThat(utils.markConfigDeleted("id", true), is(true));
assertThat(config.deleteData(), is(true));
assertThat(config.shouldDeleteData(), is(true));
assertThat(config.isDeleted(), is(true));
assertThat(config.isSuspended(), is(true));
verify(replicator).cancelSyncRequest("id");
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,9 +37,9 @@ public class DeleteReplicationTest {

private static final String ID = "abc123";

DeleteReplication deleteReplication;
private DeleteReplication deleteReplication;

Map<String, Object> args;
private Map<String, Object> args;

@Mock ReplicationUtils utils;

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -37,8 +37,8 @@

/**
* Periodically polls for available {@link ReplicatorConfig}s and deletes them based on the {@link
* ReplicatorConfig#isDeleted()} and {@link ReplicatorConfig#deleteData()} properties. A {@link
* ReplicatorConfig} marked as deleted always has its history deleted.
* ReplicatorConfig#isDeleted()} and {@link ReplicatorConfig#shouldDeleteData()} properties. A
* {@link ReplicatorConfig} marked as deleted always has its history deleted.
*/
public class ScheduledReplicatorDeleter {

Expand Down Expand Up @@ -77,6 +77,8 @@ public ScheduledReplicatorDeleter(
DEFAULT_PAGE_SIZE);
}

@VisibleForTesting
@SuppressWarnings("squid:S00107" /* Only for testing */)
ScheduledReplicatorDeleter(
ReplicatorConfigManager replicatorConfigManager,
ScheduledExecutorService scheduledExecutorService,
Expand Down Expand Up @@ -105,7 +107,7 @@ public void destroy() {
scheduledExecutorService.shutdownNow();
}

public void cleanup() {
void cleanup() {
security.runAsAdmin(
() -> {
try {
Expand Down Expand Up @@ -195,7 +197,7 @@ private void cleanupDeletedConfigs(List<ReplicatorConfig> replicatorConfigs) {
final String configId = config.getId();
final String configName = config.getName();

if (config.deleteData()) {
if (config.shouldDeleteData()) {
try {
deleteMetacards(configId);
} catch (PersistenceException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -96,7 +96,7 @@ public Map<String, Object> toMap() {
result.put(DESCRIPTION_KEY, getDescription());
result.put(SUSPENDED_KEY, Boolean.toString(isSuspended()));
result.put(DELETED_KEY, Boolean.toString(isDeleted()));
result.put(DELETE_DATA_KEY, Boolean.toString(deleteData()));
result.put(DELETE_DATA_KEY, Boolean.toString(shouldDeleteData()));
return result;
}

Expand Down Expand Up @@ -206,7 +206,7 @@ public void setDeleted(boolean deleted) {
}

@Override
public boolean deleteData() {
public boolean shouldDeleteData() {
return deleteData;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -2,7 +2,6 @@

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.codice.ditto.replication.api.persistence.ReplicatorConfigManager;
Expand Down Expand Up @@ -50,7 +49,7 @@ public void remove(String id) {
public boolean exists(String id) {
try {
persistentStore.get(ReplicatorConfigImpl.class, id);
} catch (NotFoundException | ReplicationPersistenceException e) {
} catch (NotFoundException e) {
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -97,7 +97,7 @@ public void remove(String id) {
public boolean exists(String id) {
try {
persistentStore.get(ReplicationSiteImpl.class, id);
} catch (NotFoundException | ReplicationPersistenceException e) {
} catch (NotFoundException e) {
return false;
}
return true;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -330,7 +330,7 @@ private ReplicatorConfig mockConfig(
when(config.getId()).thenReturn(id);
when(config.getName()).thenReturn(name);
when(config.isDeleted()).thenReturn(isDeleted);
when(config.deleteData()).thenReturn(deleteData);
when(config.shouldDeleteData()).thenReturn(deleteData);
return config;
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,6 @@

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 Down Expand Up @@ -84,13 +83,6 @@ public void testExistsNotFound() {
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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,7 +27,6 @@
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 Down Expand Up @@ -138,13 +137,6 @@ public void testExistsNotFound() {
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));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -138,7 +138,7 @@ public interface ReplicatorConfig extends Persistable {
void setSuspended(boolean suspended);

/**
* See {@link #deleteData()}.
* See {@link #shouldDeleteData()}.
*
* @return whether or not this {@code ReplicatorConfig} should be considered as deleted
*/
Expand All @@ -147,7 +147,7 @@ public interface ReplicatorConfig extends Persistable {
/**
* Marks this {@code ReplicatorConfig} as deleted.
*
* @param deleted
* @param deleted whether or not this {@code ReplicatorConfig} should be considered as deleted
*/
void setDeleted(boolean deleted);

Expand All @@ -160,10 +160,10 @@ public interface ReplicatorConfig extends Persistable {
* @return if {@code true}, delete the associated data replicated by this {@code
* ReplicatorConfig}, otherwise retain the data.
*/
boolean deleteData();
boolean shouldDeleteData();

/**
* See {@link #deleteData()}.
* See {@link #shouldDeleteData()}.
*
* @param deleteData {@code true} if this {@code ReplicatorConfig}'s data should be deleted,
* otherwise false
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -21,8 +21,9 @@ public interface DataManager<T extends Persistable> {
*
* @param id The object id
* @return the T object with the given id
* @throws ReplicationPersistenceException if an error occurs while trying to retrieve the object
* @throws NotFoundException if an object with the given id cannot be found
* @throws {@link org.codice.ditto.replication.api.ReplicationPersistenceException} if an error
* occurs while trying to retrieve the object
* @throws {@link javax.ws.rs.NotFoundException} if an object with the given id cannot be found
* @throws IllegalStateException if multiple objects were found with the given id
*/
T get(String id);
Expand All @@ -31,7 +32,8 @@ public interface DataManager<T extends Persistable> {
* Gets all the currently saved T objects
*
* @return Stream of all T objects
* @throws ReplicationPersistenceException if an error occurs while trying to retrieve the objects
* @throws {@link org.codice.ditto.replication.api.ReplicationPersistenceException} if an error
* occurs while trying to retrieve the objects
*/
Stream<T> objects();

Expand All @@ -41,7 +43,8 @@ public interface DataManager<T extends Persistable> {
* method included in this interface.
*
* @param object The T object to save or update
* @throws ReplicationPersistenceException if an error occurs while trying to save the config
* @throws {@link org.codice.ditto.replication.api.ReplicationPersistenceException} if an error
* occurs while trying to save the config
* @throws IllegalArgumentException if the T implementation is not one that can be saved
*/
void save(T object);
Expand All @@ -50,14 +53,17 @@ public interface DataManager<T extends Persistable> {
* Deletes a T object with the given id
*
* @param id The id of the object to be removed
* @throws ReplicationPersistenceException if an error occurs while trying to delete the config
* @throws NotFoundException if an object with the given id cannot be found
* @throws {@link org.codice.ditto.replication.api.ReplicationPersistenceException} if an error
* occurs while trying to delete the config
* @throws {@link javax.ws.rs.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}.
* @throws {@link org.codice.ditto.replication.api.ReplicationPersistenceException} if there is an
* error accessing storage
*/
boolean exists(String id);
}
4 changes: 2 additions & 2 deletions ui/package.json
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@
"graphql-tag": "2.10.0",
"immutable": "3.8.2",
"moment": "2.24.0",
"notistack": "0.5.0",
"notistack": "0.6.1",
"prop-types": "15.6.2",
"react": "16.8.3",
"react-apollo": "2.3.3",
Expand Down Expand Up @@ -76,7 +76,7 @@
"global": {
"statements": 6,
"branches": 6,
"lines": 7,
"lines": 6,
"functions": 1
}
},
Expand Down
18 changes: 9 additions & 9 deletions ui/src/main/webapp/components/replications/ActionsMenu.js
Original file line number Diff line number Diff line change
Expand Up @@ -30,7 +30,7 @@ import {
import { allReplications } from './gql/queries'
import { Mutation } from 'react-apollo'
import Replications from './replications'
import { withSnackbar } from 'notistack'
import { withSnackbar, useSnackbar } from 'notistack'
import Confirmable from '../common/Confirmable'

const DeleteReplication = withSnackbar(
Expand Down Expand Up @@ -119,8 +119,9 @@ const DeleteReplication = withSnackbar(
)
DeleteReplication.displayName = 'DeleteReplication'

const SuspendReplication = withSnackbar(props => {
const { id, suspend, key, label, onClose, enqueueSnackbar, name } = props
const SuspendReplication = props => {
const { id, suspend, key, label, onClose, name } = props
const { enqueueSnackbar } = useSnackbar()

return (
<Mutation mutation={suspendReplication}>
Expand Down Expand Up @@ -167,11 +168,11 @@ const SuspendReplication = withSnackbar(props => {
)}
</Mutation>
)
})
SuspendReplication.displayName = 'SuspendReplication'
}

const CancelReplication = withSnackbar(props => {
const { id, onClose, name, enqueueSnackbar } = props
const CancelReplication = props => {
const { id, onClose, name } = props
const { enqueueSnackbar } = useSnackbar()

return (
<Mutation mutation={cancelReplication}>
Expand Down Expand Up @@ -218,8 +219,7 @@ const CancelReplication = withSnackbar(props => {
)}
</Mutation>
)
})
CancelReplication.displayName = 'CancelReplication'
}

const ActionsMenu = function(props) {
const { menuId, anchorEl = null, onClose, replication } = props
Expand Down
8 changes: 4 additions & 4 deletions ui/yarn.lock
Original file line number Diff line number Diff line change
Expand Up @@ -5406,10 +5406,10 @@ normalize-scroll-left@^0.1.2:
resolved "https://registry.yarnpkg.com/normalize-scroll-left/-/normalize-scroll-left-0.1.2.tgz#6b79691ba79eb5fb107fa5edfbdc06b55caee2aa"
integrity sha512-F9YMRls0zCF6BFIE2YnXDRpHPpfd91nOIaNdDgrx5YMoPLo8Wqj+6jNXHQsYBavJeXP4ww8HCt0xQAKc5qk2Fg==

notistack@0.5.0:
version "0.5.0"
resolved "https://registry.yarnpkg.com/notistack/-/notistack-0.5.0.tgz#4fece949c973cad6d1d4a9d34da8024ef3158b5e"
integrity sha512-DYAix/jnN/Qv/AHwgbNFcQ2d6HjeQ8duEge6OJr5rn0pmxUutQjuGpZ+3dJ/4FBJtfRbnv0BX59ZtHrn2tE+Jw==
notistack@0.6.1:
version "0.6.1"
resolved "https://registry.yarnpkg.com/notistack/-/notistack-0.6.1.tgz#d29a7b85deb67ed144e5427438280961615c7b1b"
integrity sha512-tuXmw/0TBmdeHqRjWQwf9Jh298OMEwi18mrnTwxV4IfkxO0n2AplTj/WM9D9kV7aEmMjHTcHixRzllZUou1mAw==
dependencies:
classnames "^2.2.6"
prop-types "^15.6.2"
Expand Down

0 comments on commit fe6316b

Please sign in to comment.