Skip to content
This repository has been archived by the owner on Oct 5, 2023. It is now read-only.

Minor fixes #404

Open
wants to merge 2 commits into
base: kraken
Choose a base branch
from
Open

Minor fixes #404

wants to merge 2 commits into from

Conversation

trema96
Copy link

@trema96 trema96 commented Jun 13, 2022

  • Fix InternalDAO get with rev
  • Make purge(Collection) reactive?

@CLAassistant
Copy link

CLAassistant commented Jun 13, 2022

CLA assistant check
All committers have signed the CLA.

@trema96 trema96 changed the title Fix InternalDAO get with rev Minor fixes Jun 17, 2022
Copy link
Contributor

@lisatr2812 lisatr2812 left a comment

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Before refactoring the GenericDAO, I would like to be sure we are not breaking anything, as this is one of the most used components in BackEnd.
Could you add a unit test to ensure the behaviour of purge method is still the same and do some test in acceptance environment ?

@@ -291,17 +290,16 @@ abstract class GenericDAOImpl<T : StoredDocument>(
}
}

// This function is not reactive, but it doesn't seem to be used at all anyway...
override suspend fun purge(entities: Collection<T>) {
override suspend fun purge(entities: Collection<T>): Flow<DocIdentifier> = flow {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

If this method returns a Flow, it should not be a suspend function anymore.

afterDelete(entity)
}
val entitiesById = entities.associateBy { it.id }
val bulkDeleteResults = client.bulkDelete(entities.map { beforeDelete(it) })
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

You can do it as a one-line code, this way :

emitAll(client.bulkDelete(entities.map { beforeDelete(it) })
				.onEach { r -> entitiesById[r.id]?.let { afterDelete(it) } }
				.map { updateResult -> DocIdentifier(updateResult.id, updateResult.rev) })

Sign up for free to subscribe to this conversation on GitHub. Already have an account? Sign in.
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants