From 20415a8e9aa4be7c83d7aceb41dcf87f00256388 Mon Sep 17 00:00:00 2001 From: Gavin Date: Mon, 16 Oct 2023 14:45:37 -0700 Subject: [PATCH] Add a MetadataUpdate class in preparation for batch object updates So we can pass a Map to the method --- src/us/kbase/workspace/WorkspaceServer.java | 3 +- .../workspace/database/MetadataUpdate.java | 77 +++++++++++++++++ src/us/kbase/workspace/database/Util.java | 4 +- .../kbase/workspace/database/Workspace.java | 14 +--- .../workspace/database/WorkspaceDatabase.java | 9 +- .../database/mongo/MongoWorkspaceDB.java | 31 +++---- .../workspace/test/database/UtilTest.java | 6 +- .../database/mongo/MongoWorkspaceDBTest.java | 49 +++++------ .../test/workspace/MetadataUpdateTest.java | 82 +++++++++++++++++++ .../test/workspace/WorkspaceListenerTest.java | 17 ++-- .../test/workspace/WorkspaceTest.java | 46 +++++++---- .../test/workspace/WorkspaceTester.java | 6 +- .../test/workspace/WorkspaceUnitTest.java | 11 +++ 13 files changed, 266 insertions(+), 89 deletions(-) create mode 100644 src/us/kbase/workspace/database/MetadataUpdate.java create mode 100644 src/us/kbase/workspace/test/workspace/MetadataUpdateTest.java diff --git a/src/us/kbase/workspace/WorkspaceServer.java b/src/us/kbase/workspace/WorkspaceServer.java index 75cf6493f..f12f9da33 100644 --- a/src/us/kbase/workspace/WorkspaceServer.java +++ b/src/us/kbase/workspace/WorkspaceServer.java @@ -57,6 +57,7 @@ import us.kbase.typedobj.core.TypeDefId; import us.kbase.workspace.database.DependencyStatus; import us.kbase.workspace.database.ListObjectsParameters; +import us.kbase.workspace.database.MetadataUpdate; import us.kbase.workspace.database.ObjectIDNoWSNoVer; import us.kbase.workspace.database.ResourceUsageConfigurationBuilder.ResourceUsageConfiguration; import us.kbase.workspace.database.Workspace; @@ -427,7 +428,7 @@ public void alterWorkspaceMetadata(AlterWorkspaceMetadataParams params, AuthToke } final WorkspaceIdentifier wsi = processWorkspaceIdentifier(params.getWsi()); final WorkspaceUser user = wsmeth.getUser(authPart); - ws.setWorkspaceMetadata(user, wsi, meta, remove); + ws.setWorkspaceMetadata(user, wsi, new MetadataUpdate(meta, remove)); //END alter_workspace_metadata } diff --git a/src/us/kbase/workspace/database/MetadataUpdate.java b/src/us/kbase/workspace/database/MetadataUpdate.java new file mode 100644 index 000000000..9b338f739 --- /dev/null +++ b/src/us/kbase/workspace/database/MetadataUpdate.java @@ -0,0 +1,77 @@ +package us.kbase.workspace.database; + +import static us.kbase.workspace.database.Util.noNulls; + +import java.util.Collection; +import java.util.Collections; +import java.util.HashSet; +import java.util.Objects; +import java.util.Optional; +import java.util.Set; + +/** An update to a a set of metadata, including keys to add or replace and keys to remove. */ +public class MetadataUpdate { + + private final WorkspaceUserMetadata meta; + private final Set remove; + + /** Create the update. + * @param meta keys to add to or replace in the target metadata. + * @param toRemove keys to remove from the target metadata. + */ + public MetadataUpdate(final WorkspaceUserMetadata meta, final Collection toRemove) { + this.meta = meta == null || meta.isEmpty() ? null : meta; + if (toRemove != null && !toRemove.isEmpty()) { + this.remove = Collections.unmodifiableSet(new HashSet<>( + noNulls( + toRemove, + "null metadata keys are not allowed in the remove parameter" + ) + )); + } else { + this.remove = null; + } + } + + /** Get the keys to add or replace in the target metadata. + * @return the keys. + */ + public Optional getMeta() { + return Optional.ofNullable(meta); + } + + /** Get the keys to remove from the target metadata. + * @return + */ + public Optional> getToRemove() { + return Optional.ofNullable(remove); + } + + /** Return whether this metadata update has an update. + * @return true if there are keys to add, replace, or remove in this update. + */ + public boolean hasUpdate() { + return meta != null || remove != null; + } + + @Override + public int hashCode() { + return Objects.hash(meta, remove); + } + + @Override + public boolean equals(Object obj) { + if (this == obj) { + return true; + } + if (obj == null) { + return false; + } + if (getClass() != obj.getClass()) { + return false; + } + MetadataUpdate other = (MetadataUpdate) obj; + return Objects.equals(meta, other.meta) && Objects.equals(remove, other.remove); + } + +} diff --git a/src/us/kbase/workspace/database/Util.java b/src/us/kbase/workspace/database/Util.java index 7fe5d542d..4f1962342 100644 --- a/src/us/kbase/workspace/database/Util.java +++ b/src/us/kbase/workspace/database/Util.java @@ -42,13 +42,15 @@ public static void nonNull(final Object o, final String message) { * @param col the collection to check. * @param message the exception message. * @param the type of the elements in the collection. + * @return the collection. */ - public static void noNulls(final Collection col, final String message) { + public static Collection noNulls(final Collection col, final String message) { for (final T item: col) { if (item == null) { throw new NullPointerException(message); } } + return col; } /** Check that the provided collection is not null and contains no null or whitespace-only diff --git a/src/us/kbase/workspace/database/Workspace.java b/src/us/kbase/workspace/database/Workspace.java index 6cea4d87e..5b0332e9a 100644 --- a/src/us/kbase/workspace/database/Workspace.java +++ b/src/us/kbase/workspace/database/Workspace.java @@ -204,9 +204,6 @@ public WorkspaceInformation createWorkspace(final WorkspaceUser user, * @param user the user altering the metadata. * @param wsi the workspace to alter. * @param meta updated metadata. Keys will overwrite any keys already set on the workspace. - * Send null to make no changes. - * @param keysToRemove metadata keys to remove from the workspace. Send null to make no - * changes. * @throws CorruptWorkspaceDBException if corrupt data is found in the database. * @throws NoSuchWorkspaceException if the workspace does not exist or is deleted. * @throws WorkspaceCommunicationException if a communication error occurs when contacting the @@ -216,20 +213,17 @@ public WorkspaceInformation createWorkspace(final WorkspaceUser user, public void setWorkspaceMetadata( final WorkspaceUser user, final WorkspaceIdentifier wsi, - final WorkspaceUserMetadata meta, - final List keysToRemove) + final MetadataUpdate meta) throws CorruptWorkspaceDBException, NoSuchWorkspaceException, WorkspaceCommunicationException, WorkspaceAuthorizationException { + requireNonNull(meta, "meta"); final ResolvedWorkspaceID wsid = new PermissionsCheckerFactory(db, user) .getWorkspaceChecker(wsi, Permission.ADMIN) .withOperation("alter metadata for").check(); - if (keysToRemove == null && (meta == null || meta.isEmpty())) { + if (!meta.hasUpdate()) { return; } - if (keysToRemove != null) { - noNulls(keysToRemove, "null metadata keys are not allowed"); - } - final Optional time = db.setWorkspaceMeta(wsid, meta, keysToRemove); + final Optional time = db.setWorkspaceMeta(wsid, meta); if (time.isPresent()) { for (final WorkspaceEventListener l: listeners) { l.setWorkspaceMetadata(user, wsid.getID(), time.get()); diff --git a/src/us/kbase/workspace/database/WorkspaceDatabase.java b/src/us/kbase/workspace/database/WorkspaceDatabase.java index 0826dd461..425346297 100644 --- a/src/us/kbase/workspace/database/WorkspaceDatabase.java +++ b/src/us/kbase/workspace/database/WorkspaceDatabase.java @@ -90,9 +90,7 @@ WorkspaceInformation createWorkspace(WorkspaceUser owner, * duplicate key is supplied. * * @param wsid the workspace for which metadata will be altered. - * @param meta the metadata to add to the workspace. - * @param remove the metadata keys to remove from the workspace. If any provided keys do - * not exist they will be ignored. + * @param meta the metadata update to apply to the workspace. * @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. @@ -100,10 +98,7 @@ WorkspaceInformation createWorkspace(WorkspaceUser owner, * @throws IllegalArgumentException if no metadata is supplied or the * updated metadata exceeds the allowed size. */ - Optional setWorkspaceMeta( - ResolvedWorkspaceID wsid, - WorkspaceUserMetadata meta, - List remove) + Optional setWorkspaceMeta(ResolvedWorkspaceID wsid, MetadataUpdate meta) throws WorkspaceCommunicationException, CorruptWorkspaceDBException; /** Clone a workspace. diff --git a/src/us/kbase/workspace/database/mongo/MongoWorkspaceDB.java b/src/us/kbase/workspace/database/mongo/MongoWorkspaceDB.java index 777b5ff27..662910603 100644 --- a/src/us/kbase/workspace/database/mongo/MongoWorkspaceDB.java +++ b/src/us/kbase/workspace/database/mongo/MongoWorkspaceDB.java @@ -65,6 +65,7 @@ import us.kbase.workspace.database.ByteArrayFileCacheManager; import us.kbase.workspace.database.CopyResult; import us.kbase.workspace.database.ListObjectsParameters.ResolvedListObjectParameters; +import us.kbase.workspace.database.MetadataUpdate; import us.kbase.workspace.database.ObjectIDNoWSNoVer; import us.kbase.workspace.database.ObjectIDResolvedWS; import us.kbase.workspace.database.ObjectInfoWithModDate; @@ -696,8 +697,6 @@ private void setCreatedWorkspacePermissions( } } - private static final Set FLDS_WS_META = newHashSet(Fields.WS_META); - @FunctionalInterface interface DocumentProvider { Map getDocument() @@ -707,34 +706,29 @@ Map getDocument() @Override public Optional setWorkspaceMeta( final ResolvedWorkspaceID rwsi, - final WorkspaceUserMetadata newMeta, - final List remove) + final MetadataUpdate meta) throws WorkspaceCommunicationException, CorruptWorkspaceDBException { + requireNonNull(rwsi, "rwsi"); return setMetadataOnDocument( - newMeta, - remove, + meta, COL_WORKSPACES, - () -> query.queryWorkspace(rwsi, FLDS_WS_META), + () -> query.queryWorkspace(rwsi, newHashSet(Fields.WS_META)), new Document(Fields.WS_ID, rwsi.getID()), Fields.WS_META, Fields.WS_MODDATE); } private Optional setMetadataOnDocument( - final WorkspaceUserMetadata newMeta, - final List remove, + final MetadataUpdate newMeta, final String collection, final DocumentProvider dp, final Document identifier, final String metaField, final String moddateField) throws WorkspaceCommunicationException, CorruptWorkspaceDBException { - if (remove == null && (newMeta == null || newMeta.isEmpty())) { + if (newMeta == null || !newMeta.hasUpdate()) { 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; while (time == null) { @@ -743,11 +737,11 @@ private Optional setMetadataOnDocument( final List> mlist = (List>) doc.get(metaField); final Map oldMeta = metaMongoArrayToHash(mlist); final Map updatedMeta = new HashMap<>(oldMeta); - if (newMeta != null) { - updatedMeta.putAll(newMeta.getMetadata()); + if (newMeta.getMeta().isPresent()) { + updatedMeta.putAll(newMeta.getMeta().get().getMetadata()); } - if (remove != null) { - for (final String k: remove) { + if (newMeta.getToRemove().isPresent()) { + for (final String k: newMeta.getToRemove().get()) { updatedMeta.remove(k); } } @@ -3014,8 +3008,7 @@ private Map resolveObjectIDs( final Map> ids = queryObjects(objectIDs, FLDS_RESOLVE_OBJS, exceptIfDeleted, includeDeleted, exceptIfMissing); - final Map ret = - new HashMap(); + final Map ret = new HashMap<>(); for (final ObjectIDResolvedWS o: ids.keySet()) { final String name = (String) ids.get(o).get(Fields.OBJ_NAME); final long id = (Long) ids.get(o).get(Fields.OBJ_ID); diff --git a/src/us/kbase/workspace/test/database/UtilTest.java b/src/us/kbase/workspace/test/database/UtilTest.java index 1845cb0ff..f1666bb7f 100644 --- a/src/us/kbase/workspace/test/database/UtilTest.java +++ b/src/us/kbase/workspace/test/database/UtilTest.java @@ -145,8 +145,10 @@ public void nonNullFail() throws Exception { @Test public void noNullsPass() throws Exception { - Util.noNulls(NON_WHITESPACE_STRINGS, TYPE_NAME); - Util.noNulls(WHITESPACE_STRINGS, TYPE_NAME); + assertThat("incorrect return", Util.noNulls(NON_WHITESPACE_STRINGS, TYPE_NAME), + is(NON_WHITESPACE_STRINGS)); + assertThat("incorrect return", Util.noNulls(WHITESPACE_STRINGS, TYPE_NAME), + is(WHITESPACE_STRINGS)); } @Test diff --git a/src/us/kbase/workspace/test/database/mongo/MongoWorkspaceDBTest.java b/src/us/kbase/workspace/test/database/mongo/MongoWorkspaceDBTest.java index 5202640e4..60502af9e 100644 --- a/src/us/kbase/workspace/test/database/mongo/MongoWorkspaceDBTest.java +++ b/src/us/kbase/workspace/test/database/mongo/MongoWorkspaceDBTest.java @@ -73,6 +73,7 @@ import us.kbase.workspace.database.DynamicConfig; import us.kbase.workspace.database.ObjectIDNoWSNoVer; import us.kbase.workspace.database.DynamicConfig.DynamicConfigUpdate; +import us.kbase.workspace.database.MetadataUpdate; import us.kbase.workspace.database.mongo.Fields; import us.kbase.workspace.database.mongo.GridFSBlobStore; import us.kbase.workspace.database.mongo.MongoWorkspaceDB; @@ -862,9 +863,11 @@ public void setWorkspaceMetadata() throws Exception { // test against empty metadata final Optional result = mocks.mdb.setWorkspaceMeta( rwsi, - new WorkspaceUserMetadata(ImmutableMap.of( - "foo", "bar", "baz", "bat", "to_remove", "a")), - Arrays.asList("no_key_present") + new MetadataUpdate( + new WorkspaceUserMetadata(ImmutableMap.of( + "foo", "bar", "baz", "bat", "to_remove", "a")), + Arrays.asList("no_key_present") + ) ); assertThat("incorrect time", result, is(Optional.of(inst(10000)))); @@ -877,8 +880,10 @@ public void setWorkspaceMetadata() throws Exception { // test against non-empty metadata final Optional result2 = mocks.mdb.setWorkspaceMeta( rwsi, - new WorkspaceUserMetadata(ImmutableMap.of("baz", "bing", "thingy", "thinger")), - Arrays.asList("to_remove", "somecrap") + new MetadataUpdate( + new WorkspaceUserMetadata(ImmutableMap.of("baz", "bing", "thingy", "thinger")), + Arrays.asList("to_remove", "somecrap") + ) ); assertThat("incorrect time", result2, is(Optional.of(inst(15000)))); @@ -910,10 +915,7 @@ public void setWorkspaceMetadataRemoveOnly() throws Exception { when(mocks.clockmock.instant()).thenReturn(inst(10000), (Instant) null); final Optional result = mocks.mdb.setWorkspaceMeta( - rwsi, - null, - Arrays.asList("x", "a") - ); + rwsi, new MetadataUpdate(null, Arrays.asList("x", "a"))); assertThat("incorrect time", result, is(Optional.of(inst(10000)))); @@ -937,8 +939,10 @@ public void setWorkspaceMetadataNoop() throws Exception { final Optional result = mocks.mdb.setWorkspaceMeta( rwsi, - new WorkspaceUserMetadata(ImmutableMap.of("a", "b", "x", "y")), - Arrays.asList("x", "z") + new MetadataUpdate( + new WorkspaceUserMetadata(ImmutableMap.of("a", "b", "x", "y")), + Arrays.asList("x", "z") + ) ); assertThat("incorrect time", result, is(Optional.empty())); @@ -952,13 +956,12 @@ public void setWorkspaceMetadataNoop() throws Exception { 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, null, new IllegalArgumentException( - "No metadata changes provided")); - final WorkspaceUserMetadata meta = new WorkspaceUserMetadata(); - 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")); + final MetadataUpdate mu = new MetadataUpdate(null, list("foo")); + failSetWorkspaceMeta(mocks.mdb, null, mu, new NullPointerException("rwsi")); + failSetWorkspaceMeta(mocks.mdb, rwsi, null, + new IllegalArgumentException("No metadata changes provided")); + failSetWorkspaceMeta(mocks.mdb, rwsi, new MetadataUpdate(null, null), + new IllegalArgumentException("No metadata changes provided")); } private List setWorkspaceMetaBigMetaSetup() throws Exception { @@ -995,7 +998,7 @@ public void setWorkspaceMetaPassBigMeta() throws Exception { // test against empty metadata final Optional result = mocks.mdb.setWorkspaceMeta( - rwsi, new WorkspaceUserMetadata(meta2), null); + rwsi, new MetadataUpdate(new WorkspaceUserMetadata(meta2), null)); assertThat("incorrect time", result, is(Optional.of(inst(10000)))); @@ -1022,19 +1025,19 @@ public void setWorkspaceMetaFailBigMeta() throws Exception { final Map meta2 = (Map) setup.get(3); meta2.put("b10", TestCommon.LONG1001.substring(0, 725)); - failSetWorkspaceMeta(mocks.mdb, rwsi, new WorkspaceUserMetadata(meta2), null, + failSetWorkspaceMeta( + mocks.mdb, rwsi, new MetadataUpdate(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 remove, + final MetadataUpdate meta, final Exception expected) throws Exception { try { - mdb.setWorkspaceMeta(rwsi, meta, remove); + mdb.setWorkspaceMeta(rwsi, meta); fail("expected exception"); } catch (Exception got) { TestCommon.assertExceptionCorrect(got, expected); diff --git a/src/us/kbase/workspace/test/workspace/MetadataUpdateTest.java b/src/us/kbase/workspace/test/workspace/MetadataUpdateTest.java new file mode 100644 index 000000000..7d2301d96 --- /dev/null +++ b/src/us/kbase/workspace/test/workspace/MetadataUpdateTest.java @@ -0,0 +1,82 @@ +package us.kbase.workspace.test.workspace; + +import static org.hamcrest.CoreMatchers.is; +import static org.junit.Assert.assertThat; +import static org.junit.Assert.fail; +import static us.kbase.common.test.TestCommon.assertExceptionCorrect; +import static us.kbase.common.test.TestCommon.set; + +import java.util.Arrays; +import java.util.Optional; + +import org.junit.Test; + +import com.google.common.collect.ImmutableMap; + +import nl.jqno.equalsverifier.EqualsVerifier; +import us.kbase.workspace.database.MetadataUpdate; +import us.kbase.workspace.database.WorkspaceUserMetadata; + +public class MetadataUpdateTest { + + @Test + public void equals() throws Exception { + EqualsVerifier.forClass(MetadataUpdate.class).usingGetClass().verify(); + } + + @Test + public void constructNulls() throws Exception { + final MetadataUpdate mu = new MetadataUpdate(null, null); + assertThat("incorrect meta", mu.getMeta(), is(Optional.empty())); + assertThat("incorrect remove", mu.getToRemove(), is(Optional.empty())); + assertThat("incorrect hasUpdate", mu.hasUpdate(), is(false)); + } + + @Test + public void constructEmpties() throws Exception { + final MetadataUpdate mu = new MetadataUpdate(new WorkspaceUserMetadata(), set()); + assertThat("incorrect meta", mu.getMeta(), is(Optional.empty())); + assertThat("incorrect remove", mu.getToRemove(), is(Optional.empty())); + assertThat("incorrect hasUpdate", mu.hasUpdate(), is(false)); + } + + @Test + public void constructWithMeta() throws Exception { + final MetadataUpdate mu = new MetadataUpdate( + new WorkspaceUserMetadata(ImmutableMap.of("a", "b")), null); + assertThat("incorrect meta", mu.getMeta(), is(Optional.of( + new WorkspaceUserMetadata(ImmutableMap.of("a", "b"))))); + assertThat("incorrect remove", mu.getToRemove(), is(Optional.empty())); + assertThat("incorrect hasUpdate", mu.hasUpdate(), is(true)); + } + + @Test + public void constructWithRemove() throws Exception { + final MetadataUpdate mu = new MetadataUpdate(null, Arrays.asList("foo", "bar")); + assertThat("incorrect meta", mu.getMeta(), is(Optional.empty())); + assertThat("incorrect remove", mu.getToRemove(), is(Optional.of(set("bar", "foo")))); + assertThat("incorrect hasUpdate", mu.hasUpdate(), is(true)); + } + + @Test + public void constructWithBoth() throws Exception { + final MetadataUpdate mu = new MetadataUpdate( + new WorkspaceUserMetadata(ImmutableMap.of("a", "b")), set("baz", "bat")); + assertThat("incorrect meta", mu.getMeta(), is(Optional.of( + new WorkspaceUserMetadata(ImmutableMap.of("a", "b"))))); + assertThat("incorrect remove", mu.getToRemove(), is(Optional.of(set("baz", "bat")))); + assertThat("incorrect hasUpdate", mu.hasUpdate(), is(true)); + } + + @Test + public void constructFailNullInRemove() throws Exception { + try { + new MetadataUpdate(null, Arrays.asList("foo", null)); + fail("expected exception"); + } catch (Exception got) { + assertExceptionCorrect(got, new NullPointerException( + "null metadata keys are not allowed in the remove parameter")); + } + + } +} diff --git a/src/us/kbase/workspace/test/workspace/WorkspaceListenerTest.java b/src/us/kbase/workspace/test/workspace/WorkspaceListenerTest.java index b66237880..ab4ba1042 100644 --- a/src/us/kbase/workspace/test/workspace/WorkspaceListenerTest.java +++ b/src/us/kbase/workspace/test/workspace/WorkspaceListenerTest.java @@ -38,6 +38,7 @@ import us.kbase.typedobj.idref.IdReferenceHandlerSetFactoryBuilder; import us.kbase.workspace.database.AllUsers; import us.kbase.workspace.database.CopyResult; +import us.kbase.workspace.database.MetadataUpdate; import us.kbase.workspace.database.ObjectIDNoWSNoVer; import us.kbase.workspace.database.ObjectIDResolvedWS; import us.kbase.workspace.database.ObjectIdentifier; @@ -204,7 +205,7 @@ public void setWorkspaceMetadataNoUpdate() throws Exception { PermissionSet.getBuilder(user, new AllUsers('*')) .withWorkspace(rwsi, Permission.ADMIN, Permission.NONE).build()); - ws.setWorkspaceMetadata(user, wsi, META, null); + ws.setWorkspaceMetadata(user, wsi, new MetadataUpdate(META, null)); verify(m.l1, never()).setWorkspaceMetadata( any(WorkspaceUser.class), anyLong(), any(Instant.class)); @@ -226,10 +227,11 @@ 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, null)).thenReturn(Optional.of(inst(20000))); + when(m.db.setWorkspaceMeta(rwsi, new MetadataUpdate(meta, null))) + .thenReturn(Optional.of(inst(20000))); - ws.setWorkspaceMetadata(user, wsi, meta, null); + ws.setWorkspaceMetadata(user, wsi, new MetadataUpdate(meta, null)); verify(m.l1).setWorkspaceMetadata(user, 24L, Instant.ofEpochMilli(20000)); } @@ -250,10 +252,11 @@ 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, null)).thenReturn(Optional.of(inst(20000))); + when(m.db.setWorkspaceMeta(rwsi, new MetadataUpdate(meta, null))) + .thenReturn(Optional.of(inst(20000))); - ws.setWorkspaceMetadata(user, wsi, meta, null); + ws.setWorkspaceMetadata(user, wsi, new MetadataUpdate(meta, null)); verify(m.l1).setWorkspaceMetadata(user, 24L, Instant.ofEpochMilli(20000)); verify(m.l2).setWorkspaceMetadata(user, 24L, Instant.ofEpochMilli(20000)); @@ -277,9 +280,9 @@ public void setWorkspaceMetadataExceptionOnSetMeta() throws Exception { .withWorkspace(rwsi, Permission.ADMIN, Permission.NONE).build()); doThrow(new WorkspaceCommunicationException("whee")) - .when(m.db).setWorkspaceMeta(rwsi, meta, Arrays.asList("foo")); + .when(m.db).setWorkspaceMeta(rwsi, new MetadataUpdate(meta, Arrays.asList("foo"))); try { - ws.setWorkspaceMetadata(user, wsi, meta, Arrays.asList("foo")); + ws.setWorkspaceMetadata(user, wsi, new MetadataUpdate(meta, Arrays.asList("foo"))); } catch(WorkspaceCommunicationException e) { //fine } diff --git a/src/us/kbase/workspace/test/workspace/WorkspaceTest.java b/src/us/kbase/workspace/test/workspace/WorkspaceTest.java index 8deaefbb3..4247bd6a4 100644 --- a/src/us/kbase/workspace/test/workspace/WorkspaceTest.java +++ b/src/us/kbase/workspace/test/workspace/WorkspaceTest.java @@ -76,6 +76,7 @@ import us.kbase.workspace.database.AllUsers; import us.kbase.workspace.database.DependencyStatus; import us.kbase.workspace.database.ListObjectsParameters; +import us.kbase.workspace.database.MetadataUpdate; import us.kbase.workspace.database.ModuleInfo; import us.kbase.workspace.database.ObjectIDNoWSNoVer; import us.kbase.workspace.database.ObjectIDResolvedWS; @@ -698,13 +699,15 @@ public void workspaceMetadata() throws Exception { meta.put("foo2", "bar3"); //replace Map putmeta = new HashMap(); putmeta.put("foo2", "bar3"); - ws.setWorkspaceMetadata(user, wsi, new WorkspaceUserMetadata(putmeta), null); + ws.setWorkspaceMetadata( + user, wsi, new MetadataUpdate(new WorkspaceUserMetadata(putmeta), null)); final Instant d1 = checkWSInfo(wsi, user, wsi.getName(), 0, Permission.OWNER, false, info.getId(), "unlocked", meta); meta.put("foo3", "bar4"); //new putmeta.clear(); putmeta.put("foo3", "bar4"); - ws.setWorkspaceMetadata(user, wsi, new WorkspaceUserMetadata(putmeta), null); + ws.setWorkspaceMetadata( + user, wsi, new MetadataUpdate(new WorkspaceUserMetadata(putmeta), null)); final Instant d2 = checkWSInfo(wsi, user, wsi.getName(), 0, Permission.OWNER, false, info.getId(), "unlocked", meta); @@ -717,35 +720,40 @@ public void workspaceMetadata() throws Exception { meta.put("some.garbage", "with.dots"); meta.put("foo", "whoa this is new"); meta.put("no, this part is new", "prunker"); - ws.setWorkspaceMetadata(user, wsi, new WorkspaceUserMetadata(putmeta), null); + ws.setWorkspaceMetadata( + user, wsi, new MetadataUpdate(new WorkspaceUserMetadata(putmeta), null)); final Instant d3 = checkWSInfo(wsi, user, wsi.getName(), 0, Permission.OWNER, false, info.getId(), "unlocked", meta); Map newmeta = new HashMap(); newmeta.put("new", "meta"); - ws.setWorkspaceMetadata(user, wsiNo, new WorkspaceUserMetadata(newmeta), - Collections.emptyList()); + ws.setWorkspaceMetadata(user, wsiNo, new MetadataUpdate(new WorkspaceUserMetadata(newmeta), + Collections.emptyList())); final Instant nod1 = checkWSInfo(wsiNo, user, wsiNo.getName(), 0, Permission.OWNER, false, infoNo.getId(), "unlocked", newmeta); assertDatesAscending(infoNo.getModDate(), nod1); meta.remove("foo2"); - ws.setWorkspaceMetadata(user, wsi, null, Arrays.asList("foo2")); + ws.setWorkspaceMetadata(user, wsi, new MetadataUpdate(null, Arrays.asList("foo2"))); final Instant d4 = checkWSInfo(wsi, user, wsi.getName(), 0, Permission.OWNER, false, info.getId(), "unlocked", meta); meta.remove("some"); - ws.setWorkspaceMetadata(user2, wsi, null, Arrays.asList("some")); + ws.setWorkspaceMetadata(user2, wsi, new MetadataUpdate(null, Arrays.asList("some"))); final Instant d5 = checkWSInfo(wsi, user, wsi.getName(), 0, Permission.OWNER, false, info.getId(), "unlocked", meta); - ws.setWorkspaceMetadata(user, wsi, null, Arrays.asList("fake")); //no effect + //no effect + ws.setWorkspaceMetadata(user, wsi, new MetadataUpdate(null, Arrays.asList("fake"))); checkWSInfo(wsi, user, wsi.getName(), 0, Permission.OWNER, false, info.getId(), d5, "unlocked", meta); assertDatesAscending(info.getModDate(), d1, d2, d3, d4, d5); - checkWSInfo(wsiNo2, user, wsiNo2.getName(), 0, Permission.OWNER, false, infoNo2.getId(), infoNo2.getModDate(), "unlocked", MT_MAP); - ws.setWorkspaceMetadata(user, wsiNo2, null, Arrays.asList("somekey")); //should do nothing - checkWSInfo(wsiNo2, user, wsiNo2.getName(), 0, Permission.OWNER, false, infoNo2.getId(), infoNo2.getModDate(), "unlocked", MT_MAP); + checkWSInfo(wsiNo2, user, wsiNo2.getName(), 0, Permission.OWNER, false, + infoNo2.getId(), infoNo2.getModDate(), "unlocked", MT_MAP); + //should do nothing + ws.setWorkspaceMetadata(user, wsiNo2, new MetadataUpdate(null, Arrays.asList("somekey"))); + checkWSInfo(wsiNo2, user, wsiNo2.getName(), 0, Permission.OWNER, false, + infoNo2.getId(), infoNo2.getModDate(), "unlocked", MT_MAP); ws.setPermissions(user, wsi, Arrays.asList(user2), Permission.WRITE); @@ -769,7 +777,9 @@ public void workspaceMetadata() throws Exception { failWSSetMeta(user, wsi, putmeta, new IllegalArgumentException( "Updated metadata exceeds allowed size of 16000B")); - ws.setWorkspaceMetadata(user, wsiNo, new WorkspaceUserMetadata(putmeta), null); //should work + //should work + ws.setWorkspaceMetadata( + user, wsiNo, new MetadataUpdate(new WorkspaceUserMetadata(putmeta), null)); putmeta.put("148", TEXT100); failWSSetMeta(user, wsiNo2, putmeta, new MetadataSizeException( "Metadata exceeds maximum of 16000B")); @@ -786,14 +796,15 @@ public void workspaceMetadataRemoveMultiple() throws Exception { meta.put("some", "meta"); ws.createWorkspace(user, wsi.getName(), false, null, new WorkspaceUserMetadata(meta)); - ws.setWorkspaceMetadata(user, wsi, null, Arrays.asList("foo", "some")); + ws.setWorkspaceMetadata(user, wsi, new MetadataUpdate(null, Arrays.asList("foo", "some"))); final Map gotmeta = ws.getWorkspaceInformation(user, wsi) .getUserMeta().getMetadata(); assertThat("incorrect metadata", gotmeta, is(ImmutableMap.of("foo2", "bar2"))); - ws.setWorkspaceMetadata(user, wsi, new WorkspaceUserMetadata(meta), null); - ws.setWorkspaceMetadata(user, wsi, new WorkspaceUserMetadata(), - Arrays.asList("foo2")); + ws.setWorkspaceMetadata( + user, wsi, new MetadataUpdate(new WorkspaceUserMetadata(meta), null)); + ws.setWorkspaceMetadata(user, wsi, + new MetadataUpdate(new WorkspaceUserMetadata(), Arrays.asList("foo2"))); final Map gotmeta2 = ws.getWorkspaceInformation(user, wsi) .getUserMeta().getMetadata(); @@ -806,7 +817,8 @@ public void workspaceMetadataRemoveFail() throws Exception { final WorkspaceUser user = new WorkspaceUser("user"); ws.createWorkspace(user, "foo", false, null, null); failWSSetMeta(ws, user, new WorkspaceIdentifier(1), null, - Arrays.asList("foo", null), new NullPointerException("null metadata keys are not allowed")); + Arrays.asList("foo", null), new NullPointerException( + "null metadata keys are not allowed in the remove parameter")); } @Test diff --git a/src/us/kbase/workspace/test/workspace/WorkspaceTester.java b/src/us/kbase/workspace/test/workspace/WorkspaceTester.java index 6e2375926..07d732617 100644 --- a/src/us/kbase/workspace/test/workspace/WorkspaceTester.java +++ b/src/us/kbase/workspace/test/workspace/WorkspaceTester.java @@ -60,6 +60,7 @@ import us.kbase.workspace.database.AllUsers; import us.kbase.workspace.database.DynamicConfig.DynamicConfigUpdate; import us.kbase.workspace.database.ListObjectsParameters; +import us.kbase.workspace.database.MetadataUpdate; import us.kbase.workspace.database.ObjectIDNoWSNoVer; import us.kbase.workspace.database.ObjectIDResolvedWS; import us.kbase.workspace.database.ObjectIdentifier; @@ -528,7 +529,7 @@ public static void failWSRemoveMeta( final String key, final Exception e) { try { - ws.setWorkspaceMetadata(user, wsi, null, Arrays.asList(key)); + ws.setWorkspaceMetadata(user, wsi, new MetadataUpdate(null, Arrays.asList(key))); fail("expected remove ws meta to fail"); } catch (Exception exp) { assertExceptionCorrect(exp, e); @@ -551,7 +552,8 @@ public static void failWSSetMeta( final List remove, final Exception e) { try { - ws.setWorkspaceMetadata(user, wsi, new WorkspaceUserMetadata(meta), remove); + ws.setWorkspaceMetadata( + user, wsi, new MetadataUpdate(new WorkspaceUserMetadata(meta), remove)); fail("expected set ws meta to fail"); } catch (Exception exp) { assertExceptionCorrect(exp, e); diff --git a/src/us/kbase/workspace/test/workspace/WorkspaceUnitTest.java b/src/us/kbase/workspace/test/workspace/WorkspaceUnitTest.java index 4d1347bb4..bace91c4e 100644 --- a/src/us/kbase/workspace/test/workspace/WorkspaceUnitTest.java +++ b/src/us/kbase/workspace/test/workspace/WorkspaceUnitTest.java @@ -163,6 +163,17 @@ public void setConfigFail() throws Exception { } } + @Test + public void setWorkspaceMetadataFailNullMeta() throws Exception { + try { + initMocks().ws.setWorkspaceMetadata( + new WorkspaceUser("u"), new WorkspaceIdentifier(1), null); + fail("expected exception"); + } catch (Exception got) { + TestCommon.assertExceptionCorrect(got, new NullPointerException("meta")); + } + } + @Test public void setWorkspaceDescriptionNull() throws Exception { setWorkspaceDescription(null, null);