diff --git a/app/attributes/attributecontroller.cpp b/app/attributes/attributecontroller.cpp index 6c071485a..ae29f39d4 100644 --- a/app/attributes/attributecontroller.cpp +++ b/app/attributes/attributecontroller.cpp @@ -18,6 +18,7 @@ #include "attributetabmodel.h" #include "fieldvalidator.h" +#include #include #include @@ -622,8 +623,21 @@ void AttributeController::updateOnFeatureChange() ); if ( shouldUseRememberedValue ) { - mFeatureLayerPair.featureRef().setAttribute( fieldIndex, rememberedValue ); - itemData->setRawValue( rememberedValue ); + QVariant valueToUse = rememberedValue; + + if ( itemData->editorWidgetType() == QStringLiteral( "ExternalResource" ) && !rememberedValue.toString().isEmpty() ) + { + QString clonedPath; + if ( !cloneExternalResource( *itemData, rememberedValue.toString(), clonedPath ) ) + { + ++formItemsIterator; + continue; + } + valueToUse = clonedPath; + } + + mFeatureLayerPair.featureRef().setAttribute( fieldIndex, valueToUse ); + itemData->setRawValue( valueToUse ); } } } @@ -1576,6 +1590,43 @@ void AttributeController::onFeatureAdded( QgsFeatureId newFeatureId ) emit featureIdChanged(); } +void AttributeController::resolveExternalResourcePaths( const QVariantMap &config, QString &targetDir, QString &prefix ) const +{ + const FeatureLayerPair parentPair = mParentController ? mParentController->featureLayerPair() : FeatureLayerPair(); + targetDir = InputUtils::resolveTargetDir( QgsProject::instance()->homePath(), config, mFeatureLayerPair, parentPair, QgsProject::instance() ); + prefix = InputUtils::resolvePrefixForRelativePath( config[ QStringLiteral( "RelativeStorage" ) ].toInt(), QgsProject::instance()->homePath(), targetDir ); +} + +bool AttributeController::cloneExternalResource( const FormItem &item, const QString &rememberedPath, QString &newRelativePath ) const +{ + QString targetDir, prefix; + resolveExternalResourcePaths( item.editorWidgetConfig(), targetDir, prefix ); + const QString src = InputUtils::getAbsolutePath( rememberedPath, prefix ); + const QFileInfo fi( src ); + + if ( !fi.isFile() ) + { + CoreUtils::log( QStringLiteral( "Attribute Controller" ), QStringLiteral( "User wanted to reuse photo value, but the value is not a valid file: %1" ).arg( src ) ); + return false; + } + + // temporary name; renamePhotos() applies the custom naming expression at save + QString newName = QDateTime::currentDateTime().toString( QStringLiteral( "yyyyMMdd_HHmmsszzz" ) ); + if ( !fi.suffix().isEmpty() ) + newName += QStringLiteral( "." ) + fi.suffix(); + + const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( newName, targetDir ) ); + + if ( !InputUtils::copyFile( src, dst ) ) + { + CoreUtils::log( QStringLiteral( "Attribute Controller" ), QStringLiteral( "User wanted to reuse photo value, but the file could not be copied from %1 to %2" ).arg( src, dst ) ); + return false; + } + + newRelativePath = InputUtils::getRelativePath( dst, prefix ); + return true; +} + void AttributeController::renamePhotos() { const QStringList photoNameFormat = QgsProject::instance()->entryList( QStringLiteral( "Mergin" ), QStringLiteral( "PhotoNaming/%1" ).arg( mFeatureLayerPair.layer()->id() ) ); @@ -1636,9 +1687,8 @@ void AttributeController::renamePhotos() continue; } - const FeatureLayerPair parentPair = mParentController ? mParentController->featureLayerPair() : FeatureLayerPair(); - const QString targetDir = InputUtils::resolveTargetDir( QgsProject::instance()->homePath(), config, mFeatureLayerPair, parentPair, QgsProject::instance() ); - const QString prefix = InputUtils::resolvePrefixForRelativePath( config[ QStringLiteral( "RelativeStorage" ) ].toInt(), QgsProject::instance()->homePath(), targetDir ); + QString targetDir, prefix; + resolveExternalResourcePaths( config, targetDir, prefix ); const QString src = InputUtils::getAbsolutePath( mFeatureLayerPair.feature().attribute( item->fieldIndex() ).toString(), prefix ); QString newName = val.toString(); @@ -1649,13 +1699,15 @@ void AttributeController::renamePhotos() InputUtils::sanitizePath( newName ); const QFileInfo fi( src ); - newName = QStringLiteral( "%1.%2" ).arg( newName, fi.completeSuffix() ); + newName = QStringLiteral( "%1.%2" ).arg( newName, fi.suffix() ); const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( newName, targetDir ) ); if ( InputUtils::renameFile( src, dst ) ) { const QString newValue = InputUtils::getRelativePath( dst, prefix ); setFormValue( item->id(), newValue ); + // avoids renaming it again on the next save + item->setOriginalValue( newValue ); expressionContext.setFeature( featureLayerPair().featureRef() ); } else @@ -1680,9 +1732,8 @@ void AttributeController::saveSketches() if ( item->rawValue().isValid() ) { const QVariantMap config = item->editorWidgetConfig(); - const FeatureLayerPair &parentPair = mParentController ? mParentController->featureLayerPair() : FeatureLayerPair(); - const QString targetDir = InputUtils::resolveTargetDir( QgsProject::instance()->homePath(), config, mFeatureLayerPair, parentPair, QgsProject::instance() ); - const QString prefix = InputUtils::resolvePrefixForRelativePath( config[ QStringLiteral( "RelativeStorage" ) ].toInt(), QgsProject::instance()->homePath(), targetDir ); + QString targetDir, prefix; + resolveExternalResourcePaths( config, targetDir, prefix ); const QString src = InputUtils::getAbsolutePath( mFeatureLayerPair.feature().attribute( item->fieldIndex() ).toString(), prefix ); const QString tempFilePath = QString( "%1/%2/%3" ).arg( QDir::temp().absolutePath(), QUrl::fromLocalFile( QgsProject::instance()->homePath() ).fileName(), src.section( "/", -1 ) ); diff --git a/app/attributes/attributecontroller.h b/app/attributes/attributecontroller.h index 59535feb5..b05d7c9db 100644 --- a/app/attributes/attributecontroller.h +++ b/app/attributes/attributecontroller.h @@ -225,6 +225,11 @@ class AttributeController : public QObject */ bool allowTabs( QgsAttributeEditorContainer *container ); + //! resolves the storage folder and relative-path prefix for an ExternalResource field + void resolveExternalResourcePaths( const QVariantMap &config, QString &targetDir, QString &prefix ) const; + //! copies the remembered external resource file to a new uniquely named file, so the new feature does not share it with the previous one + //! returns false (and logs the problem) if the source is not a valid file or the copy fails + bool cloneExternalResource( const FormItem &item, const QString &rememberedPath, QString &newRelativePath ) const; //! renames photos if necessary void renamePhotos(); //! save temporary sketched image to original image diff --git a/app/inpututils.cpp b/app/inpututils.cpp index 49fbb6716..5fd6b5203 100644 --- a/app/inpututils.cpp +++ b/app/inpututils.cpp @@ -1018,6 +1018,8 @@ QString InputUtils::resolveTargetDir( const QString &homePath, const QVariantMap { QString result = evaluateExpression( pair, parentPair, activeProject, expression ); sanitizePath( result ); + if ( !result.isEmpty() && !QDir::isAbsolutePath( result ) ) + result = QDir( homePath ).absoluteFilePath( result ); return result; } else @@ -1029,6 +1031,8 @@ QString InputUtils::resolveTargetDir( const QString &homePath, const QVariantMap } else { + if ( !QDir::isAbsolutePath( defaultRoot ) ) + defaultRoot = QDir( homePath ).absoluteFilePath( defaultRoot ); return defaultRoot; } } diff --git a/app/test/testattributecontroller.cpp b/app/test/testattributecontroller.cpp index 2145bb058..3f618117a 100644 --- a/app/test/testattributecontroller.cpp +++ b/app/test/testattributecontroller.cpp @@ -894,6 +894,183 @@ void TestAttributeController::testPhotoRenaming() } } +void TestAttributeController::testPhotoReuseRenamesWithFreshExpressionValue() +{ + QString projectName = QStringLiteral( "testPhotoReuseRenamesWithFreshExpressionValue" ); + QString projectDir = QDir::tempPath() + "/MM_test_projects/" + projectName; + + QDir tempDir( projectDir ); + QVERIFY( tempDir.removeRecursively() ); + + QVERIFY( InputUtils::cpDir( TestUtils::testDataDir() + "/test_photo_rename", projectDir ) ); + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image1.jpg" ) ) ); + + QVERIFY( QgsProject::instance()->read( projectDir + QStringLiteral( "/test_photo_rename.qgz" ) ) ); + + QgsMapLayer *layer = QgsProject::instance()->mapLayersByName( QStringLiteral( "Survey" ) ).at( 0 ); + QgsVectorLayer *surveyLayer = static_cast( layer ); + QVERIFY( surveyLayer && surveyLayer->isValid() ); + + RememberAttributesController remController; + remController.reset(); + remController.setRememberValuesAllowed( true ); + + // feature 1: notes = "first" -> naming expression 'image_' + "notes" saves image_first.jpg + QgsFeature feat1( surveyLayer->fields() ); + FeatureLayerPair pair1( feat1, surveyLayer ); + + AttributeController controller1; + controller1.setRememberAttributesController( &remController ); + controller1.setFeatureLayerPair( pair1 ); + + const TabItem *tab1 = controller1.tabItem( 0 ); + const QVector items1 = tab1->formItems(); + + // mark the photo field to be reused on the next new feature + remController.setShouldRememberValue( surveyLayer, 3, true ); + + controller1.setFormValue( items1.at( 2 ), QStringLiteral( "first" ) ); + controller1.setFormValue( items1.at( 3 ), QStringLiteral( "image1.jpg" ) ); + controller1.save(); + + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); + + // feature 2 reuses the photo but has notes = "second" - expression must re-evaluate fresh + QgsFeature feat2( surveyLayer->fields() ); + FeatureLayerPair pair2( feat2, surveyLayer ); + + AttributeController controller2; + controller2.setRememberAttributesController( &remController ); + controller2.setFeatureLayerPair( pair2 ); + + const TabItem *tab2 = controller2.tabItem( 0 ); + const QVector items2 = tab2->formItems(); + + controller2.setFormValue( items2.at( 2 ), QStringLiteral( "second" ) ); + controller2.save(); + + const QgsFeature f2 = controller2.featureLayerPair().feature(); + + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); // untouched + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_second.jpg" ) ) ); // fresh name + QCOMPARE( f2.attribute( 3 ), QStringLiteral( "image_second.jpg" ) ); +} + +void TestAttributeController::testPhotoRenamingNotRepeatedOnResave() +{ + QString projectName = QStringLiteral( "testPhotoRenamingNotRepeatedOnResave" ); + QString projectDir = QDir::tempPath() + "/MM_test_projects/" + projectName; + + QDir tempDir( projectDir ); + QVERIFY( tempDir.removeRecursively() ); + + QVERIFY( InputUtils::cpDir( TestUtils::testDataDir() + "/test_photo_rename", projectDir ) ); + QVERIFY( QgsProject::instance()->read( projectDir + QStringLiteral( "/test_photo_rename.qgz" ) ) ); + + QgsMapLayer *layer = QgsProject::instance()->mapLayersByName( QStringLiteral( "Survey" ) ).at( 0 ); + QgsVectorLayer *surveyLayer = static_cast( layer ); + QVERIFY( surveyLayer && surveyLayer->isValid() ); + + RememberAttributesController remController; + remController.reset(); + remController.setRememberValuesAllowed( true ); + + QgsFeature feat1( surveyLayer->fields() ); + FeatureLayerPair pair1( feat1, surveyLayer ); + + AttributeController controller1; + controller1.setRememberAttributesController( &remController ); + controller1.setFeatureLayerPair( pair1 ); + + const TabItem *tab1 = controller1.tabItem( 0 ); + const QVector items1 = tab1->formItems(); + + remController.setShouldRememberValue( surveyLayer, 3, true ); + + controller1.setFormValue( items1.at( 2 ), QStringLiteral( "first" ) ); + controller1.setFormValue( items1.at( 3 ), QStringLiteral( "image1.jpg" ) ); + controller1.save(); + + QgsFeature feat2( surveyLayer->fields() ); + FeatureLayerPair pair2( feat2, surveyLayer ); + + AttributeController controller2; + controller2.setRememberAttributesController( &remController ); + controller2.setFeatureLayerPair( pair2 ); + + const TabItem *tab2 = controller2.tabItem( 0 ); + const QVector items2 = tab2->formItems(); + + controller2.setFormValue( items2.at( 2 ), QStringLiteral( "second" ) ); + controller2.save(); + + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_second.jpg" ) ) ); + + // save the very same feature again without changing anything - must not re-trigger the rename + controller2.save(); + + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_second.jpg" ) ) ); + QVERIFY( !QFile::exists( projectDir + QStringLiteral( "/image_second (1).jpg" ) ) ); +} + +void TestAttributeController::testReusedPhotoIsIndependentFile() +{ + QString projectName = QStringLiteral( "testReusedPhotoIsIndependentFile" ); + QString projectDir = QDir::tempPath() + "/MM_test_projects/" + projectName; + + QDir tempDir( projectDir ); + QVERIFY( tempDir.removeRecursively() ); + + QVERIFY( InputUtils::cpDir( TestUtils::testDataDir() + "/test_photo_rename", projectDir ) ); + QVERIFY( QgsProject::instance()->read( projectDir + QStringLiteral( "/test_photo_rename.qgz" ) ) ); + + QgsMapLayer *layer = QgsProject::instance()->mapLayersByName( QStringLiteral( "Survey" ) ).at( 0 ); + QgsVectorLayer *surveyLayer = static_cast( layer ); + QVERIFY( surveyLayer && surveyLayer->isValid() ); + + RememberAttributesController remController; + remController.reset(); + remController.setRememberValuesAllowed( true ); + + QgsFeature feat1( surveyLayer->fields() ); + FeatureLayerPair pair1( feat1, surveyLayer ); + + AttributeController controller1; + controller1.setRememberAttributesController( &remController ); + controller1.setFeatureLayerPair( pair1 ); + + const TabItem *tab1 = controller1.tabItem( 0 ); + const QVector items1 = tab1->formItems(); + + remController.setShouldRememberValue( surveyLayer, 3, true ); + + controller1.setFormValue( items1.at( 2 ), QStringLiteral( "first" ) ); + controller1.setFormValue( items1.at( 3 ), QStringLiteral( "image1.jpg" ) ); + controller1.save(); + + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); + + QgsFeature feat2( surveyLayer->fields() ); + FeatureLayerPair pair2( feat2, surveyLayer ); + + AttributeController controller2; + controller2.setRememberAttributesController( &remController ); + controller2.setFeatureLayerPair( pair2 ); + + const QString reusedRelativePath = controller2.featureLayerPair().feature().attribute( 3 ).toString(); + QVERIFY( !reusedRelativePath.isEmpty() ); + // must be its own file, not literally feature 1's path + QVERIFY( reusedRelativePath != QStringLiteral( "image_first.jpg" ) ); + + const QString reusedAbsolutePath = projectDir + "/" + reusedRelativePath; + QVERIFY( QFile::exists( reusedAbsolutePath ) ); + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); + + // removing feature 2's file directly must not affect feature 1's, since they're independent + QVERIFY( QFile::remove( reusedAbsolutePath ) ); + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); +} + void TestAttributeController::testHtmlAndTextWidgets() { QString projectDir = TestUtils::testDataDir() + "/expressions"; diff --git a/app/test/testattributecontroller.h b/app/test/testattributecontroller.h index 5fa25b9dc..2b9bb7079 100644 --- a/app/test/testattributecontroller.h +++ b/app/test/testattributecontroller.h @@ -28,6 +28,16 @@ class TestAttributeController: public QObject void testRawValue(); void testFieldsOutsideForm(); void testPhotoRenaming(); + + //! A reused photo must be renamed with the new feature's own expression value, not the old one + void testPhotoReuseRenamesWithFreshExpressionValue(); + + //! Saving the same feature twice must not rename an already-renamed photo again + void testPhotoRenamingNotRepeatedOnResave(); + + //! Reusing a photo must create an independent file, not just copy the path string + void testReusedPhotoIsIndependentFile(); + void testHtmlAndTextWidgets(); void testVirtualFields(); diff --git a/app/test/testutilsfunctions.cpp b/app/test/testutilsfunctions.cpp index 376852341..97f67d707 100644 --- a/app/test/testutilsfunctions.cpp +++ b/app/test/testutilsfunctions.cpp @@ -331,6 +331,7 @@ void TestUtilsFunctions::resolveTargetDir() { QString homePath = TestUtils::testDataDir(); QString DEFAULT_ROOT( "DEFAULT/ROOT/PATH" ); // can be not existing path + QString ABSOLUTE_DEFAULT_ROOT( "/absolute/default/root/path" ); // can be not existing path QgsProject *activeProject = nullptr; QVariantMap config; @@ -344,10 +345,16 @@ void TestUtilsFunctions::resolveTargetDir() QString resultDir = mUtils->resolveTargetDir( homePath, config, pair, FeatureLayerPair(), activeProject ); QCOMPARE( resultDir, homePath ); - // case 2: defined default root config, no expression + // case 2: defined default root config as a relative path, no expression - resolved against homePath config.insert( QStringLiteral( "DefaultRoot" ), DEFAULT_ROOT ); QString resultDir2 = mUtils->resolveTargetDir( homePath, config, pair, FeatureLayerPair(), activeProject ); - QCOMPARE( resultDir2, DEFAULT_ROOT ); + QCOMPARE( resultDir2, QStringLiteral( "%1/%2" ).arg( homePath, DEFAULT_ROOT ) ); + config.clear(); + + // case 2b: defined default root config as an already-absolute path, no expression - stays unchanged + config.insert( QStringLiteral( "DefaultRoot" ), ABSOLUTE_DEFAULT_ROOT ); + QString resultDir2b = mUtils->resolveTargetDir( homePath, config, pair, FeatureLayerPair(), activeProject ); + QCOMPARE( resultDir2b, ABSOLUTE_DEFAULT_ROOT ); config.clear(); // case 3: defined expression in config->"PropertyCollection" -> "properties" -> "propertyRootPath" -> "expression"