diff --git a/src/akonadi/akonadinoterepository.h b/src/akonadi/akonadinoterepository.h --- a/src/akonadi/akonadinoterepository.h +++ b/src/akonadi/akonadinoterepository.h @@ -45,7 +45,8 @@ bool isDefaultSource(Domain::DataSource::Ptr source) const Q_DECL_OVERRIDE; void setDefaultSource(Domain::DataSource::Ptr source) Q_DECL_OVERRIDE; - KJob *save(Domain::Note::Ptr note) Q_DECL_OVERRIDE; + KJob *create(Domain::Note::Ptr note) Q_DECL_OVERRIDE; + KJob *update(Domain::Note::Ptr note) Q_DECL_OVERRIDE; KJob *remove(Domain::Note::Ptr note) Q_DECL_OVERRIDE; private: diff --git a/src/akonadi/akonadinoterepository.cpp b/src/akonadi/akonadinoterepository.cpp --- a/src/akonadi/akonadinoterepository.cpp +++ b/src/akonadi/akonadinoterepository.cpp @@ -26,10 +26,14 @@ #include +#include "akonadicollectionfetchjobinterface.h" #include "akonadiitemfetchjobinterface.h" #include "akonadistoragesettings.h" +#include "utils/compositejob.h" + using namespace Akonadi; +using namespace Utils; NoteRepository::NoteRepository(const StorageInterface::Ptr &storage, const SerializerInterface::Ptr &serializer) @@ -51,17 +55,45 @@ StorageSettings::instance().setDefaultNoteCollection(collection); } -KJob *NoteRepository::save(Domain::Note::Ptr note) +KJob *NoteRepository::create(Domain::Note::Ptr note) { auto item = m_serializer->createItemFromNote(note); + Q_ASSERT(!item.isValid()); - if (item.isValid()) { - return m_storage->updateItem(item); + const Akonadi::Collection defaultCollection = m_storage->defaultNoteCollection(); + if (defaultCollection.isValid()) { + return m_storage->createItem(item, defaultCollection); } else { - return m_storage->createItem(item, m_storage->defaultNoteCollection()); + auto job = new CompositeJob(); + CollectionFetchJobInterface *fetchCollectionJob = m_storage->fetchCollections(Akonadi::Collection::root(), + StorageInterface::Recursive, + StorageInterface::Notes); + job->install(fetchCollectionJob->kjob(), [fetchCollectionJob, item, job, this] { + if (fetchCollectionJob->kjob()->error() != KJob::NoError) + return; + + Q_ASSERT(fetchCollectionJob->collections().size() > 0); + const Akonadi::Collection::List collections = fetchCollectionJob->collections(); + Akonadi::Collection col = *std::find_if(collections.constBegin(), collections.constEnd(), + [] (const Akonadi::Collection &c) { + return c.rights() == Akonadi::Collection::AllRights; + }); + Q_ASSERT(col.isValid()); + auto createJob = m_storage->createItem(item, col); + job->addSubjob(createJob); + createJob->start(); + }); + return job; } } +KJob *NoteRepository::update(Domain::Note::Ptr note) +{ + auto item = m_serializer->createItemFromNote(note); + Q_ASSERT(item.isValid()); + return m_storage->updateItem(item); +} + KJob *NoteRepository::remove(Domain::Note::Ptr note) { auto item = m_serializer->createItemFromNote(note); diff --git a/src/domain/noterepository.h b/src/domain/noterepository.h --- a/src/domain/noterepository.h +++ b/src/domain/noterepository.h @@ -42,7 +42,8 @@ virtual bool isDefaultSource(DataSource::Ptr source) const = 0; virtual void setDefaultSource(DataSource::Ptr source) = 0; - virtual KJob *save(Note::Ptr note) = 0; + virtual KJob *create(Note::Ptr task) = 0; + virtual KJob *update(Note::Ptr note) = 0; virtual KJob *remove(Note::Ptr note) = 0; }; diff --git a/src/presentation/artifacteditormodel.cpp b/src/presentation/artifacteditormodel.cpp --- a/src/presentation/artifacteditormodel.cpp +++ b/src/presentation/artifacteditormodel.cpp @@ -256,7 +256,7 @@ } else { auto note = m_artifact.objectCast(); Q_ASSERT(note); - const auto job = m_noteRepository->save(note); + const auto job = m_noteRepository->update(note); installHandler(job, tr("Cannot modify note %1").arg(currentTitle)); } diff --git a/src/presentation/inboxpagemodel.cpp b/src/presentation/inboxpagemodel.cpp --- a/src/presentation/inboxpagemodel.cpp +++ b/src/presentation/inboxpagemodel.cpp @@ -127,7 +127,7 @@ const auto currentTitle = note->title(); note->setTitle(value.toString()); - const auto job = m_noteRepository->save(note); + const auto job = m_noteRepository->update(note); installHandler(job, tr("Cannot modify note %1 in Inbox").arg(currentTitle)); return true; diff --git a/src/presentation/projectpagemodel.cpp b/src/presentation/projectpagemodel.cpp --- a/src/presentation/projectpagemodel.cpp +++ b/src/presentation/projectpagemodel.cpp @@ -134,7 +134,7 @@ const auto currentTitle = note->title(); note->setTitle(value.toString()); - const auto job = m_noteRepository->save(note); + const auto job = m_noteRepository->update(note); installHandler(job, tr("Cannot modify note %1 in project %2").arg(currentTitle).arg(m_project->name())); return true; diff --git a/src/presentation/tagpagemodel.cpp b/src/presentation/tagpagemodel.cpp --- a/src/presentation/tagpagemodel.cpp +++ b/src/presentation/tagpagemodel.cpp @@ -144,7 +144,7 @@ const auto currentTitle = note->title(); note->setTitle(value.toString()); - const auto job = m_noteRepository->save(note); + const auto job = m_noteRepository->update(note); installHandler(job, tr("Cannot modify note %1 in tag %2").arg(currentTitle).arg(m_tag->name())); return true; diff --git a/tests/units/akonadi/akonadinoterepositorytest.cpp b/tests/units/akonadi/akonadinoterepositorytest.cpp --- a/tests/units/akonadi/akonadinoterepositorytest.cpp +++ b/tests/units/akonadi/akonadinoterepositorytest.cpp @@ -133,16 +133,16 @@ } - void shouldCreateNewItemsOnSave() + void shouldCreateNewItems() { // GIVEN // A default collection for saving - Akonadi::Collection col(42); + auto col = Akonadi::Collection(42); // A note and its corresponding item not existing in storage yet - Akonadi::Item item; - Domain::Note::Ptr note(new Domain::Note); + auto item = Akonadi::Item(); + auto note = Domain::Note::Ptr::create(); // A mock create job auto itemCreateJob = new FakeJob(this); @@ -160,24 +160,68 @@ // WHEN QScopedPointer repository(new Akonadi::NoteRepository(storageMock.getInstance(), serializerMock.getInstance())); - repository->save(note)->exec(); + repository->create(note)->exec(); // THEN QVERIFY(serializerMock(&Akonadi::SerializerInterface::createItemFromNote).when(note).exactly(1)); QVERIFY(storageMock(&Akonadi::StorageInterface::defaultNoteCollection).when().exactly(1)); QVERIFY(storageMock(&Akonadi::StorageInterface::createItem).when(item, col).exactly(1)); } - void shouldUpdateExistingItemsOnSave() + void shouldCreateNewItemsInFirstWritableCollectionIfNothingInSettings() + { + // GIVEN + + // A few collections + auto col1 = Akonadi::Collection(42); + col1.setRights(Akonadi::Collection::ReadOnly); + auto col2 = Akonadi::Collection(43); + auto col3 = Akonadi::Collection(44); + auto collectionFetchJob = new Testlib::AkonadiFakeCollectionFetchJob; + collectionFetchJob->setCollections(Akonadi::Collection::List() << col1 << col2 << col3); + + // A note and its corresponding item not existing in storage yet + auto item = Akonadi::Item(); + auto note = Domain::Note::Ptr::create(); + + // A mock create job + auto itemCreateJob = new FakeJob(this); + + // Storage mock returning the create job and with no default collection + Utils::MockObject storageMock; + storageMock(&Akonadi::StorageInterface::defaultNoteCollection).when().thenReturn(Akonadi::Collection()); + storageMock(&Akonadi::StorageInterface::fetchCollections).when(Akonadi::Collection::root(), + Akonadi::StorageInterface::Recursive, + Akonadi::StorageInterface::Notes) + .thenReturn(collectionFetchJob); + storageMock(&Akonadi::StorageInterface::createItem).when(item, col2) + .thenReturn(itemCreateJob); + + // Serializer mock returning the item for the note + Utils::MockObject serializerMock; + serializerMock(&Akonadi::SerializerInterface::createItemFromNote).when(note).thenReturn(item); + + // WHEN + QScopedPointer repository(new Akonadi::NoteRepository(storageMock.getInstance(), + serializerMock.getInstance())); + repository->create(note)->exec(); + + // THEN + QVERIFY(serializerMock(&Akonadi::SerializerInterface::createItemFromNote).when(note).exactly(1)); + QVERIFY(storageMock(&Akonadi::StorageInterface::defaultNoteCollection).when().exactly(1)); + QVERIFY(storageMock(&Akonadi::StorageInterface::createItem).when(item, col2).exactly(1)); + } + + void shouldUpdateExistingItems() { // GIVEN // A default collection for saving - Akonadi::Collection col(42); + auto col = Akonadi::Collection(42); // A note and its corresponding item already existing in storage - Akonadi::Item item(42); - Domain::Note::Ptr note(new Domain::Note); + auto item = Akonadi::Item(42); + auto note = Domain::Note::Ptr::create(); // A mock create job auto itemModifyJob = new FakeJob(this); @@ -187,14 +231,14 @@ storageMock(&Akonadi::StorageInterface::updateItem).when(item, Q_NULLPTR) .thenReturn(itemModifyJob); - // Serializer mock returning the item for the note + // Serializer mock returning the item for the task Utils::MockObject serializerMock; serializerMock(&Akonadi::SerializerInterface::createItemFromNote).when(note).thenReturn(item); // WHEN QScopedPointer repository(new Akonadi::NoteRepository(storageMock.getInstance(), serializerMock.getInstance())); - repository->save(note)->exec(); + repository->update(note)->exec(); // THEN QVERIFY(serializerMock(&Akonadi::SerializerInterface::createItemFromNote).when(note).exactly(1)); diff --git a/tests/units/presentation/artifacteditormodeltest.cpp b/tests/units/presentation/artifacteditormodeltest.cpp --- a/tests/units/presentation/artifacteditormodeltest.cpp +++ b/tests/units/presentation/artifacteditormodeltest.cpp @@ -293,7 +293,7 @@ Utils::MockObject taskRepositoryMock; taskRepositoryMock(&Domain::TaskRepository::update).when(task).thenReturn(new FakeJob(this)); Utils::MockObject noteRepositoryMock; - noteRepositoryMock(&Domain::NoteRepository::save).when(note).thenReturn(new FakeJob(this)); + noteRepositoryMock(&Domain::NoteRepository::update).when(note).thenReturn(new FakeJob(this)); Presentation::ArtifactEditorModel model(taskRepositoryMock.getInstance(), noteRepositoryMock.getInstance()); @@ -309,7 +309,7 @@ QCOMPARE(model.property(propertyName), propertyValue); QVERIFY(artifact->property(propertyName) != propertyValue); QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(task).exactly(0)); - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(note).exactly(0)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(note).exactly(0)); // WHEN (apply after delay) QTest::qWait(model.autoSaveDelay() + 50); @@ -319,7 +319,7 @@ if (task) { QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(task).exactly(1)); } else { - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(note).exactly(1)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(note).exactly(1)); } } @@ -342,7 +342,7 @@ Utils::MockObject taskRepositoryMock; taskRepositoryMock(&Domain::TaskRepository::update).when(task).thenReturn(new FakeJob(this)); Utils::MockObject noteRepositoryMock; - noteRepositoryMock(&Domain::NoteRepository::save).when(note).thenReturn(new FakeJob(this)); + noteRepositoryMock(&Domain::NoteRepository::update).when(note).thenReturn(new FakeJob(this)); Presentation::ArtifactEditorModel model(taskRepositoryMock.getInstance(), noteRepositoryMock.getInstance()); @@ -358,7 +358,7 @@ QCOMPARE(model.property(propertyName), propertyValue); QVERIFY(artifact->property(propertyName) != propertyValue); QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(task).exactly(0)); - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(note).exactly(0)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(note).exactly(0)); // WHEN (apply immediately) model.setArtifact(Domain::Task::Ptr::create()); @@ -368,7 +368,7 @@ if (task) { QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(task).exactly(1)); } else { - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(note).exactly(1)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(note).exactly(1)); } // WHEN (nothing else happens after a delay) @@ -379,7 +379,7 @@ if (task) { QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(task).exactly(1)); } else { - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(note).exactly(1)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(note).exactly(1)); } } @@ -402,7 +402,7 @@ Utils::MockObject taskRepositoryMock; taskRepositoryMock(&Domain::TaskRepository::update).when(task).thenReturn(new FakeJob(this)); Utils::MockObject noteRepositoryMock; - noteRepositoryMock(&Domain::NoteRepository::save).when(note).thenReturn(new FakeJob(this)); + noteRepositoryMock(&Domain::NoteRepository::update).when(note).thenReturn(new FakeJob(this)); auto model = new Presentation::ArtifactEditorModel(taskRepositoryMock.getInstance(), noteRepositoryMock.getInstance()); @@ -418,7 +418,7 @@ QCOMPARE(model->property(propertyName), propertyValue); QVERIFY(artifact->property(propertyName) != propertyValue); QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(task).exactly(0)); - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(note).exactly(0)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(note).exactly(0)); // WHEN (apply immediately) delete model; @@ -428,7 +428,7 @@ if (task) { QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(task).exactly(1)); } else { - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(note).exactly(1)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(note).exactly(1)); } } @@ -488,7 +488,7 @@ job->setExpectedError(KJob::KilledJobError, "Foo"); Utils::MockObject taskRepositoryMock; Utils::MockObject noteRepositoryMock; - noteRepositoryMock(&Domain::NoteRepository::save).when(note).thenReturn(job); + noteRepositoryMock(&Domain::NoteRepository::update).when(note).thenReturn(job); auto model = new Presentation::ArtifactEditorModel(taskRepositoryMock.getInstance(), noteRepositoryMock.getInstance()); diff --git a/tests/units/presentation/inboxpagemodeltest.cpp b/tests/units/presentation/inboxpagemodeltest.cpp --- a/tests/units/presentation/inboxpagemodeltest.cpp +++ b/tests/units/presentation/inboxpagemodeltest.cpp @@ -128,7 +128,7 @@ // WHEN taskRepositoryMock(&Domain::TaskRepository::update).when(rootTask).thenReturn(new FakeJob(this)); taskRepositoryMock(&Domain::TaskRepository::update).when(childTask).thenReturn(new FakeJob(this)); - noteRepositoryMock(&Domain::NoteRepository::save).when(rootNote).thenReturn(new FakeJob(this)); + noteRepositoryMock(&Domain::NoteRepository::update).when(rootNote).thenReturn(new FakeJob(this)); QVERIFY(model->setData(rootTaskIndex, "newRootTask")); QVERIFY(model->setData(rootNoteIndex, "newRootNote")); @@ -141,7 +141,7 @@ // THEN QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(rootTask).exactly(2)); QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(childTask).exactly(2)); - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(rootNote).exactly(1)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(rootNote).exactly(1)); QCOMPARE(rootTask->title(), QString("newRootTask")); QCOMPARE(rootNote->title(), QString("newRootNote")); @@ -456,7 +456,7 @@ // WHEN auto job = new FakeJob(this); job->setExpectedError(KJob::KilledJobError, "Foo"); - noteRepositoryMock(&Domain::NoteRepository::save).when(rootNote).thenReturn(job); + noteRepositoryMock(&Domain::NoteRepository::update).when(rootNote).thenReturn(job); QVERIFY(model->setData(rootNoteIndex, "newRootNote")); diff --git a/tests/units/presentation/projectpagemodeltest.cpp b/tests/units/presentation/projectpagemodeltest.cpp --- a/tests/units/presentation/projectpagemodeltest.cpp +++ b/tests/units/presentation/projectpagemodeltest.cpp @@ -135,7 +135,7 @@ // WHEN taskRepositoryMock(&Domain::TaskRepository::update).when(rootTask).thenReturn(new FakeJob(this)); taskRepositoryMock(&Domain::TaskRepository::update).when(childTask).thenReturn(new FakeJob(this)); - noteRepositoryMock(&Domain::NoteRepository::save).when(rootNote).thenReturn(new FakeJob(this)); + noteRepositoryMock(&Domain::NoteRepository::update).when(rootNote).thenReturn(new FakeJob(this)); QVERIFY(model->setData(rootTaskIndex, "newRootTask")); QVERIFY(model->setData(rootNoteIndex, "newRootNote")); @@ -148,7 +148,7 @@ // THEN QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(rootTask).exactly(2)); QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(childTask).exactly(2)); - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(rootNote).exactly(1)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(rootNote).exactly(1)); QCOMPARE(rootTask->title(), QString("newRootTask")); QCOMPARE(rootNote->title(), QString("newRootNote")); @@ -500,7 +500,7 @@ // WHEN auto job = new FakeJob(this); job->setExpectedError(KJob::KilledJobError, "Foo"); - noteRepositoryMock(&Domain::NoteRepository::save).when(rootNote).thenReturn(job); + noteRepositoryMock(&Domain::NoteRepository::update).when(rootNote).thenReturn(job); QVERIFY(model->setData(rootNoteIndex, "newRootNote")); diff --git a/tests/units/presentation/tagpagemodeltest.cpp b/tests/units/presentation/tagpagemodeltest.cpp --- a/tests/units/presentation/tagpagemodeltest.cpp +++ b/tests/units/presentation/tagpagemodeltest.cpp @@ -140,7 +140,7 @@ // WHEN taskRepositoryMock(&Domain::TaskRepository::update).when(rootTask).thenReturn(new FakeJob(this)); taskRepositoryMock(&Domain::TaskRepository::update).when(childTask).thenReturn(new FakeJob(this)); - noteRepositoryMock(&Domain::NoteRepository::save).when(rootNote).thenReturn(new FakeJob(this)); + noteRepositoryMock(&Domain::NoteRepository::update).when(rootNote).thenReturn(new FakeJob(this)); QVERIFY(model->setData(rootTaskIndex, "newRootTask")); QVERIFY(model->setData(rootNoteIndex, "newRootNote")); @@ -153,7 +153,7 @@ // THEN QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(rootTask).exactly(2)); QVERIFY(taskRepositoryMock(&Domain::TaskRepository::update).when(childTask).exactly(2)); - QVERIFY(noteRepositoryMock(&Domain::NoteRepository::save).when(rootNote).exactly(1)); + QVERIFY(noteRepositoryMock(&Domain::NoteRepository::update).when(rootNote).exactly(1)); QCOMPARE(rootTask->title(), QString("newRootTask")); QCOMPARE(rootNote->title(), QString("newRootNote")); @@ -503,7 +503,7 @@ // WHEN auto job = new FakeJob(this); job->setExpectedError(KJob::KilledJobError, "Foo"); - noteRepositoryMock(&Domain::NoteRepository::save).when(rootNote).thenReturn(job); + noteRepositoryMock(&Domain::NoteRepository::update).when(rootNote).thenReturn(job); QVERIFY(model->setData(rootNoteIndex, "newRootNote"));