From d250088842492b722c566464b0af9e28d8ba45d6 Mon Sep 17 00:00:00 2001 From: Richard Kello Date: Sat, 8 Aug 2026 10:10:49 +0200 Subject: [PATCH 1/6] Add setter to avoid mismatch in values --- app/attributes/attributecontroller.cpp | 3 +++ 1 file changed, 3 insertions(+) diff --git a/app/attributes/attributecontroller.cpp b/app/attributes/attributecontroller.cpp index 6c071485a..702287ae5 100644 --- a/app/attributes/attributecontroller.cpp +++ b/app/attributes/attributecontroller.cpp @@ -624,6 +624,9 @@ void AttributeController::updateOnFeatureChange() { mFeatureLayerPair.featureRef().setAttribute( fieldIndex, rememberedValue ); itemData->setRawValue( rememberedValue ); + // keep originalValue in sync so change-detection (e.g. renamePhotos()) doesn't + // mistake a reused value for a freshly captured one + itemData->setOriginalValue( rememberedValue ); } } } From 779c4a1aaab8057c6190eb5004de64c8ead40bb3 Mon Sep 17 00:00:00 2001 From: Richard Kello Date: Tue, 25 Aug 2026 14:32:25 +0200 Subject: [PATCH 2/6] Fix broken previews --- .../src/uk/co/lutraconsulting/MMActivity.java | 1 + app/attributes/attributecontroller.cpp | 81 +++++++++++++++++-- app/attributes/attributecontroller.h | 1 + app/attributes/attributedata.cpp | 20 +++++ app/attributes/attributedata.h | 8 ++ app/inpututils.cpp | 4 + app/ios/iosviewdelegate.mm | 2 + 7 files changed, 109 insertions(+), 8 deletions(-) diff --git a/app/android/src/uk/co/lutraconsulting/MMActivity.java b/app/android/src/uk/co/lutraconsulting/MMActivity.java index cb26be66a..988530b2d 100644 --- a/app/android/src/uk/co/lutraconsulting/MMActivity.java +++ b/app/android/src/uk/co/lutraconsulting/MMActivity.java @@ -201,6 +201,7 @@ public String importImage(Uri imageUri, String targetPath) { String fileName = getFileName( imageUri ); File newCopyFile = new File( targetPath + "/" + fileName ); try { + newCopyFile.getParentFile().mkdirs(); newCopyFile.createNewFile(); InputStream fileStream = getContentResolver().openInputStream( imageUri ); copyFile( fileStream, newCopyFile ); diff --git a/app/attributes/attributecontroller.cpp b/app/attributes/attributecontroller.cpp index 702287ae5..2acebbe4c 100644 --- a/app/attributes/attributecontroller.cpp +++ b/app/attributes/attributecontroller.cpp @@ -612,7 +612,8 @@ void AttributeController::updateOnFeatureChange() const QVariant newVal = feature.attribute( fieldIndex ); mFormItems[itemData->id()]->setOriginalValue( newVal ); mFormItems[itemData->id()]->setRawValue( newVal ); // we need to set raw value as well, as we use it in form now - if ( mRememberAttributesController && isNewFeature() ) // this is a new feature + itemData->setReusedValue( false ); + if ( mRememberAttributesController && isNewFeature() && newVal.toString().isEmpty() ) { QVariant rememberedValue; bool shouldUseRememberedValue = mRememberAttributesController->rememberedValue( @@ -622,11 +623,35 @@ void AttributeController::updateOnFeatureChange() ); if ( shouldUseRememberedValue ) { - mFeatureLayerPair.featureRef().setAttribute( fieldIndex, rememberedValue ); - itemData->setRawValue( rememberedValue ); - // keep originalValue in sync so change-detection (e.g. renamePhotos()) doesn't - // mistake a reused value for a freshly captured one - itemData->setOriginalValue( rememberedValue ); + QVariant valueToUse = rememberedValue; + + if ( itemData->editorWidgetType() == QStringLiteral( "ExternalResource" ) && !rememberedValue.toString().isEmpty() ) + { + const QVariantMap config = itemData->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 ); + const QString src = InputUtils::getAbsolutePath( rememberedValue.toString(), prefix ); + const QFileInfo fi( src ); + + static const QRegularExpression trailingCounter( QStringLiteral( "\\s\\(\\d+\\)$" ) ); + QString baseName = fi.completeBaseName(); + baseName.remove( trailingCounter ); + const QString canonicalName = fi.suffix().isEmpty() ? baseName : QStringLiteral( "%1.%2" ).arg( baseName, fi.suffix() ); + + const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( canonicalName, targetDir ) ); + + if ( InputUtils::copyFile( src, dst ) ) + { + valueToUse = InputUtils::getRelativePath( dst, prefix ); + itemData->setReusedCopyPath( dst ); + } + } + + mFeatureLayerPair.featureRef().setAttribute( fieldIndex, valueToUse ); + itemData->setRawValue( valueToUse ); + itemData->setOriginalValue( valueToUse ); + itemData->setReusedValue( true ); } } } @@ -794,7 +819,8 @@ void AttributeController::recalculateDefaultValues( bool shouldApplyDefaultValue = !defaultDefinition.expression().isEmpty() && - ( isFirstUpdateOfNewFeature || ( isFormValueChange && defaultDefinition.applyOnUpdate() ) ); + ( isFirstUpdateOfNewFeature || ( isFormValueChange && defaultDefinition.applyOnUpdate() ) ) && + !item->isReusedValue(); if ( shouldApplyDefaultValue ) { @@ -1214,6 +1240,8 @@ bool AttributeController::deleteFeature() bool AttributeController::rollback() { + discardReusedPhotoCopies( true ); + if ( !mFeatureLayerPair.layer() ) return false; @@ -1296,6 +1324,11 @@ bool AttributeController::save() disconnect( mFeatureLayerPair.layer(), &QgsVectorLayer::featureAdded, this, &AttributeController::onFeatureAdded ); } + if ( rv ) + { + discardReusedPhotoCopies( false ); + } + // Store the feature attributes for future use if ( featureIsNew && mRememberAttributesController ) { @@ -1500,6 +1533,7 @@ bool AttributeController::setFormValue( const QUuid &id, QVariant value ) QgsField field = item->field(); QVariant val( value ); + item->setReusedValue( false ); item->setRawValue( val ); emit formDataChanged( item->id(), { AttributeFormModel::RawValue } ); @@ -1579,6 +1613,36 @@ void AttributeController::onFeatureAdded( QgsFeatureId newFeatureId ) emit featureIdChanged(); } +void AttributeController::discardReusedPhotoCopies( bool force ) +{ + QMap>::const_iterator formItemsIterator = mFormItems.constBegin(); + while ( formItemsIterator != mFormItems.constEnd() ) + { + std::shared_ptr item = formItemsIterator.value(); + const QString copyPath = item->reusedCopyPath(); + if ( !copyPath.isEmpty() ) + { + bool stillReferenced = false; + if ( !force ) + { + 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 ); + const QString currentPath = InputUtils::getAbsolutePath( mFeatureLayerPair.feature().attribute( item->fieldIndex() ).toString(), prefix ); + stillReferenced = ( currentPath == copyPath ); + } + + if ( force || !stillReferenced ) + { + InputUtils::removeFile( copyPath ); + } + item->setReusedCopyPath( QString() ); + } + ++formItemsIterator; + } +} + void AttributeController::renamePhotos() { const QStringList photoNameFormat = QgsProject::instance()->entryList( QStringLiteral( "Mergin" ), QStringLiteral( "PhotoNaming/%1" ).arg( mFeatureLayerPair.layer()->id() ) ); @@ -1611,7 +1675,7 @@ void AttributeController::renamePhotos() continue; } - if ( item->originalValue() != mFeatureLayerPair.feature().attribute( item->fieldIndex() ) ) + if ( item->isReusedValue() || item->originalValue() != mFeatureLayerPair.feature().attribute( item->fieldIndex() ) ) { const QString expString = QgsProject::instance()->readEntry( QStringLiteral( "Mergin" ), QStringLiteral( "PhotoNaming/%1/%2" ).arg( mFeatureLayerPair.layer()->id(), field.name() ) ); QgsExpression exp( expString ); @@ -1659,6 +1723,7 @@ void AttributeController::renamePhotos() { const QString newValue = InputUtils::getRelativePath( dst, prefix ); setFormValue( item->id(), newValue ); + item->setReusedCopyPath( QString() ); expressionContext.setFeature( featureLayerPair().featureRef() ); } else diff --git a/app/attributes/attributecontroller.h b/app/attributes/attributecontroller.h index 59535feb5..e219febe6 100644 --- a/app/attributes/attributecontroller.h +++ b/app/attributes/attributecontroller.h @@ -225,6 +225,7 @@ class AttributeController : public QObject */ bool allowTabs( QgsAttributeEditorContainer *container ); + void discardReusedPhotoCopies( bool force ); //! renames photos if necessary void renamePhotos(); //! save temporary sketched image to original image diff --git a/app/attributes/attributedata.cpp b/app/attributes/attributedata.cpp index a9e761cda..c19124789 100644 --- a/app/attributes/attributedata.cpp +++ b/app/attributes/attributedata.cpp @@ -300,6 +300,26 @@ void FormItem::setOriginalValue( const QVariant &originalValue ) mOriginalValue = originalValue; } +bool FormItem::isReusedValue() const +{ + return mIsReusedValue; +} + +void FormItem::setReusedValue( bool reused ) +{ + mIsReusedValue = reused; +} + +QString FormItem::reusedCopyPath() const +{ + return mReusedCopyPath; +} + +void FormItem::setReusedCopyPath( const QString &path ) +{ + mReusedCopyPath = path; +} + QgsRelation FormItem::relation() const { return mRelation; diff --git a/app/attributes/attributedata.h b/app/attributes/attributedata.h index 1ecdf130f..d40d07725 100644 --- a/app/attributes/attributedata.h +++ b/app/attributes/attributedata.h @@ -152,6 +152,12 @@ class FormItem QVariant rawValue() const; void setRawValue( const QVariant &rawValue ); + bool isReusedValue() const; + void setReusedValue( bool reused ); + + QString reusedCopyPath() const; + void setReusedCopyPath( const QString &path ); + QgsRelation relation() const; QString fieldError() const; @@ -178,6 +184,8 @@ class FormItem bool mVisible = false; QVariant mOriginalValue; // original unmodified value QVariant mRawValue; + bool mIsReusedValue = false; + QString mReusedCopyPath; const QgsRelation mRelation; // Only used for FormItemType::Relation }; 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/ios/iosviewdelegate.mm b/app/ios/iosviewdelegate.mm index 86d5e243f..3b932df10 100644 --- a/app/ios/iosviewdelegate.mm +++ b/app/ios/iosviewdelegate.mm @@ -94,6 +94,8 @@ - ( void )picker:( PHPickerViewController * )picker didFinishPicking:( NSArray

Date: Tue, 25 Aug 2026 15:09:02 +0200 Subject: [PATCH 3/6] Update and add tests --- app/test/testutilsfunctions.cpp | 11 +++++++++-- 1 file changed, 9 insertions(+), 2 deletions(-) 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" From c6c8810300f371350e61b30ee13531f246bf564d Mon Sep 17 00:00:00 2001 From: Richard Kello Date: Tue, 8 Sep 2026 13:48:15 +0200 Subject: [PATCH 4/6] Add tests, fix wrong photo renaming --- app/attributes/attributecontroller.cpp | 34 +- app/test/testattributecontroller.cpp | 452 +++++++++++++++++++++++++ app/test/testattributecontroller.h | 25 ++ core/coreutils.cpp | 6 +- core/coreutils.h | 5 +- 5 files changed, 501 insertions(+), 21 deletions(-) diff --git a/app/attributes/attributecontroller.cpp b/app/attributes/attributecontroller.cpp index 2acebbe4c..fb6a8eda3 100644 --- a/app/attributes/attributecontroller.cpp +++ b/app/attributes/attributecontroller.cpp @@ -612,7 +612,7 @@ void AttributeController::updateOnFeatureChange() const QVariant newVal = feature.attribute( fieldIndex ); mFormItems[itemData->id()]->setOriginalValue( newVal ); mFormItems[itemData->id()]->setRawValue( newVal ); // we need to set raw value as well, as we use it in form now - itemData->setReusedValue( false ); + mFormItems[itemData->id()]->setReusedValue( false ); if ( mRememberAttributesController && isNewFeature() && newVal.toString().isEmpty() ) { QVariant rememberedValue; @@ -639,7 +639,7 @@ void AttributeController::updateOnFeatureChange() baseName.remove( trailingCounter ); const QString canonicalName = fi.suffix().isEmpty() ? baseName : QStringLiteral( "%1.%2" ).arg( baseName, fi.suffix() ); - const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( canonicalName, targetDir ) ); + const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( canonicalName, targetDir ), true ); if ( InputUtils::copyFile( src, dst ) ) { @@ -651,7 +651,8 @@ void AttributeController::updateOnFeatureChange() mFeatureLayerPair.featureRef().setAttribute( fieldIndex, valueToUse ); itemData->setRawValue( valueToUse ); itemData->setOriginalValue( valueToUse ); - itemData->setReusedValue( true ); + // an empty value means there's nothing to reuse, so don't mark it as such + itemData->setReusedValue( !valueToUse.toString().isEmpty() ); } } } @@ -817,6 +818,7 @@ void AttributeController::recalculateDefaultValues( const QgsField field = item->field(); const QgsDefaultValue defaultDefinition = field.defaultValueDefinition(); + // don't let a Default Value expression overwrite a value we just reused bool shouldApplyDefaultValue = !defaultDefinition.expression().isEmpty() && ( isFirstUpdateOfNewFeature || ( isFormValueChange && defaultDefinition.applyOnUpdate() ) ) && @@ -1326,6 +1328,7 @@ bool AttributeController::save() if ( rv ) { + // catches a reused copy that got deleted/replaced before save discardReusedPhotoCopies( false ); } @@ -1622,18 +1625,9 @@ void AttributeController::discardReusedPhotoCopies( bool force ) const QString copyPath = item->reusedCopyPath(); if ( !copyPath.isEmpty() ) { - bool stillReferenced = false; - if ( !force ) - { - 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 ); - const QString currentPath = InputUtils::getAbsolutePath( mFeatureLayerPair.feature().attribute( item->fieldIndex() ).toString(), prefix ); - stillReferenced = ( currentPath == copyPath ); - } - - if ( force || !stillReferenced ) + // isReusedValue() is cleared as soon as the field changes, so it already tells + // whether the copy is still the field's current value + if ( force || !item->isReusedValue() ) { InputUtils::removeFile( copyPath ); } @@ -1716,14 +1710,18 @@ 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 ) ); + const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( newName, targetDir ), true ); if ( InputUtils::renameFile( src, dst ) ) { const QString newValue = InputUtils::getRelativePath( dst, prefix ); setFormValue( item->id(), newValue ); - item->setReusedCopyPath( QString() ); + // keep originalValue() in sync so this doesn't get renamed again on the next save + item->setOriginalValue( newValue ); + // only clear if we actually renamed the tracked clone, not a replacement photo + if ( src == item->reusedCopyPath() ) + item->setReusedCopyPath( QString() ); expressionContext.setFeature( featureLayerPair().featureRef() ); } else diff --git a/app/test/testattributecontroller.cpp b/app/test/testattributecontroller.cpp index 2145bb058..2c1ed87a2 100644 --- a/app/test/testattributecontroller.cpp +++ b/app/test/testattributecontroller.cpp @@ -19,6 +19,7 @@ #include "qgsapplication.h" #include "qgsvectorlayer.h" #include "qgsproject.h" +#include "qgsdefaultvalue.h" #include "attributecontroller.h" #include "attributetabproxymodel.h" @@ -894,6 +895,457 @@ void TestAttributeController::testPhotoRenaming() } } +void TestAttributeController::testPhotoRenamingCollisionWithDotInName() +{ + QString projectName = QStringLiteral( "testPhotoRenamingCollisionWithDotInName" ); + 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" ) ) ); + + // "photo" naming expression is 'image_' + "notes", so a dot in notes lands in the name too + const QString collidingPath = projectDir + QStringLiteral( "/image_my.notes.jpg" ); + QVERIFY( QFile::copy( projectDir + QStringLiteral( "/image1.jpg" ), collidingPath ) ); + QVERIFY( QFile::exists( collidingPath ) ); + + 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() ); + + QgsFeature feat( surveyLayer->fields() ); + FeatureLayerPair pair( feat, surveyLayer ); + + AttributeController controller; + controller.setFeatureLayerPair( pair ); + + const TabItem *tab = controller.tabItem( 0 ); + const QVector items = tab->formItems(); + + controller.setFormValue( items.at( 2 ), QStringLiteral( "my.notes" ) ); + controller.setFormValue( items.at( 3 ), QStringLiteral( "image1.jpg" ) ); + + controller.save(); + + const QgsFeature f = controller.featureLayerPair().feature(); + + QVERIFY( QFile::exists( collidingPath ) ); // untouched + QVERIFY( !QFile::exists( projectDir + QStringLiteral( "/image_my (1).notes.jpg" ) ) ); + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_my.notes (1).jpg" ) ) ); + QCOMPARE( f.attribute( 3 ), QStringLiteral( "image_my.notes (1).jpg" ) ); +} + +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::testDefaultValueDoesNotOverwriteReusedPhoto() +{ + QString projectName = QStringLiteral( "testDefaultValueDoesNotOverwriteReusedPhoto" ); + 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() ); + + // field-level Default Value, only applied on a brand new feature - same moment as reuse + surveyLayer->setDefaultValueDefinition( 3, QgsDefaultValue( QStringLiteral( "'should_not_apply.jpg'" ), false ) ); + + 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" ) ) ); + + // feature 2: photo is reused - the Default Value expression must not clobber it + QgsFeature feat2( surveyLayer->fields() ); + FeatureLayerPair pair2( feat2, surveyLayer ); + + AttributeController controller2; + controller2.setRememberAttributesController( &remController ); + controller2.setFeatureLayerPair( pair2 ); + + const TabItem *tab2 = controller2.tabItem( 0 ); + const FormItem *photoItem2 = controller2.formItem( tab2->formItems().at( 3 ) ); + + QVERIFY( photoItem2->isReusedValue() ); + QVERIFY( !photoItem2->rawValue().toString().isEmpty() ); + QVERIFY( photoItem2->rawValue().toString() != QStringLiteral( "should_not_apply.jpg" ) ); +} + +void TestAttributeController::testDefaultValueAppliesWhenNothingReused() +{ + QString projectName = QStringLiteral( "testDefaultValueAppliesWhenNothingReused" ); + 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() ); + + surveyLayer->setDefaultValueDefinition( 3, QgsDefaultValue( QStringLiteral( "'default_photo.jpg'" ), false ) ); + + 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" ) ); + // photo left empty - nothing to reuse for the next feature + controller1.save(); + + // feature 2: nothing was reused, so the Default Value expression must still apply + QgsFeature feat2( surveyLayer->fields() ); + FeatureLayerPair pair2( feat2, surveyLayer ); + + AttributeController controller2; + controller2.setRememberAttributesController( &remController ); + controller2.setFeatureLayerPair( pair2 ); + + const TabItem *tab2 = controller2.tabItem( 0 ); + const FormItem *photoItem2 = controller2.formItem( tab2->formItems().at( 3 ) ); + + QVERIFY( !photoItem2->isReusedValue() ); + QCOMPARE( photoItem2->rawValue().toString(), QStringLiteral( "default_photo.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::testDiscardReusedPhotoCopyOnRollback() +{ + QString projectName = QStringLiteral( "testDiscardReusedPhotoCopyOnRollback" ); + 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" ) ) ); + + // feature 2: photo gets reused (cloned), but the draft is discarded before it is ever saved + QgsFeature feat2( surveyLayer->fields() ); + FeatureLayerPair pair2( feat2, surveyLayer ); + + AttributeController controller2; + controller2.setRememberAttributesController( &remController ); + controller2.setFeatureLayerPair( pair2 ); + + const QString clonedRelativePath = controller2.featureLayerPair().feature().attribute( 3 ).toString(); + QVERIFY( !clonedRelativePath.isEmpty() ); + const QString clonedAbsolutePath = projectDir + "/" + clonedRelativePath; + QVERIFY( QFile::exists( clonedAbsolutePath ) ); + + controller2.rollback(); + + QVERIFY( !QFile::exists( clonedAbsolutePath ) ); + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); +} + +void TestAttributeController::testDiscardReusedPhotoCopyOnReplace() +{ + QString projectName = QStringLiteral( "testDiscardReusedPhotoCopyOnReplace" ); + 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( QFile::exists( projectDir + QStringLiteral( "/image2.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 ); + + 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" ) ) ); + + // feature 2: photo gets reused (cloned), then replaced with a different photo before saving + 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(); + + const QString clonedRelativePath = controller2.featureLayerPair().feature().attribute( 3 ).toString(); + QVERIFY( !clonedRelativePath.isEmpty() ); + const QString clonedAbsolutePath = projectDir + "/" + clonedRelativePath; + QVERIFY( QFile::exists( clonedAbsolutePath ) ); + + controller2.setFormValue( items2.at( 2 ), QStringLiteral( "replaced" ) ); + controller2.setFormValue( items2.at( 3 ), QStringLiteral( "image2.jpg" ) ); + controller2.save(); + + // the orphaned clone from the earlier reuse must be gone, feature 1's photo untouched + QVERIFY( !QFile::exists( clonedAbsolutePath ) ); + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); + QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_replaced.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..36f776a71 100644 --- a/app/test/testattributecontroller.h +++ b/app/test/testattributecontroller.h @@ -28,6 +28,31 @@ class TestAttributeController: public QObject void testRawValue(); void testFieldsOutsideForm(); void testPhotoRenaming(); + + //! A collision suffix must be inserted before the real extension, even if the name has a dot + void testPhotoRenamingCollisionWithDotInName(); + + //! A reused photo must be renamed with the new feature's own expression value, not the old one + void testPhotoReuseRenamesWithFreshExpressionValue(); + + //! A reused photo must not let a field-level Default Value expression overwrite it + void testDefaultValueDoesNotOverwriteReusedPhoto(); + + //! When there's nothing to reuse (empty value), the Default Value expression must still apply + void testDefaultValueAppliesWhenNothingReused(); + + //! Saving the same feature twice must not rename an already-renamed photo again + void testPhotoRenamingNotRepeatedOnResave(); + + //! Discarding a draft feature must delete the clone made for a reused photo + void testDiscardReusedPhotoCopyOnRollback(); + + //! Replacing a reused photo before saving must delete the now-orphaned clone + void testDiscardReusedPhotoCopyOnReplace(); + + //! Reusing a photo must create an independent file, not just copy the path string + void testReusedPhotoIsIndependentFile(); + void testHtmlAndTextWidgets(); void testVirtualFields(); diff --git a/core/coreutils.cpp b/core/coreutils.cpp index 143e1f81b..828423624 100644 --- a/core/coreutils.cpp +++ b/core/coreutils.cpp @@ -165,7 +165,7 @@ void CoreUtils::appendLog( const QByteArray &data, const QString &path ) } } -QString CoreUtils::findUniquePath( const QString &path ) +QString CoreUtils::findUniquePath( const QString &path, bool splitOnLastDot ) { QFileInfo originalPath( path ); QString uniquePath = path; @@ -186,7 +186,9 @@ QString CoreUtils::findUniquePath( const QString &path ) } else // file { - uniquePath = originalPath.path() + '/' + originalPath.baseName() + " (" + QString::number( i ) + ")." + originalPath.completeSuffix(); + const QString baseName = splitOnLastDot ? originalPath.completeBaseName() : originalPath.baseName(); + const QString suffix = splitOnLastDot ? originalPath.suffix() : originalPath.completeSuffix(); + uniquePath = originalPath.path() + '/' + baseName + " (" + QString::number( i ) + ")." + suffix; } f.setFile( uniquePath ); } diff --git a/core/coreutils.h b/core/coreutils.h index 990fbca45..26d2168a5 100644 --- a/core/coreutils.h +++ b/core/coreutils.h @@ -50,8 +50,11 @@ class CoreUtils * Returns given path if it does not exist yet, otherwise adds a number to the path in format: * - if path is a directory: "folder" -> "folder (1)" * - if path is a file: "filename.txt" -> "filename (1).txt" + * + * By default splits the name on the first dot, keeping compound extensions like ".tar.gz" + * intact. Pass splitOnLastDot TRUE for names with a dot but only a simple extension. */ - static QString findUniquePath( const QString &path ); + static QString findUniquePath( const QString &path, bool splitOnLastDot = false ); //! Creates a unique project directory for given project name (used for initial download of a project) static QString createUniqueProjectDirectory( const QString &baseDataDir, const QString &projectName ); From 99a76d61926ab1ad9b7a8a82091213d8a9b5ae95 Mon Sep 17 00:00:00 2001 From: Richard Kello Date: Fri, 18 Sep 2026 13:42:13 +0200 Subject: [PATCH 5/6] Refactor code --- .../src/uk/co/lutraconsulting/MMActivity.java | 1 - app/attributes/attributecontroller.cpp | 90 ++---- app/attributes/attributecontroller.h | 3 +- app/attributes/attributedata.cpp | 20 -- app/attributes/attributedata.h | 8 - app/ios/iosviewdelegate.mm | 2 - app/test/testattributecontroller.cpp | 275 ------------------ app/test/testattributecontroller.h | 15 - core/coreutils.cpp | 6 +- core/coreutils.h | 5 +- 10 files changed, 37 insertions(+), 388 deletions(-) diff --git a/app/android/src/uk/co/lutraconsulting/MMActivity.java b/app/android/src/uk/co/lutraconsulting/MMActivity.java index 988530b2d..cb26be66a 100644 --- a/app/android/src/uk/co/lutraconsulting/MMActivity.java +++ b/app/android/src/uk/co/lutraconsulting/MMActivity.java @@ -201,7 +201,6 @@ public String importImage(Uri imageUri, String targetPath) { String fileName = getFileName( imageUri ); File newCopyFile = new File( targetPath + "/" + fileName ); try { - newCopyFile.getParentFile().mkdirs(); newCopyFile.createNewFile(); InputStream fileStream = getContentResolver().openInputStream( imageUri ); copyFile( fileStream, newCopyFile ); diff --git a/app/attributes/attributecontroller.cpp b/app/attributes/attributecontroller.cpp index fb6a8eda3..df1d5f38d 100644 --- a/app/attributes/attributecontroller.cpp +++ b/app/attributes/attributecontroller.cpp @@ -18,6 +18,7 @@ #include "attributetabmodel.h" #include "fieldvalidator.h" +#include #include #include @@ -612,8 +613,7 @@ void AttributeController::updateOnFeatureChange() const QVariant newVal = feature.attribute( fieldIndex ); mFormItems[itemData->id()]->setOriginalValue( newVal ); mFormItems[itemData->id()]->setRawValue( newVal ); // we need to set raw value as well, as we use it in form now - mFormItems[itemData->id()]->setReusedValue( false ); - if ( mRememberAttributesController && isNewFeature() && newVal.toString().isEmpty() ) + if ( mRememberAttributesController && isNewFeature() ) // this is a new feature { QVariant rememberedValue; bool shouldUseRememberedValue = mRememberAttributesController->rememberedValue( @@ -628,31 +628,35 @@ void AttributeController::updateOnFeatureChange() if ( itemData->editorWidgetType() == QStringLiteral( "ExternalResource" ) && !rememberedValue.toString().isEmpty() ) { const QVariantMap config = itemData->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( rememberedValue.toString(), prefix ); const QFileInfo fi( src ); - static const QRegularExpression trailingCounter( QStringLiteral( "\\s\\(\\d+\\)$" ) ); - QString baseName = fi.completeBaseName(); - baseName.remove( trailingCounter ); - const QString canonicalName = fi.suffix().isEmpty() ? baseName : QStringLiteral( "%1.%2" ).arg( baseName, fi.suffix() ); + if ( !fi.isFile() ) + { + ++formItemsIterator; + continue; + } + + // 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( canonicalName, targetDir ), true ); + const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( newName, targetDir ) ); - if ( InputUtils::copyFile( src, dst ) ) + if ( !InputUtils::copyFile( src, dst ) ) { - valueToUse = InputUtils::getRelativePath( dst, prefix ); - itemData->setReusedCopyPath( dst ); + ++formItemsIterator; + continue; } + + valueToUse = InputUtils::getRelativePath( dst, prefix ); } mFeatureLayerPair.featureRef().setAttribute( fieldIndex, valueToUse ); itemData->setRawValue( valueToUse ); - itemData->setOriginalValue( valueToUse ); - // an empty value means there's nothing to reuse, so don't mark it as such - itemData->setReusedValue( !valueToUse.toString().isEmpty() ); } } } @@ -818,11 +822,9 @@ void AttributeController::recalculateDefaultValues( const QgsField field = item->field(); const QgsDefaultValue defaultDefinition = field.defaultValueDefinition(); - // don't let a Default Value expression overwrite a value we just reused bool shouldApplyDefaultValue = !defaultDefinition.expression().isEmpty() && - ( isFirstUpdateOfNewFeature || ( isFormValueChange && defaultDefinition.applyOnUpdate() ) ) && - !item->isReusedValue(); + ( isFirstUpdateOfNewFeature || ( isFormValueChange && defaultDefinition.applyOnUpdate() ) ); if ( shouldApplyDefaultValue ) { @@ -1242,8 +1244,6 @@ bool AttributeController::deleteFeature() bool AttributeController::rollback() { - discardReusedPhotoCopies( true ); - if ( !mFeatureLayerPair.layer() ) return false; @@ -1326,12 +1326,6 @@ bool AttributeController::save() disconnect( mFeatureLayerPair.layer(), &QgsVectorLayer::featureAdded, this, &AttributeController::onFeatureAdded ); } - if ( rv ) - { - // catches a reused copy that got deleted/replaced before save - discardReusedPhotoCopies( false ); - } - // Store the feature attributes for future use if ( featureIsNew && mRememberAttributesController ) { @@ -1536,7 +1530,6 @@ bool AttributeController::setFormValue( const QUuid &id, QVariant value ) QgsField field = item->field(); QVariant val( value ); - item->setReusedValue( false ); item->setRawValue( val ); emit formDataChanged( item->id(), { AttributeFormModel::RawValue } ); @@ -1616,25 +1609,11 @@ void AttributeController::onFeatureAdded( QgsFeatureId newFeatureId ) emit featureIdChanged(); } -void AttributeController::discardReusedPhotoCopies( bool force ) +void AttributeController::resolveExternalResourcePaths( const QVariantMap &config, QString &targetDir, QString &prefix ) const { - QMap>::const_iterator formItemsIterator = mFormItems.constBegin(); - while ( formItemsIterator != mFormItems.constEnd() ) - { - std::shared_ptr item = formItemsIterator.value(); - const QString copyPath = item->reusedCopyPath(); - if ( !copyPath.isEmpty() ) - { - // isReusedValue() is cleared as soon as the field changes, so it already tells - // whether the copy is still the field's current value - if ( force || !item->isReusedValue() ) - { - InputUtils::removeFile( copyPath ); - } - item->setReusedCopyPath( QString() ); - } - ++formItemsIterator; - } + 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 ); } void AttributeController::renamePhotos() @@ -1669,7 +1648,7 @@ void AttributeController::renamePhotos() continue; } - if ( item->isReusedValue() || item->originalValue() != mFeatureLayerPair.feature().attribute( item->fieldIndex() ) ) + if ( item->originalValue() != mFeatureLayerPair.feature().attribute( item->fieldIndex() ) ) { const QString expString = QgsProject::instance()->readEntry( QStringLiteral( "Mergin" ), QStringLiteral( "PhotoNaming/%1/%2" ).arg( mFeatureLayerPair.layer()->id(), field.name() ) ); QgsExpression exp( expString ); @@ -1697,9 +1676,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(); @@ -1712,16 +1690,13 @@ void AttributeController::renamePhotos() const QFileInfo fi( src ); newName = QStringLiteral( "%1.%2" ).arg( newName, fi.suffix() ); - const QString dst = CoreUtils::findUniquePath( InputUtils::getAbsolutePath( newName, targetDir ), true ); + 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 ); - // keep originalValue() in sync so this doesn't get renamed again on the next save + // avoids renaming it again on the next save item->setOriginalValue( newValue ); - // only clear if we actually renamed the tracked clone, not a replacement photo - if ( src == item->reusedCopyPath() ) - item->setReusedCopyPath( QString() ); expressionContext.setFeature( featureLayerPair().featureRef() ); } else @@ -1746,9 +1721,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 e219febe6..fb403048d 100644 --- a/app/attributes/attributecontroller.h +++ b/app/attributes/attributecontroller.h @@ -225,7 +225,8 @@ class AttributeController : public QObject */ bool allowTabs( QgsAttributeEditorContainer *container ); - void discardReusedPhotoCopies( bool force ); + //! resolves the storage folder and relative-path prefix for an ExternalResource field + void resolveExternalResourcePaths( const QVariantMap &config, QString &targetDir, QString &prefix ) const; //! renames photos if necessary void renamePhotos(); //! save temporary sketched image to original image diff --git a/app/attributes/attributedata.cpp b/app/attributes/attributedata.cpp index c19124789..a9e761cda 100644 --- a/app/attributes/attributedata.cpp +++ b/app/attributes/attributedata.cpp @@ -300,26 +300,6 @@ void FormItem::setOriginalValue( const QVariant &originalValue ) mOriginalValue = originalValue; } -bool FormItem::isReusedValue() const -{ - return mIsReusedValue; -} - -void FormItem::setReusedValue( bool reused ) -{ - mIsReusedValue = reused; -} - -QString FormItem::reusedCopyPath() const -{ - return mReusedCopyPath; -} - -void FormItem::setReusedCopyPath( const QString &path ) -{ - mReusedCopyPath = path; -} - QgsRelation FormItem::relation() const { return mRelation; diff --git a/app/attributes/attributedata.h b/app/attributes/attributedata.h index d40d07725..1ecdf130f 100644 --- a/app/attributes/attributedata.h +++ b/app/attributes/attributedata.h @@ -152,12 +152,6 @@ class FormItem QVariant rawValue() const; void setRawValue( const QVariant &rawValue ); - bool isReusedValue() const; - void setReusedValue( bool reused ); - - QString reusedCopyPath() const; - void setReusedCopyPath( const QString &path ); - QgsRelation relation() const; QString fieldError() const; @@ -184,8 +178,6 @@ class FormItem bool mVisible = false; QVariant mOriginalValue; // original unmodified value QVariant mRawValue; - bool mIsReusedValue = false; - QString mReusedCopyPath; const QgsRelation mRelation; // Only used for FormItemType::Relation }; diff --git a/app/ios/iosviewdelegate.mm b/app/ios/iosviewdelegate.mm index 3b932df10..86d5e243f 100644 --- a/app/ios/iosviewdelegate.mm +++ b/app/ios/iosviewdelegate.mm @@ -94,8 +94,6 @@ - ( void )picker:( PHPickerViewController * )picker didFinishPicking:( NSArray

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() ); - - QgsFeature feat( surveyLayer->fields() ); - FeatureLayerPair pair( feat, surveyLayer ); - - AttributeController controller; - controller.setFeatureLayerPair( pair ); - - const TabItem *tab = controller.tabItem( 0 ); - const QVector items = tab->formItems(); - - controller.setFormValue( items.at( 2 ), QStringLiteral( "my.notes" ) ); - controller.setFormValue( items.at( 3 ), QStringLiteral( "image1.jpg" ) ); - - controller.save(); - - const QgsFeature f = controller.featureLayerPair().feature(); - - QVERIFY( QFile::exists( collidingPath ) ); // untouched - QVERIFY( !QFile::exists( projectDir + QStringLiteral( "/image_my (1).notes.jpg" ) ) ); - QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_my.notes (1).jpg" ) ) ); - QCOMPARE( f.attribute( 3 ), QStringLiteral( "image_my.notes (1).jpg" ) ); -} - void TestAttributeController::testPhotoReuseRenamesWithFreshExpressionValue() { QString projectName = QStringLiteral( "testPhotoReuseRenamesWithFreshExpressionValue" ); @@ -1001,114 +956,6 @@ void TestAttributeController::testPhotoReuseRenamesWithFreshExpressionValue() QCOMPARE( f2.attribute( 3 ), QStringLiteral( "image_second.jpg" ) ); } -void TestAttributeController::testDefaultValueDoesNotOverwriteReusedPhoto() -{ - QString projectName = QStringLiteral( "testDefaultValueDoesNotOverwriteReusedPhoto" ); - 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() ); - - // field-level Default Value, only applied on a brand new feature - same moment as reuse - surveyLayer->setDefaultValueDefinition( 3, QgsDefaultValue( QStringLiteral( "'should_not_apply.jpg'" ), false ) ); - - 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" ) ) ); - - // feature 2: photo is reused - the Default Value expression must not clobber it - QgsFeature feat2( surveyLayer->fields() ); - FeatureLayerPair pair2( feat2, surveyLayer ); - - AttributeController controller2; - controller2.setRememberAttributesController( &remController ); - controller2.setFeatureLayerPair( pair2 ); - - const TabItem *tab2 = controller2.tabItem( 0 ); - const FormItem *photoItem2 = controller2.formItem( tab2->formItems().at( 3 ) ); - - QVERIFY( photoItem2->isReusedValue() ); - QVERIFY( !photoItem2->rawValue().toString().isEmpty() ); - QVERIFY( photoItem2->rawValue().toString() != QStringLiteral( "should_not_apply.jpg" ) ); -} - -void TestAttributeController::testDefaultValueAppliesWhenNothingReused() -{ - QString projectName = QStringLiteral( "testDefaultValueAppliesWhenNothingReused" ); - 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() ); - - surveyLayer->setDefaultValueDefinition( 3, QgsDefaultValue( QStringLiteral( "'default_photo.jpg'" ), false ) ); - - 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" ) ); - // photo left empty - nothing to reuse for the next feature - controller1.save(); - - // feature 2: nothing was reused, so the Default Value expression must still apply - QgsFeature feat2( surveyLayer->fields() ); - FeatureLayerPair pair2( feat2, surveyLayer ); - - AttributeController controller2; - controller2.setRememberAttributesController( &remController ); - controller2.setFeatureLayerPair( pair2 ); - - const TabItem *tab2 = controller2.tabItem( 0 ); - const FormItem *photoItem2 = controller2.formItem( tab2->formItems().at( 3 ) ); - - QVERIFY( !photoItem2->isReusedValue() ); - QCOMPARE( photoItem2->rawValue().toString(), QStringLiteral( "default_photo.jpg" ) ); -} - void TestAttributeController::testPhotoRenamingNotRepeatedOnResave() { QString projectName = QStringLiteral( "testPhotoRenamingNotRepeatedOnResave" ); @@ -1166,128 +1013,6 @@ void TestAttributeController::testPhotoRenamingNotRepeatedOnResave() QVERIFY( !QFile::exists( projectDir + QStringLiteral( "/image_second (1).jpg" ) ) ); } -void TestAttributeController::testDiscardReusedPhotoCopyOnRollback() -{ - QString projectName = QStringLiteral( "testDiscardReusedPhotoCopyOnRollback" ); - 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" ) ) ); - - // feature 2: photo gets reused (cloned), but the draft is discarded before it is ever saved - QgsFeature feat2( surveyLayer->fields() ); - FeatureLayerPair pair2( feat2, surveyLayer ); - - AttributeController controller2; - controller2.setRememberAttributesController( &remController ); - controller2.setFeatureLayerPair( pair2 ); - - const QString clonedRelativePath = controller2.featureLayerPair().feature().attribute( 3 ).toString(); - QVERIFY( !clonedRelativePath.isEmpty() ); - const QString clonedAbsolutePath = projectDir + "/" + clonedRelativePath; - QVERIFY( QFile::exists( clonedAbsolutePath ) ); - - controller2.rollback(); - - QVERIFY( !QFile::exists( clonedAbsolutePath ) ); - QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); -} - -void TestAttributeController::testDiscardReusedPhotoCopyOnReplace() -{ - QString projectName = QStringLiteral( "testDiscardReusedPhotoCopyOnReplace" ); - 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( QFile::exists( projectDir + QStringLiteral( "/image2.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 ); - - 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" ) ) ); - - // feature 2: photo gets reused (cloned), then replaced with a different photo before saving - 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(); - - const QString clonedRelativePath = controller2.featureLayerPair().feature().attribute( 3 ).toString(); - QVERIFY( !clonedRelativePath.isEmpty() ); - const QString clonedAbsolutePath = projectDir + "/" + clonedRelativePath; - QVERIFY( QFile::exists( clonedAbsolutePath ) ); - - controller2.setFormValue( items2.at( 2 ), QStringLiteral( "replaced" ) ); - controller2.setFormValue( items2.at( 3 ), QStringLiteral( "image2.jpg" ) ); - controller2.save(); - - // the orphaned clone from the earlier reuse must be gone, feature 1's photo untouched - QVERIFY( !QFile::exists( clonedAbsolutePath ) ); - QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_first.jpg" ) ) ); - QVERIFY( QFile::exists( projectDir + QStringLiteral( "/image_replaced.jpg" ) ) ); -} - void TestAttributeController::testReusedPhotoIsIndependentFile() { QString projectName = QStringLiteral( "testReusedPhotoIsIndependentFile" ); diff --git a/app/test/testattributecontroller.h b/app/test/testattributecontroller.h index 36f776a71..2b9bb7079 100644 --- a/app/test/testattributecontroller.h +++ b/app/test/testattributecontroller.h @@ -29,27 +29,12 @@ class TestAttributeController: public QObject void testFieldsOutsideForm(); void testPhotoRenaming(); - //! A collision suffix must be inserted before the real extension, even if the name has a dot - void testPhotoRenamingCollisionWithDotInName(); - //! A reused photo must be renamed with the new feature's own expression value, not the old one void testPhotoReuseRenamesWithFreshExpressionValue(); - //! A reused photo must not let a field-level Default Value expression overwrite it - void testDefaultValueDoesNotOverwriteReusedPhoto(); - - //! When there's nothing to reuse (empty value), the Default Value expression must still apply - void testDefaultValueAppliesWhenNothingReused(); - //! Saving the same feature twice must not rename an already-renamed photo again void testPhotoRenamingNotRepeatedOnResave(); - //! Discarding a draft feature must delete the clone made for a reused photo - void testDiscardReusedPhotoCopyOnRollback(); - - //! Replacing a reused photo before saving must delete the now-orphaned clone - void testDiscardReusedPhotoCopyOnReplace(); - //! Reusing a photo must create an independent file, not just copy the path string void testReusedPhotoIsIndependentFile(); diff --git a/core/coreutils.cpp b/core/coreutils.cpp index 828423624..143e1f81b 100644 --- a/core/coreutils.cpp +++ b/core/coreutils.cpp @@ -165,7 +165,7 @@ void CoreUtils::appendLog( const QByteArray &data, const QString &path ) } } -QString CoreUtils::findUniquePath( const QString &path, bool splitOnLastDot ) +QString CoreUtils::findUniquePath( const QString &path ) { QFileInfo originalPath( path ); QString uniquePath = path; @@ -186,9 +186,7 @@ QString CoreUtils::findUniquePath( const QString &path, bool splitOnLastDot ) } else // file { - const QString baseName = splitOnLastDot ? originalPath.completeBaseName() : originalPath.baseName(); - const QString suffix = splitOnLastDot ? originalPath.suffix() : originalPath.completeSuffix(); - uniquePath = originalPath.path() + '/' + baseName + " (" + QString::number( i ) + ")." + suffix; + uniquePath = originalPath.path() + '/' + originalPath.baseName() + " (" + QString::number( i ) + ")." + originalPath.completeSuffix(); } f.setFile( uniquePath ); } diff --git a/core/coreutils.h b/core/coreutils.h index 26d2168a5..990fbca45 100644 --- a/core/coreutils.h +++ b/core/coreutils.h @@ -50,11 +50,8 @@ class CoreUtils * Returns given path if it does not exist yet, otherwise adds a number to the path in format: * - if path is a directory: "folder" -> "folder (1)" * - if path is a file: "filename.txt" -> "filename (1).txt" - * - * By default splits the name on the first dot, keeping compound extensions like ".tar.gz" - * intact. Pass splitOnLastDot TRUE for names with a dot but only a simple extension. */ - static QString findUniquePath( const QString &path, bool splitOnLastDot = false ); + static QString findUniquePath( const QString &path ); //! Creates a unique project directory for given project name (used for initial download of a project) static QString createUniqueProjectDirectory( const QString &baseDataDir, const QString &projectName ); From 8a9edcf9be4187d5fececef7e3ab4a9dd1476a4d Mon Sep 17 00:00:00 2001 From: Richard Kello Date: Thu, 1 Oct 2026 13:01:12 +0200 Subject: [PATCH 6/6] Add logs and some code improwements --- app/attributes/attributecontroller.cpp | 55 +++++++++++++++----------- app/attributes/attributecontroller.h | 3 ++ 2 files changed, 36 insertions(+), 22 deletions(-) diff --git a/app/attributes/attributecontroller.cpp b/app/attributes/attributecontroller.cpp index df1d5f38d..ae29f39d4 100644 --- a/app/attributes/attributecontroller.cpp +++ b/app/attributes/attributecontroller.cpp @@ -627,32 +627,13 @@ void AttributeController::updateOnFeatureChange() if ( itemData->editorWidgetType() == QStringLiteral( "ExternalResource" ) && !rememberedValue.toString().isEmpty() ) { - const QVariantMap config = itemData->editorWidgetConfig(); - QString targetDir, prefix; - resolveExternalResourcePaths( config, targetDir, prefix ); - const QString src = InputUtils::getAbsolutePath( rememberedValue.toString(), prefix ); - const QFileInfo fi( src ); - - if ( !fi.isFile() ) - { - ++formItemsIterator; - continue; - } - - // 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 ) ) + QString clonedPath; + if ( !cloneExternalResource( *itemData, rememberedValue.toString(), clonedPath ) ) { ++formItemsIterator; continue; } - - valueToUse = InputUtils::getRelativePath( dst, prefix ); + valueToUse = clonedPath; } mFeatureLayerPair.featureRef().setAttribute( fieldIndex, valueToUse ); @@ -1616,6 +1597,36 @@ void AttributeController::resolveExternalResourcePaths( const QVariantMap &confi 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() ) ); diff --git a/app/attributes/attributecontroller.h b/app/attributes/attributecontroller.h index fb403048d..b05d7c9db 100644 --- a/app/attributes/attributecontroller.h +++ b/app/attributes/attributecontroller.h @@ -227,6 +227,9 @@ class AttributeController : public QObject //! 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