Skip to content

Commit

Permalink
Merge pull request #695 from MrCreosote/develop
Browse files Browse the repository at this point in the history 
Integrate the 2 ws meta alteration methods & make all changes in 1 shot
  • Loading branch information
@MrCreosote
MrCreosote authored Oct 13, 2023
2 parents b02d1b1 + abe48d1 commit 37acadd
Show file tree
Hide file tree
Showing 5 changed files with 120 additions and 137 deletions.
30 changes: 10 additions & 20 deletions src/us/kbase/workspace/database/Workspace.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -223,26 +223,16 @@ public void setWorkspaceMetadata(
final ResolvedWorkspaceID wsid = new PermissionsCheckerFactory(db, user)
.getWorkspaceChecker(wsi, Permission.ADMIN)
.withOperation("alter metadata for").check();
boolean set = false;
Instant time = null;
// should merge this into one db call
try {
if (meta != null && !meta.isEmpty()) {
time = db.setWorkspaceMeta(wsid, meta);
set = true;
}
if (keysToRemove != null) {
noNulls(keysToRemove, "null metadata keys are not allowed");
for (final String key: keysToRemove) {
time = db.removeWorkspaceMetaKey(wsid, key);
set = true;
}
}
} finally {
if (set) {
for (final WorkspaceEventListener l: listeners) {
l.setWorkspaceMetadata(user, wsid.getID(), time);
}
if (keysToRemove == null && (meta == null || meta.isEmpty())) {
return;
}
if (keysToRemove != null) {
noNulls(keysToRemove, "null metadata keys are not allowed");
}
final Optional<Instant> time = db.setWorkspaceMeta(wsid, meta, keysToRemove);
if (time.isPresent()) {
for (final WorkspaceEventListener l: listeners) {
l.setWorkspaceMetadata(user, wsid.getID(), time.get());
}
}
}
Expand Down
19 changes: 8 additions & 11 deletions src/us/kbase/workspace/database/WorkspaceDatabase.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -91,24 +91,21 @@ WorkspaceInformation createWorkspace(WorkspaceUser owner,
*
* @param wsid the workspace for which metadata will be altered.
* @param meta the metadata to add to the workspace.
* @return the workspace modification time.
* @param remove the metadata keys to remove from the workspace. If any provided keys do
* not exist they will be ignored.
* @return the workspace modification time if the metadata was altered, or an empty optional
* if the changes had no practical effect.
* @throws WorkspaceCommunicationException if a communication error occurs.
* @throws CorruptWorkspaceDBException if the workspace database is corrupt.
* @throws IllegalArgumentException if no metadata is supplied or the
* updated metadata exceeds the allowed size.
*/
Instant setWorkspaceMeta(ResolvedWorkspaceID wsid, WorkspaceUserMetadata meta)
Optional<Instant> setWorkspaceMeta(
ResolvedWorkspaceID wsid,
WorkspaceUserMetadata meta,
List<String> remove)
throws WorkspaceCommunicationException, CorruptWorkspaceDBException;

/** Remove a metadata key from a workspace.
* @param wsid the workspace for which metadata will be altered.
* @param key the key to remove from the metadata.
* @return the workspace modification time.
* @throws WorkspaceCommunicationException if a communication error occurs.
*/
Instant removeWorkspaceMetaKey(ResolvedWorkspaceID wsid, String key)
throws WorkspaceCommunicationException;

/** Clone a workspace.
* @param user the user cloning the workspace
* @param wsid the ID of the workspace to be cloned.
Expand Down
52 changes: 22 additions & 30 deletions src/us/kbase/workspace/database/mongo/MongoWorkspaceDB.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -699,14 +699,16 @@ private void setCreatedWorkspacePermissions(
private static final Set<String> FLDS_WS_META = newHashSet(Fields.WS_META);

@Override
public Instant setWorkspaceMeta(
public Optional<Instant> setWorkspaceMeta(
final ResolvedWorkspaceID rwsi,
final WorkspaceUserMetadata newMeta)
final WorkspaceUserMetadata newMeta,
final List<String> remove)
throws WorkspaceCommunicationException, CorruptWorkspaceDBException {
// TODO NOW CODE integrate keys to remove (below) into this method and do it all in
// one shot
if (newMeta == null || newMeta.isEmpty()) {
throw new IllegalArgumentException("Metadata cannot be null or empty");
if (remove == null && (newMeta == null || newMeta.isEmpty())) {
throw new IllegalArgumentException("No metadata changes provided");
}
if (remove != null) {
noNulls(remove, "Null metadata keys found in remove parameter");
}
int attempts = 1;
Instant time = null;
Expand All @@ -715,8 +717,19 @@ public Instant setWorkspaceMeta(
@SuppressWarnings("unchecked")
final List<Map<String, String>> mlist = (List<Map<String, String>>)
ws.get(Fields.WS_META);
final Map<String, String> updatedMeta = metaMongoArrayToHash(mlist);
updatedMeta.putAll(newMeta.getMetadata());
final Map<String, String> oldMeta = metaMongoArrayToHash(mlist);
final Map<String, String> updatedMeta = new HashMap<>(oldMeta);
if (newMeta != null) {
updatedMeta.putAll(newMeta.getMetadata());
}
if (remove != null) {
for (final String k: remove) {
updatedMeta.remove(k);
}
}
if (oldMeta.equals(updatedMeta)) {
return Optional.empty();
}
try {
WorkspaceUserMetadata.checkMetadataSize(updatedMeta);
} catch (MetadataException me) {
Expand All @@ -731,7 +744,7 @@ public Instant setWorkspaceMeta(
time = _internal_setWorkspaceMeta(attempts, 5, query, metaUpdate);
attempts++;
}
return time;
return Optional.of(time);
}

// split the method for testing purposes
Expand Down Expand Up @@ -761,27 +774,6 @@ private Instant _internal_setWorkspaceMeta(
}
}

@Override
public Instant removeWorkspaceMetaKey(
final ResolvedWorkspaceID rwsi,
final String key)
throws WorkspaceCommunicationException {
final Instant time = Instant.now();
final String mkey = Fields.WS_META + Fields.FIELD_SEP + Fields.META_KEY;
try {
wsmongo.getCollection(COL_WORKSPACES).updateOne(
new Document(Fields.WS_ID, rwsi.getID()).append(mkey, key),
new Document(
"$pull", new Document(Fields.WS_META,
new Document(Fields.META_KEY, key)))
.append("$set", new Document(
Fields.WS_MODDATE, Date.from(time))));
} catch (MongoException me) {
throw new WorkspaceCommunicationException(ERR_DB_COMM, me);
}
return time;
}

private static final Set<String> FLDS_CLONE_WS =
newHashSet(Fields.OBJ_ID, Fields.OBJ_NAME, Fields.OBJ_DEL, Fields.OBJ_HIDE);

Expand Down
93 changes: 77 additions & 16 deletions src/us/kbase/workspace/test/database/mongo/MongoWorkspaceDBTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -857,22 +857,28 @@ public void setWorkspaceMetadata() throws Exception {
when(mocks.clockmock.instant()).thenReturn(inst(10000), inst(15000), null);

// test against empty metadata
final Instant result = mocks.mdb.setWorkspaceMeta(
final Optional<Instant> result = mocks.mdb.setWorkspaceMeta(
rwsi,
new WorkspaceUserMetadata(ImmutableMap.of("foo", "bar", "baz", "bat")));
new WorkspaceUserMetadata(ImmutableMap.of(
"foo", "bar", "baz", "bat", "to_remove", "a")),
Arrays.asList("no_key_present")
);

assertThat("incorrect time", result, is(inst(10000)));
assertThat("incorrect time", result, is(Optional.of(inst(10000))));

final Map<String, String> m = mocks.mdb.getWorkspaceInformation(user, rwsi)
.getUserMeta().getMetadata();
assertThat("incorrect meta", m, is(ImmutableMap.of("foo", "bar", "baz", "bat")));
assertThat("incorrect meta", m, is(ImmutableMap.of(
"foo", "bar", "baz", "bat", "to_remove", "a")));

// test against non-empty metadata
final Instant result2 = mocks.mdb.setWorkspaceMeta(
final Optional<Instant> result2 = mocks.mdb.setWorkspaceMeta(
rwsi,
new WorkspaceUserMetadata(ImmutableMap.of("baz", "bing", "thingy", "thinger")));
new WorkspaceUserMetadata(ImmutableMap.of("baz", "bing", "thingy", "thinger")),
Arrays.asList("to_remove", "somecrap")
);

assertThat("incorrect time", result2, is(inst(15000)));
assertThat("incorrect time", result2, is(Optional.of(inst(15000))));

final Map<String, String> m2 = mocks.mdb.getWorkspaceInformation(
user, rwsi).getUserMeta().getMetadata();
Expand All @@ -881,14 +887,66 @@ public void setWorkspaceMetadata() throws Exception {
}

@Test
public void setWorkspaceMetaFailNoMeta() throws Exception {
public void setWorkspaceMetadataRemoveOnly() throws Exception {
final PartialMock mocks = new PartialMock(MONGO_DB);
final WorkspaceUser user = new WorkspaceUser("a");
final ResolvedWorkspaceID rwsi = new ResolvedWorkspaceID(1, "wsn", false, false);

mocks.mdb.createWorkspace(user, "wsn", false, null, new WorkspaceUserMetadata(
ImmutableMap.of("x", "y", "a", "b", "leave", "me alone")));

when(mocks.clockmock.instant()).thenReturn(inst(10000), (Instant) null);

final Optional<Instant> result = mocks.mdb.setWorkspaceMeta(
rwsi,
null,
Arrays.asList("x", "a")
);

assertThat("incorrect time", result, is(Optional.of(inst(10000))));

final Map<String, String> m = mocks.mdb.getWorkspaceInformation(user, rwsi)
.getUserMeta().getMetadata();
assertThat("incorrect meta", m, is(ImmutableMap.of("leave", "me alone")));
}

@Test
public void setWorkspaceMetadataNoop() throws Exception {
// Tests the case where if the DB was updated with the provided changes the actual
// metadata wouldn't change
final PartialMock mocks = new PartialMock(MONGO_DB);
final WorkspaceUser user = new WorkspaceUser("a");
final ResolvedWorkspaceID rwsi = new ResolvedWorkspaceID(1, "wsn", false, false);

mocks.mdb.createWorkspace(user, "wsn", false, null, new WorkspaceUserMetadata(
ImmutableMap.of("a", "b", "c", "d")));

when(mocks.clockmock.instant()).thenReturn(inst(10000), (Instant) null);

final Optional<Instant> result = mocks.mdb.setWorkspaceMeta(
rwsi,
new WorkspaceUserMetadata(ImmutableMap.of("a", "b", "x", "y")),
Arrays.asList("x", "z")
);

assertThat("incorrect time", result, is(Optional.empty()));

final Map<String, String> m = mocks.mdb.getWorkspaceInformation(user, rwsi)
.getUserMeta().getMetadata();
assertThat("incorrect meta", m, is(ImmutableMap.of("a", "b", "c", "d")));
}

@Test
public void setWorkspaceMetaFailBadInput() throws Exception {
final PartialMock mocks = new PartialMock(MONGO_DB);
final ResolvedWorkspaceID rwsi = new ResolvedWorkspaceID(1, "wsn", false, false);
failSetWorkspaceMeta(mocks.mdb, rwsi, null, new IllegalArgumentException(
"Metadata cannot be null or empty"));
failSetWorkspaceMeta(mocks.mdb, rwsi, null, null, new IllegalArgumentException(
"No metadata changes provided"));
final WorkspaceUserMetadata meta = new WorkspaceUserMetadata();
failSetWorkspaceMeta(mocks.mdb, rwsi, meta, new IllegalArgumentException(
"Metadata cannot be null or empty"));
failSetWorkspaceMeta(mocks.mdb, rwsi, meta, null, new IllegalArgumentException(
"No metadata changes provided"));
failSetWorkspaceMeta(mocks.mdb, rwsi, null, Arrays.asList("foo", null),
new NullPointerException("Null metadata keys found in remove parameter"));
}

private List<Object> setWorkspaceMetaBigMetaSetup() throws Exception {
Expand All @@ -910,6 +968,7 @@ private List<Object> setWorkspaceMetaBigMetaSetup() throws Exception {

@Test
public void setWorkspaceMetaPassBigMeta() throws Exception {
// also tests setting metadata without the remove parameter
final List<Object> setup = setWorkspaceMetaBigMetaSetup();
final PartialMock mocks = (PartialMock) setup.get(0);
final ResolvedWorkspaceID rwsi = (ResolvedWorkspaceID) setup.get(1);
Expand All @@ -923,9 +982,10 @@ public void setWorkspaceMetaPassBigMeta() throws Exception {
when(mocks.clockmock.instant()).thenReturn(inst(10000), (Instant) null);

// test against empty metadata
final Instant result = mocks.mdb.setWorkspaceMeta(rwsi, new WorkspaceUserMetadata(meta2));
final Optional<Instant> result = mocks.mdb.setWorkspaceMeta(
rwsi, new WorkspaceUserMetadata(meta2), null);

assertThat("incorrect time", result, is(inst(10000)));
assertThat("incorrect time", result, is(Optional.of(inst(10000))));

final Map<String, String> m2 = mocks.mdb.getWorkspaceInformation(
new WorkspaceUser("a"), rwsi).getUserMeta().getMetadata();
Expand All @@ -950,18 +1010,19 @@ public void setWorkspaceMetaFailBigMeta() throws Exception {
final Map<String, String> meta2 = (Map<String, String>) setup.get(3);
meta2.put("b10", TestCommon.LONG1001.substring(0, 725));

failSetWorkspaceMeta(mocks.mdb, rwsi, new WorkspaceUserMetadata(meta2),
failSetWorkspaceMeta(mocks.mdb, rwsi, new WorkspaceUserMetadata(meta2), null,
new IllegalArgumentException("Updated metadata exceeds allowed size of 16000B"));
}

private void failSetWorkspaceMeta(
final MongoWorkspaceDB mdb,
final ResolvedWorkspaceID rwsi,
final WorkspaceUserMetadata meta,
final List<String> remove,
final Exception expected)
throws Exception {
try {
mdb.setWorkspaceMeta(rwsi, meta);
mdb.setWorkspaceMeta(rwsi, meta, remove);
fail("expected exception");
} catch (Exception got) {
TestCommon.assertExceptionCorrect(got, expected);
Expand Down
63 changes: 3 additions & 60 deletions src/us/kbase/workspace/test/workspace/WorkspaceListenerTest.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -226,7 +226,7 @@ public void setWorkspaceMetadata1Listener() throws Exception {
when(m.db.getPermissions(user, set(rwsi))).thenReturn(
PermissionSet.getBuilder(user, new AllUsers('*'))
.withWorkspace(rwsi, Permission.ADMIN, Permission.NONE).build());
when(m.db.setWorkspaceMeta(rwsi, meta)).thenReturn(Instant.ofEpochMilli(20000));
when(m.db.setWorkspaceMeta(rwsi, meta, null)).thenReturn(Optional.of(inst(20000)));


ws.setWorkspaceMetadata(user, wsi, meta, null);
Expand All @@ -250,7 +250,7 @@ public void setWorkspaceMetadata2Listeners() throws Exception {
when(m.db.getPermissions(user, set(rwsi))).thenReturn(
PermissionSet.getBuilder(user, new AllUsers('*'))
.withWorkspace(rwsi, Permission.ADMIN, Permission.NONE).build());
when(m.db.setWorkspaceMeta(rwsi, meta)).thenReturn(Instant.ofEpochMilli(20000));
when(m.db.setWorkspaceMeta(rwsi, meta, null)).thenReturn(Optional.of(inst(20000)));


ws.setWorkspaceMetadata(user, wsi, meta, null);
Expand All @@ -277,7 +277,7 @@ public void setWorkspaceMetadataExceptionOnSetMeta() throws Exception {
.withWorkspace(rwsi, Permission.ADMIN, Permission.NONE).build());

doThrow(new WorkspaceCommunicationException("whee"))
.when(m.db).setWorkspaceMeta(rwsi, meta);
.when(m.db).setWorkspaceMeta(rwsi, meta, Arrays.asList("foo"));
try {
ws.setWorkspaceMetadata(user, wsi, meta, Arrays.asList("foo"));
} catch(WorkspaceCommunicationException e) {
Expand All @@ -288,63 +288,6 @@ public void setWorkspaceMetadataExceptionOnSetMeta() throws Exception {
any(WorkspaceUser.class), anyLong(), any(Instant.class));
}

@Test
public void setWorkspaceMetadataExceptionOnFirstRemove() throws Exception {
final Mocks m = new Mocks();
final WorkspaceUserMetadata meta = new WorkspaceUserMetadata(
ImmutableMap.of("foo", "bar"));

final WorkspaceUser user = new WorkspaceUser("foo");
final WorkspaceIdentifier wsi = new WorkspaceIdentifier(24);
final ResolvedWorkspaceID rwsi = new ResolvedWorkspaceID(24, "ugh", false, false);

final Workspace ws = new Workspace(m.db, m.cfg, m.tv, m.tfm, Arrays.asList(m.l1));

when(m.db.resolveWorkspaces(set(wsi))).thenReturn(ImmutableMap.of(wsi, rwsi));
when(m.db.getPermissions(user, set(rwsi))).thenReturn(
PermissionSet.getBuilder(user, new AllUsers('*'))
.withWorkspace(rwsi, Permission.ADMIN, Permission.NONE).build());
when(m.db.setWorkspaceMeta(rwsi, meta)).thenReturn(Instant.ofEpochMilli(20000));

doThrow(new WorkspaceCommunicationException("whee"))
.when(m.db).removeWorkspaceMetaKey(rwsi, "foo");
try {
ws.setWorkspaceMetadata(user, wsi, meta, Arrays.asList("foo"));
} catch(WorkspaceCommunicationException e) {
//fine
}

verify(m.l1).setWorkspaceMetadata(user, 24L, Instant.ofEpochMilli(20000));
}

@Test
public void setWorkspaceMetadataExceptionOnSecondRemove() throws Exception {
final Mocks m = new Mocks();

final WorkspaceUser user = new WorkspaceUser("foo");
final WorkspaceIdentifier wsi = new WorkspaceIdentifier(24);
final ResolvedWorkspaceID rwsi = new ResolvedWorkspaceID(24, "ugh", false, false);

final Workspace ws = new Workspace(m.db, m.cfg, m.tv, m.tfm, Arrays.asList(m.l1));

when(m.db.resolveWorkspaces(set(wsi))).thenReturn(ImmutableMap.of(wsi, rwsi));
when(m.db.getPermissions(user, set(rwsi))).thenReturn(
PermissionSet.getBuilder(user, new AllUsers('*'))
.withWorkspace(rwsi, Permission.ADMIN, Permission.NONE).build());
when(m.db.setWorkspaceMeta(rwsi, META)).thenReturn(Instant.ofEpochMilli(20000));
when(m.db.removeWorkspaceMetaKey(rwsi, "bar")).thenReturn(Instant.ofEpochMilli(30000));

doThrow(new WorkspaceCommunicationException("whee"))
.when(m.db).removeWorkspaceMetaKey(rwsi, "foo");
try {
ws.setWorkspaceMetadata(user, wsi, META, Arrays.asList("bar", "foo"));
} catch(WorkspaceCommunicationException e) {
//fine
}

verify(m.l1).setWorkspaceMetadata(user, 24L, Instant.ofEpochMilli(30000));
}

@Test
public void lockWorkspace1Listener() throws Exception {
final Mocks m = new Mocks();
Expand Down

0 comments on commit 37acadd

Please sign in to comment.