From 3f770a827662e1d8269de537763d0262bf5b29fc Mon Sep 17 00:00:00 2001 From: pkupczyk <pkupczyk> Date: Thu, 18 Oct 2012 07:18:55 +0000 Subject: [PATCH] SP-346 / BIS-178 : Metaprojects: Metapoject names should be more flexible than codes SVN: 27225 --- .../server/dataaccess/db/MetaprojectDAO.java | 8 +- .../generic/shared/basic/MetaprojectName.java | 52 ++++++++++++ openbis/source/sql/generic/124/schema-124.sql | 3 +- .../migration/migration-123-124.sql | 4 +- .../shared/basic/MetaprojectNameTest.java | 83 +++++++++++++++++++ ...GeneralInformationChangingServiceTest.java | 34 ++++++++ .../api/v1/GeneralInformationServiceTest.java | 10 +++ .../sql/postgresql/124/finish-124.sql | 4 +- .../IGeneralInformationChangingService.java | 14 ++-- .../metaproject/MetaprojectIdentifierId.java | 2 +- .../basic/dto/MetaprojectIdentifier.java | 3 + 11 files changed, 200 insertions(+), 17 deletions(-) create mode 100644 openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/MetaprojectName.java create mode 100644 openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/basic/MetaprojectNameTest.java diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/MetaprojectDAO.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/MetaprojectDAO.java index 084c9a7bd98..4b00f5e072b 100644 --- a/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/MetaprojectDAO.java +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/server/dataaccess/db/MetaprojectDAO.java @@ -29,7 +29,7 @@ import ch.systemsx.cisd.common.logging.LogCategory; import ch.systemsx.cisd.common.logging.LogFactory; import ch.systemsx.cisd.common.reflection.MethodUtils; import ch.systemsx.cisd.openbis.generic.server.dataaccess.IMetaprojectDAO; -import ch.systemsx.cisd.openbis.generic.shared.basic.CodeConverter; +import ch.systemsx.cisd.openbis.generic.shared.basic.MetaprojectName; import ch.systemsx.cisd.openbis.generic.shared.dto.DatabaseInstancePE; import ch.systemsx.cisd.openbis.generic.shared.dto.MetaprojectPE; import ch.systemsx.cisd.openbis.generic.shared.dto.PersonPE; @@ -55,7 +55,7 @@ public class MetaprojectDAO extends AbstractGenericEntityDAO<MetaprojectPE> impl { final DetachedCriteria criteria = DetachedCriteria.forClass(MetaprojectPE.class); criteria.createAlias("owner", "o"); - criteria.add(Restrictions.eq("name", metaprojectName)); + criteria.add(Restrictions.eq("name", metaprojectName).ignoreCase()); criteria.add(Restrictions.eq("o.userId", ownerId)); final List<MetaprojectPE> list = cast(getHibernateTemplate().findByCriteria(criteria)); final MetaprojectPE entity = tryFindEntity(list, "metaproject"); @@ -88,9 +88,10 @@ public class MetaprojectDAO extends AbstractGenericEntityDAO<MetaprojectPE> impl public void createOrUpdateMetaproject(MetaprojectPE metaproject, PersonPE owner) { assert metaproject != null : "Missing metaproject."; + validatePE(metaproject); + MetaprojectName.validate(metaproject.getName()); - metaproject.setName(CodeConverter.tryToDatabase(metaproject.getName())); metaproject.setOwner(owner); metaproject.setPrivate(true); final HibernateTemplate template = getHibernateTemplate(); @@ -101,4 +102,5 @@ public class MetaprojectDAO extends AbstractGenericEntityDAO<MetaprojectPE> impl operationLog.info(String.format("SAVE: metaproject '%s'.", metaproject)); } } + } diff --git a/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/MetaprojectName.java b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/MetaprojectName.java new file mode 100644 index 00000000000..8104e235c2d --- /dev/null +++ b/openbis/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/MetaprojectName.java @@ -0,0 +1,52 @@ +/* + * Copyright 2012 ETH Zuerich, CISD + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package ch.systemsx.cisd.openbis.generic.shared.basic; + +import java.util.regex.Pattern; + +import ch.systemsx.cisd.common.exception.UserFailureException; + +/** + * Metaproject name representation. The name cannot be null, empty or contain white spaces, commas, + * slashes or backslashes. + * + * @author pkupczyk + */ +public class MetaprojectName +{ + + private static final Pattern NAME_PATTERN = Pattern.compile("^[^\\s,/\\\\]+$", + Pattern.CASE_INSENSITIVE); + + public static void validate(String name) + { + if (name == null) + { + throw new UserFailureException("Metaproject name cannot be null"); + } + if (name.isEmpty()) + { + throw new UserFailureException("Metaproject name cannot be empty"); + } + if (NAME_PATTERN.matcher(name).matches() == false) + { + throw new UserFailureException( + "Metaproject name cannot contain white spaces, commas, slashes or backslashes"); + } + } + +} diff --git a/openbis/source/sql/generic/124/schema-124.sql b/openbis/source/sql/generic/124/schema-124.sql index 88be91ae5c2..76fc8305757 100644 --- a/openbis/source/sql/generic/124/schema-124.sql +++ b/openbis/source/sql/generic/124/schema-124.sql @@ -406,7 +406,6 @@ ALTER TABLE SCRIPTS ADD CONSTRAINT SCRI_UK UNIQUE(NAME,DBIN_ID); ALTER TABLE CORE_PLUGINS ADD CONSTRAINT COPL_NAME_VER_UK UNIQUE(NAME,VERSION); ALTER TABLE ENTITY_OPERATIONS_LOG ADD CONSTRAINT EOL_REG_ID_UK UNIQUE(REGISTRATION_ID); ALTER TABLE EXTERNAL_DATA_MANAGEMENT_SYSTEMS ADD CONSTRAINT EDMS_CODE_UK UNIQUE(CODE, DBIN_ID); -ALTER TABLE METAPROJECTS ADD CONSTRAINT METAPROJECTS_NAME_OWNER_UK UNIQUE (NAME, OWNER); -- NOTE: following uniqueness constraints for metaproject assignments work, because (null != null) in Postgres ALTER TABLE METAPROJECT_ASSIGNMENTS_ALL ADD CONSTRAINT METAPROJECT_ASSIGNMENTS_ALL_MEPR_ID_EXPE_ID_UK UNIQUE (MEPR_ID, EXPE_ID); ALTER TABLE METAPROJECT_ASSIGNMENTS_ALL ADD CONSTRAINT METAPROJECT_ASSIGNMENTS_ALL_MEPR_ID_SAMP_ID_UK UNIQUE (MEPR_ID, SAMP_ID); @@ -774,7 +773,7 @@ CREATE INDEX PRRELH_MAIN_PROJ_FK_EXPE_FK_I ON PROJECT_RELATIONSHIPS_HISTORY (MAI CREATE INDEX PRRELH_MAIN_PROJ_FK_SPACE_FK_I ON PROJECT_RELATIONSHIPS_HISTORY (MAIN_PROJ_ID, SPACE_ID); CREATE INDEX METAPROJECTS_OWNER_FK_I ON METAPROJECTS (OWNER); CREATE INDEX METAPROJECTS_NAME_I ON METAPROJECTS (NAME); -CREATE INDEX METAPROJECTS_NAME_OWNER_I ON METAPROJECTS (NAME, OWNER); +CREATE UNIQUE INDEX METAPROJECTS_NAME_OWNER_UK ON METAPROJECTS (lower(NAME), OWNER); CREATE INDEX METAPROJECT_ASSIGNMENTS_ALL_MEPR_FK_I ON METAPROJECT_ASSIGNMENTS_ALL (MEPR_ID); CREATE INDEX METAPROJECT_ASSIGNMENTS_ALL_EXPE_FK_I ON METAPROJECT_ASSIGNMENTS_ALL (EXPE_ID); CREATE INDEX METAPROJECT_ASSIGNMENTS_ALL_SAMP_FK_I ON METAPROJECT_ASSIGNMENTS_ALL (SAMP_ID); diff --git a/openbis/source/sql/postgresql/migration/migration-123-124.sql b/openbis/source/sql/postgresql/migration/migration-123-124.sql index d38e695ca76..0583fdf7f58 100644 --- a/openbis/source/sql/postgresql/migration/migration-123-124.sql +++ b/openbis/source/sql/postgresql/migration/migration-123-124.sql @@ -93,4 +93,6 @@ GRANT SELECT ON TABLE METAPROJECT_ASSIGNMENTS_ALL TO GROUP OPENBIS_READONLY; GRANT SELECT ON METAPROJECT_ASSIGNMENTS TO GROUP OPENBIS_READONLY; GRANT SELECT ON SEQUENCE METAPROJECT_ASSIGNMENT_ID_SEQ TO GROUP OPENBIS_READONLY; - +-- replace a unique constraint on metaproject (name, owner) pair after making the name case preserving but case insensitive for searching +ALTER TABLE METAPROJECTS DROP CONSTRAINT METAPROJECTS_NAME_OWNER_UK; +CREATE UNIQUE INDEX METAPROJECTS_NAME_OWNER_UK ON METAPROJECTS (lower(NAME), OWNER); \ No newline at end of file diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/basic/MetaprojectNameTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/basic/MetaprojectNameTest.java new file mode 100644 index 00000000000..a662717cb82 --- /dev/null +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/generic/shared/basic/MetaprojectNameTest.java @@ -0,0 +1,83 @@ +/* + * Copyright 2012 ETH Zuerich, CISD + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ + +package ch.systemsx.cisd.openbis.generic.shared.basic; + +import org.testng.annotations.Test; + +import ch.systemsx.cisd.common.exception.UserFailureException; + +/** + * @author pkupczyk + */ +public class MetaprojectNameTest +{ + + @Test(expectedExceptions = UserFailureException.class) + public void testValidateNullName() + { + MetaprojectName.validate(null); + } + + @Test(expectedExceptions = UserFailureException.class) + public void testValidateEmptyName() + { + MetaprojectName.validate(""); + } + + @Test(expectedExceptions = UserFailureException.class) + public void testValidateNameWithSpace() + { + MetaprojectName.validate("TEST NAME"); + } + + @Test(expectedExceptions = UserFailureException.class) + public void testValidateNameWithTab() + { + MetaprojectName.validate("TEST\tNAME"); + } + + @Test(expectedExceptions = UserFailureException.class) + public void testValidateNameWithNewLine() + { + MetaprojectName.validate("TEST\nNAME"); + } + + @Test(expectedExceptions = UserFailureException.class) + public void testValidateNameWithComma() + { + MetaprojectName.validate("TEST,NAME"); + } + + @Test(expectedExceptions = UserFailureException.class) + public void testValidateNameWithSlash() + { + MetaprojectName.validate("TEST/NAME"); + } + + @Test(expectedExceptions = UserFailureException.class) + public void testValidateNameWithBackslash() + { + MetaprojectName.validate("TEST\\NAME"); + } + + @Test + public void testValidateNameWithAllowedCharacters() + { + MetaprojectName.validate("TEST_NAME#with@allowed$characters?0123456789.[]-+*="); + } + +} diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationChangingServiceTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationChangingServiceTest.java index 5067c9d51ea..1ccf53942aa 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationChangingServiceTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationChangingServiceTest.java @@ -145,6 +145,13 @@ public class GeneralInformationChangingServiceTest extends SystemTestCase "My name is already used by the same owner"); } + @Test(expectedExceptions = UserFailureException.class) + public void testCreateMetaprojectWithNameThatIsUsedInDifferentCaseForSameOwner() + { + generalInformationChangingService.createMetaproject(sessionToken, "TEST_metaprojects", + "My name is already used by the same owner but in a different case"); + } + @Test public void testCreateMetaprojectWithNameDuplicatedForDifferentOwner() { @@ -152,12 +159,39 @@ public class GeneralInformationChangingServiceTest extends SystemTestCase "My name is already used by a different owner"); } + @Test + public void testCreateMetaprojectWithNameThatIsUsedInDifferentCaseForDifferentOwner() + { + generalInformationChangingService.createMetaproject(sessionToken, "TEST_metaprojects_2", + "My name is already used by a different owner in a different case"); + } + @Test(expectedExceptions = UserFailureException.class) public void testCreateMetaprojectWithEmptyName() { generalInformationChangingService.createMetaproject(sessionToken, null, "My name is empty"); } + @Test(expectedExceptions = UserFailureException.class) + public void testCreateMetaprojectWithNameThatContainsDisallowedCharacters() + { + generalInformationChangingService.createMetaproject(sessionToken, + "THIS NAME IS DISALLOWED", "My name is not allowed"); + } + + @Test + public void testCreateMetaprojectWithNameThatUsesVariedCaseMaintainsTheCase() + { + String name = "NameWithVariedCase"; + + Metaproject metaproject = + generalInformationChangingService.createMetaproject(sessionToken, name, + "My name uses varied case"); + assertEquals(name, metaproject.getName()); + List<String> names = getUtil().listMetaprojectNames(sessionToken); + assertTrue(names.contains(name)); + } + @Test public void testCreateMetaprojectWithEmptyDescription() { diff --git a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationServiceTest.java b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationServiceTest.java index 00d64e09ff3..c653bff1c07 100644 --- a/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationServiceTest.java +++ b/openbis/sourceTest/java/ch/systemsx/cisd/openbis/systemtest/api/v1/GeneralInformationServiceTest.java @@ -1064,4 +1064,14 @@ public class GeneralInformationServiceTest extends SystemTestCase assertTrue(metaprojectAssignments.getExperiments().get(0).toString().contains("STUB")); } + public void testGetMetaprojectReturnsMetaprojectForNameWithDifferentCase() + { + MetaprojectAssignments metaprojectAssignments = + generalInformationService.getMetaproject(sessionToken, new MetaprojectIdentifierId( + "/test/TEST_metaprojects")); + assertEquals("/test/TEST_METAPROJECTS", metaprojectAssignments.getMetaproject() + .getIdentifier()); + assertEquals("TEST_METAPROJECTS", metaprojectAssignments.getMetaproject().getName()); + } + } diff --git a/openbis/sourceTest/sql/postgresql/124/finish-124.sql b/openbis/sourceTest/sql/postgresql/124/finish-124.sql index 717656cd0ee..dcc130a624a 100644 --- a/openbis/sourceTest/sql/postgresql/124/finish-124.sql +++ b/openbis/sourceTest/sql/postgresql/124/finish-124.sql @@ -146,8 +146,6 @@ ALTER TABLE ONLY metaproject_assignments_all ADD CONSTRAINT metaproject_assignments_all_mepr_id_samp_id_uk UNIQUE (mepr_id, samp_id); ALTER TABLE ONLY metaproject_assignments_all ADD CONSTRAINT metaproject_assignments_all_pk PRIMARY KEY (id); -ALTER TABLE ONLY metaprojects - ADD CONSTRAINT metaprojects_name_owner_uk UNIQUE (name, owner); ALTER TABLE ONLY metaprojects ADD CONSTRAINT metaprojects_pk PRIMARY KEY (id); ALTER TABLE ONLY material_type_property_types @@ -307,7 +305,7 @@ CREATE INDEX metaproject_assignments_all_mate_fk_i ON metaproject_assignments_al CREATE INDEX metaproject_assignments_all_mepr_fk_i ON metaproject_assignments_all USING btree (mepr_id); CREATE INDEX metaproject_assignments_all_samp_fk_i ON metaproject_assignments_all USING btree (samp_id); CREATE INDEX metaprojects_name_i ON metaprojects USING btree (name); -CREATE INDEX metaprojects_name_owner_i ON metaprojects USING btree (name, owner); +CREATE UNIQUE INDEX METAPROJECTS_NAME_OWNER_UK ON METAPROJECTS (lower(NAME), OWNER); CREATE INDEX metaprojects_owner_fk_i ON metaprojects USING btree (owner); CREATE INDEX mtpt_maty_fk_i ON material_type_property_types USING btree (maty_id); CREATE INDEX mtpt_pers_fk_i ON material_type_property_types USING btree (pers_id_registerer); diff --git a/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/api/v1/IGeneralInformationChangingService.java b/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/api/v1/IGeneralInformationChangingService.java index db64b7dd92d..46f66462ee7 100644 --- a/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/api/v1/IGeneralInformationChangingService.java +++ b/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/api/v1/IGeneralInformationChangingService.java @@ -97,7 +97,7 @@ public interface IGeneralInformationChangingService extends IRpcService /** * Updates an existing metaproject. * - * @param metaprojectId Id of the metaproject + * @param metaprojectId Id of the metaproject to update * @param name New name of the metaproject * @param descriptionOrNull New description of the metaproject * @return Updated metaproject @@ -108,18 +108,18 @@ public interface IGeneralInformationChangingService extends IRpcService String name, String descriptionOrNull); /** - * Deletes existing metaproject. + * Deletes an existing metaproject. * - * @param metaprojectId Id of a metaproject to delete + * @param metaprojectId Id of the metaproject to delete * @throws UserFailureException when a metaproject with the specified id doesn't exist. * @since 1.3 */ public void deleteMetaproject(String sessionToken, IMetaprojectId metaprojectId); /** - * Adds given entities to existing metaproject. + * Adds given entities to an existing metaproject. * - * @param metaprojectId Id of a metaproject + * @param metaprojectId Id of the metaproject * @param assignmentsToAdd Assignments that should be added to the metaproject * @throws UserFailureException when a metaproject with the specified id doesn't exist. * @since 1.3 @@ -128,9 +128,9 @@ public interface IGeneralInformationChangingService extends IRpcService MetaprojectAssignmentsIds assignmentsToAdd); /** - * Removes given entities to existing metaproject. + * Removes given entities from an existing metaproject. * - * @param metaprojectId Id of a metaproject + * @param metaprojectId Id of the metaproject * @param assignmentsToRemove Assignments that should be removed from the metaproject * @throws UserFailureException when a metaproject with the specified id doesn't exist. * @since 1.3 diff --git a/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/api/v1/dto/id/metaproject/MetaprojectIdentifierId.java b/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/api/v1/dto/id/metaproject/MetaprojectIdentifierId.java index f73c9393d90..017db45c4d0 100644 --- a/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/api/v1/dto/id/metaproject/MetaprojectIdentifierId.java +++ b/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/api/v1/dto/id/metaproject/MetaprojectIdentifierId.java @@ -32,7 +32,7 @@ public class MetaprojectIdentifierId extends ObjectIdentifierId implements IMeta private static final long serialVersionUID = ServiceVersionHolder.VERSION; /** - * @param identifier Metaproject identifier, e.e. "/MY_USER/MY_METAPROJECT". + * @param identifier Metaproject identifier, e.g. "/MY_USER/MY_METAPROJECT". */ public MetaprojectIdentifierId(String identifier) { diff --git a/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/MetaprojectIdentifier.java b/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/MetaprojectIdentifier.java index ae156e202a2..8dd00e2a9ec 100644 --- a/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/MetaprojectIdentifier.java +++ b/openbis_api/source/java/ch/systemsx/cisd/openbis/generic/shared/basic/dto/MetaprojectIdentifier.java @@ -21,6 +21,9 @@ import java.io.Serializable; import ch.systemsx.cisd.base.annotation.JsonObject; /** + * Metaproject identifier representation. The identifier consists of metaproject owner id and + * metaproject name, e.g. "/MY_USER/MY_METAPROJECT". + * * @author pkupczyk */ @JsonObject("MetaprojectIdentifier") -- GitLab