Merge pull request #3692 from john-hill/PLFM-5702

ignore newVersion for tables/views
Sage-Bionetworks · Jul 30, 2019 · b5d3d01 · b5d3d01
2 parents 3825a09 + b72cd01
commit b5d3d01
Show file tree

Hide file tree

Showing 2 changed files with 53 additions and 0 deletions.
diff --git a/...repository-managers/src/main/java/org/sagebionetworks/repo/manager/EntityManagerImpl.java b/...repository-managers/src/main/java/org/sagebionetworks/repo/manager/EntityManagerImpl.java
@@ -40,6 +40,8 @@
 import org.sagebionetworks.repo.model.file.ChildStatsRequest;
 import org.sagebionetworks.repo.model.file.ChildStatsResponse;
 import org.sagebionetworks.repo.model.provenance.Activity;
+import org.sagebionetworks.repo.model.table.EntityView;
+import org.sagebionetworks.repo.model.table.TableEntity;
 import org.sagebionetworks.repo.transactions.WriteTransaction;
 import org.sagebionetworks.repo.web.NotFoundException;
 import org.sagebionetworks.util.ValidateArgument;
@@ -372,6 +374,18 @@ public <T extends Entity> boolean updateEntity(UserInfo userInfo, T updated,
 			}
 		}
 
+		if(updated instanceof TableEntity || updated instanceof EntityView) {
+			/*
+			 * Fix for PLFM-5702. Creating a new version is fundamentally different than
+			 * creating a table/view snapshot. We cannot block callers from creating new
+			 * versions of tables/views because the Python/R client syn.store() method
+			 * automatically sets 'newVersion'=true. Therefore, to prevent users from
+			 * breaking their tables/views by explicitly creating new entity versions, we
+			 * unconditionally ignore this parameter for table/views.
+			 */
+			newVersion = false;
+		}
+
 		final boolean newVersionFinal = newVersion;
 
 		// Set activityId if new version or if not changing versions and activityId is defined

diff --git a/...anagers/src/test/java/org/sagebionetworks/repo/manager/EntityManagerImplAutowireTest.java b/...anagers/src/test/java/org/sagebionetworks/repo/manager/EntityManagerImplAutowireTest.java
@@ -42,6 +42,7 @@
 import org.sagebionetworks.repo.model.jdo.KeyFactory;
 import org.sagebionetworks.repo.model.provenance.Activity;
 import org.sagebionetworks.repo.model.table.EntityView;
+import org.sagebionetworks.repo.model.table.TableEntity;
 import org.sagebionetworks.repo.web.NotFoundException;
 import org.springframework.beans.factory.annotation.Autowired;
 import org.springframework.test.context.ContextConfiguration;
@@ -379,4 +380,42 @@ public void testCreateWithID() {
 		// the name should match the newly issued ID.
 		assertEquals(pid, project.getName());
 	}
+
+	/**
+	 * Test for PLFM-5702
+	 */
+	@Test
+	public void testUpdateEntityNewVersionTable() {
+		// update a table with newVersion=true;
+		TableEntity table = new TableEntity();
+		table.setName("Table");
+		String id = entityManager.createEntity(userInfo, table, null);
+		table = entityManager.getEntity(adminUserInfo, id, TableEntity.class);
+		toDelete.add(id);
+		boolean newVersion = true;
+		String activityId = null;
+		// call under test
+		boolean wasNewVersionCreated = entityManager.updateEntity(adminUserInfo, table, newVersion, activityId);
+		// should not create a new version.
+		assertFalse(wasNewVersionCreated);
+	}
+
+	/**
+	 * Test for PLFM-5702
+	 */
+	@Test
+	public void testUpdateEntityNewVersionEntityView() {
+		// update a table with newVersion=true;
+		EntityView view = new EntityView();
+		view.setName("Table");
+		String id = entityManager.createEntity(userInfo, view, null);
+		view = entityManager.getEntity(adminUserInfo, id, EntityView.class);
+		toDelete.add(id);
+		boolean newVersion = true;
+		String activityId = null;
+		// call under test
+		boolean wasNewVersionCreated = entityManager.updateEntity(adminUserInfo, view, newVersion, activityId);
+		// should not create a new version.
+		assertFalse(wasNewVersionCreated);
+	}
 }