From 203f00875e84d5560ccc03890a1fd00b69aa5f9b Mon Sep 17 00:00:00 2001 From: rkv Date: Mon, 18 Feb 2013 12:38:29 +0000 Subject: [PATCH] Fix for mantis #0022110: Same files should not be attached again to the document after check-in operation: New attached file replaces now the old one with the same format. Attachment modification date is updated. --- .../src/org/splat/dal/bo/som/File.java | 17 +- .../splat/service/ScenarioServiceImpl.java | 165 ++++++++++++------ .../splat/service/TestScenarioService.java | 80 ++++++--- 3 files changed, 182 insertions(+), 80 deletions(-) diff --git a/Workspace/Siman-Common/src/org/splat/dal/bo/som/File.java b/Workspace/Siman-Common/src/org/splat/dal/bo/som/File.java index 5675884..6ffa6b4 100644 --- a/Workspace/Siman-Common/src/org/splat/dal/bo/som/File.java +++ b/Workspace/Siman-Common/src/org/splat/dal/bo/som/File.java @@ -37,7 +37,7 @@ public class File extends Persistent { this.myfile = null; } // Internal constructors - public File (String path) { + public File (final String path) { // ---------------------------- Calendar current = Calendar.getInstance(); String[] table = path.split("\\x2E"); @@ -47,7 +47,7 @@ public class File extends Persistent { this.date = current.getTime(); // Today this.myfile = null; } - protected File (String path, String format, Date date) { + protected File (final String path, final String format, final Date date) { // ------------------------------------------------------ this.path = path; // The corresponding physical file may not exist yet this.format = format; // The format name may be different from the physical file extension @@ -69,7 +69,9 @@ public class File extends Persistent { */ public java.io.File asFile () { // ----------------------------- - if (myfile == null) myfile = new java.io.File(Database.getRepositoryVaultPath() + path); + if (myfile == null) { + myfile = new java.io.File(Database.getRepositoryVaultPath() + path); + } return myfile; } @@ -102,9 +104,16 @@ public class File extends Persistent { // Protected service // ============================================================================================================================== - public void changePath (String path) { + public void changePath (final String path) { // --------------------------------------- this.path = path; this.myfile = null; } + /** + * Set the date. + * @param date the date to set + */ + public void setDate(final Date date) { + this.date = date; + } } \ No newline at end of file diff --git a/Workspace/Siman-Common/src/org/splat/service/ScenarioServiceImpl.java b/Workspace/Siman-Common/src/org/splat/service/ScenarioServiceImpl.java index ad4af9f..4562f64 100644 --- a/Workspace/Siman-Common/src/org/splat/service/ScenarioServiceImpl.java +++ b/Workspace/Siman-Common/src/org/splat/service/ScenarioServiceImpl.java @@ -38,6 +38,7 @@ import org.splat.dal.bo.som.SimulationContext; import org.splat.dal.bo.som.Study; import org.splat.dal.bo.som.UsedByRelation; import org.splat.dal.bo.som.UsesRelation; +import org.splat.dal.bo.som.Document.Properties; import org.splat.dal.dao.kernel.RoleDAO; import org.splat.dal.dao.kernel.UserDAO; import org.splat.dal.dao.som.KnowledgeElementDAO; @@ -573,53 +574,10 @@ public class ScenarioServiceImpl implements ScenarioService { // Attach the file via ConvertsRelation, create a new document or // create a new version of the document dprop.setAuthor(aUser).setDate(aDate).setFormat(fileFormat); - Publication pub, newPub; if (doc.getId() > 0) { - // If the document already exists then - // Attach the file via ConvertsRelation if the extension of the - // new file differs from the old one. - // If file format (i.e. extension) is the same then create a new - // version of the document. - // Find the document publication - pub = step.getDocument(doc.getId()); - if (pub == null) { - throw new InvalidPropertyException( - MessageKeyEnum.SCN_000002.toString(), doc.getId()); - } - if (pub.value() == null) { - throw new MismatchException(MessageKeyEnum.SCN_000002 - .toString(), doc.getId()); - } - if (LOG.isDebugEnabled()) { - LOG.debug("Old format: " + pub.value().getFormat() - + " => New format: " + fileFormat); - } - // If formats are same then create a new document version - if (pub.value().getFormat() != null - && pub.value().getFormat().equals(fileFormat)) { - newPub = getStepService().versionDocument(step, pub, dprop); - if (LOG.isDebugEnabled()) { - LOG.debug("Created document type: " - + newPub.value().getType().getName() - + ", format: " + newPub.value().getFormat()); - } - // Remeber the link from the old document to the new document version - newVersion.put(pub.value(), newPub.value()); - // Remember the new version publication - newVers.add(newPub); - - saveFile(newPub, step, file); - - } else { // If formats are different then attach the new file via ConvertsRelation - ConvertsRelation export = getPublicationService().attach( - pub, fileFormat); - if (LOG.isDebugEnabled()) { - LOG.debug("Moving " + upfile.getName() + " to " - + export.getTo().asFile().getPath()); - } - upfile.renameTo(export.getTo().asFile()); - } + checkinExistingDoc(step, doc, dprop, fileFormat, upfile, + newVersion, newVers); } else { // Otherwise create a new document of the result type @@ -641,12 +599,107 @@ public class ScenarioServiceImpl implements ScenarioService { docname += "_" + i; // The generated new document title dprop.setDescription("Checked in").setName(docname); - newPub = getStepService().createDocument(step, dprop); + Publication newPub = getStepService().createDocument(step, + dprop); // Remeber the new document newDocs.add(newPub); - saveFile(newPub, step, file); + saveFile(newPub, step, upfile); + } + } + } + + /** + * Check in existing document. + * + * @param step + * study step to check in + * @param doc + * document DTO to check in + * @param dprop + * document properties + * @param fileFormat + * checked in file format + * @param upfile + * the file to check in + * @param newVersion + * the map of created versions during this check in + * @param newVers + * the list of new versions created during this check in + * @throws InvalidPropertyException + * if publication of the document is not found in the step + * @throws MismatchException + * if the found publication does not point to a document + * @throws IOException + * if can not move the file into the vault + * @throws MultiplyDefinedException + * thrown by versionDocument + * @throws MissedPropertyException + * thrown by versionDocument + * @throws NotApplicableException + * if failed saving of a new publication with a given state + */ + private void checkinExistingDoc(final Step step, final DocumentDTO doc, + final Properties dprop, final String fileFormat, + final java.io.File upfile, + final Map newVersion, + final List newVers) throws InvalidPropertyException, + MismatchException, MissedPropertyException, + MultiplyDefinedException, IOException, NotApplicableException { + // If the document already exists then + // Attach the file via ConvertsRelation if the extension of the + // new file differs from the old one. + // If file format (i.e. extension) is the same then create a new + // version of the document. + // Find the document publication + Publication pub = step.getDocument(doc.getId()); + if (pub == null) { + throw new InvalidPropertyException(MessageKeyEnum.SCN_000002 + .toString(), doc.getId()); + } + if (pub.value() == null) { + throw new MismatchException(MessageKeyEnum.SCN_000002.toString(), + doc.getId()); + } + if (LOG.isDebugEnabled()) { + LOG.debug("Old format: " + pub.value().getFormat() + + " => New format: " + fileFormat); + } + // If formats are same then create a new document version + if (pub.value().getFormat() != null + && pub.value().getFormat().equals(fileFormat)) { + Publication newPub = getStepService().versionDocument(step, pub, + dprop); + if (LOG.isDebugEnabled()) { + LOG.debug("Created document type: " + + newPub.value().getType().getName() + ", format: " + + newPub.value().getFormat()); + } + // Remeber the link from the old document to the new document version + newVersion.put(pub.value(), newPub.value()); + // Remember the new version publication + newVers.add(newPub); + + saveFile(newPub, step, upfile); + + } else { // If formats are different then attach the new file via ConvertsRelation + File attach = pub.value().getAttachedFile(fileFormat); + if (attach == null) { + // If there is no attachment with this extension then attach the new one + ConvertsRelation export = getPublicationService().attach(pub, + fileFormat); + if (LOG.isDebugEnabled()) { + LOG.debug("Moving " + upfile.getName() + " to " + + export.getTo().asFile().getPath()); + } + upfile.renameTo(export.getTo().asFile()); + } else { + // If an attachment with this extension already exists then + // replace it by the new one + upfile.renameTo(attach.asFile()); + // Update attached file modification date + attach.setDate(new Date()); } } } @@ -658,18 +711,18 @@ public class ScenarioServiceImpl implements ScenarioService { * the new publication to save * @param step * the study step to publish the document - * @param file - * the downloaded file DTO + * @param upfile + * the downloaded file * @throws IOException * if a file can't be moved into the vault * @throws NotApplicableException * if failed saving of a new publication with a given state */ private void saveFile(final Publication newPub, final Step step, - final FileDTO file) throws IOException, NotApplicableException { + final java.io.File upfile) throws IOException, + NotApplicableException { // Attach the file to the created document java.io.File updir = newPub.getSourceFile().asFile(); - java.io.File upfile = new java.io.File(file.getPath()); if (LOG.isDebugEnabled()) { LOG.debug("Moving \"" + upfile.getName() + "\" to \"" + updir.getPath() + "\"."); @@ -681,7 +734,7 @@ public class ScenarioServiceImpl implements ScenarioService { } else { throw new IOException( "Can't delete the existing destination file to move file from " - + file.getPath() + " to " + + upfile.getAbsolutePath() + " to " + updir.getAbsolutePath()); } } @@ -690,8 +743,9 @@ public class ScenarioServiceImpl implements ScenarioService { // The old publication is removed from the scenario here. getPublicationService().saveAs(newPub, ProgressState.inWORK); // May throw FileNotFound if rename was not done } else { - throw new IOException("Can't move file from " + file.getPath() - + " to " + updir.getAbsolutePath()); + throw new IOException("Can't move file from " + + upfile.getAbsolutePath() + " to " + + updir.getAbsolutePath()); } } @@ -958,10 +1012,11 @@ public class ScenarioServiceImpl implements ScenarioService { } return isOk; } - + /** * * {@inheritDoc} + * * @see org.splat.service.ScenarioService#renameScenario(java.lang.String) */ @Transactional diff --git a/Workspace/Siman-Common/src/test/splat/service/TestScenarioService.java b/Workspace/Siman-Common/src/test/splat/service/TestScenarioService.java index 9776fc2..9afe64a 100644 --- a/Workspace/Siman-Common/src/test/splat/service/TestScenarioService.java +++ b/Workspace/Siman-Common/src/test/splat/service/TestScenarioService.java @@ -22,6 +22,7 @@ import java.util.Set; import org.splat.dal.bo.kernel.Relation; import org.splat.dal.bo.kernel.User; +import org.splat.dal.bo.som.ConvertsRelation; import org.splat.dal.bo.som.Document; import org.splat.dal.bo.som.DocumentType; import org.splat.dal.bo.som.KnowledgeElementType; @@ -350,7 +351,6 @@ public class TestScenarioService extends BaseTest { User user = aScen.getAuthor(); long userId = user.getIndex(); - // //////////////////////////////////////////////////////// // Call checkin method for empty list of modules. @@ -374,9 +374,7 @@ public class TestScenarioService extends BaseTest { aScen = _scenarioDAO.get(scenarioId); Assert.assertFalse(aScen.isCheckedout(), "Scenario is still marked as checked out after checkin."); - - - + // //////////////////////////////////////////////////////// // Call checkin method for good prepared transient data. @@ -384,6 +382,16 @@ public class TestScenarioService extends BaseTest { steps = _scenarioService.getScenarioInfo(scenarioId); _scenarioService.checkout(aScen, user); + // Remember modification dates of all attached files + Map dates = new HashMap(); + for (Publication p : aScen.getDocums()) { + for (Relation r : p.value().getRelations(ConvertsRelation.class)) { + org.splat.dal.bo.som.File attach = ((ConvertsRelation) r) + .getTo(); + dates.put(attach.getIndex(), attach.getDate()); + } + } + // Prepare test data for checkin // Checkin only two first steps (geom and mesh) for (StepDTO step : steps) { @@ -401,6 +409,7 @@ public class TestScenarioService extends BaseTest { aScen = _scenarioDAO.get(scenarioId); Assert.assertFalse(aScen.isCheckedout(), "Scenario is still marked as checked out after checkin."); + boolean modifDatesChecked = false; // Check that new document versions are created for checked in documents for (StepDTO step : stepsToCheckin) { for (DocumentDTO docDTO : step.getDocs()) { @@ -440,18 +449,21 @@ public class TestScenarioService extends BaseTest { Assert.assertFalse(aScen.publishes(prevDoc)); // Check that presentation of the previous version is removed checkFiles(docDTO, newPub); - + // Formats of files are new if they are according to the document's type on the study step if ("py".equals(prevDoc.getFormat()) - && "geometry".equals(prevDoc.getType().getName())) { - Assert.assertEquals(newPub.value().getFormat(), "brep"); - Assert.assertEquals(newPub.getSourceFile().getFormat(), + && "geometry".equals(prevDoc.getType() + .getName())) { + Assert.assertEquals(newPub.value().getFormat(), "brep"); + Assert.assertEquals(newPub.getSourceFile() + .getFormat(), "brep"); Assert.assertEquals(newPub.getSourceFile() .getRelativePath().substring( newPub.getSourceFile() - .getRelativePath().lastIndexOf( - '.') + 1), "brep"); + .getRelativePath() + .lastIndexOf('.') + 1), + "brep"); } // Check that uses relations are copied correctly @@ -468,7 +480,8 @@ public class TestScenarioService extends BaseTest { if ((lastPub.value().getPreviousVersion() != null) && (lastPub.value() .getPreviousVersion() - .getIndex() == used.getIndex())) { + .getIndex() == used + .getIndex())) { toBeUsed = lastPub; break; } @@ -506,11 +519,33 @@ public class TestScenarioService extends BaseTest { Assert.assertTrue(attFile.asFile().exists(), "File " + docDTO.getFiles().get(0).getPath() + " attached to the document " - + docDTO.getTitle() + "#" + docDTO.getId() + " doesn't exist"); - LOG.debug("Source format: " + curDoc.getFormat() + ", new format: " + newFormat); + + docDTO.getTitle() + "#" + docDTO.getId() + + " doesn't exist"); + LOG.debug("Source format: " + curDoc.getFormat() + + ", new format: " + newFormat); + // Check that attachment with the same format is not duplicated. + int attachNb = 0; + for (Relation conv : curDoc + .getRelations(ConvertsRelation.class)) { + if (newFormat.equals(((ConvertsRelation) conv) + .getTo().getFormat())) { + attachNb++; + } + } + Assert + .assertEquals(attachNb, 1, + "Attachment with the same format must be only one."); + + // Check that the attached file date is updated + if (dates.containsKey(attFile.getIndex())) { + Assert + .assertTrue(attFile.getDate().compareTo( + dates.get(attFile.getIndex())) > 0, + "Attachment modification date is not updated."); + modifDatesChecked = true; + } } - } else { // Check that new documents are created for new data boolean found = false; @@ -563,6 +598,11 @@ public class TestScenarioService extends BaseTest { } } + Assert + .assertTrue( + modifDatesChecked, + "No modification date is checked because no files were attached when attachment with same extension already exists."); + // /////////////////////////////////////////////////////////// // Call checkin method for a not existing id. try { @@ -671,8 +711,7 @@ public class TestScenarioService extends BaseTest { for (FileDTO file : doc.getFiles()) { if (file.getPath().endsWith(format) || (file.getPath().endsWith("py") && (format - .equals("brep") || format - .equals("med")))) { + .equals("brep") || format.equals("med")))) { // Create a file in the download directory docToCheckin.addFile(createDownloadedFile(userId, doc.getTitle() + "_result", format)); @@ -824,11 +863,10 @@ public class TestScenarioService extends BaseTest { // .brep // .med dprop.setName("document" + i++).setType(dtype); -/* if (step.getNumber() > 3) { - dprop.setFormat("med"); - } else { -*/ dprop.setFormat("py"); -// } + /* + * if (step.getNumber() > 3) { dprop.setFormat("med"); } else { + */dprop.setFormat("py"); + // } dprop.setLocalPath(dprop.getName() + "." + dprop.getFormat()); Publication pub = createDoc(aScenario, aScStep, dprop, "med", false); -- 2.39.2