Skip to content

Commit

Permalink
Updating .isPolicyPublished and .readPolicies to take an optional con… (
Browse files Browse the repository at this point in the history
#1058)

* Updating .isPolicyPublished and .readPolicies to take an optional connection parameter and using it in the .publishPolicy code path to avoid a race condition where the DB reader has not received all policies yet, thus causing the publish function to believe the policy that needs publishing does not exist yet

* minor fixes

* minor fixes

* minor fixes
  • Loading branch information
janedotx authored Aug 17, 2022
1 parent 7d8d84c commit d98196b
Show file tree
Hide file tree
Showing 2 changed files with 38 additions and 31 deletions.
5 changes: 5 additions & 0 deletions .changeset/loud-meals-thank.md
Original file line number Diff line number Diff line change
@@ -0,0 +1,5 @@
---
"@mds-core/mds-policy-service": patch
---

Updating .isPolicyPublished and .readPolicies to take an optional connection parameter and using it in the .publishPolicy code path to avoid a race condition where the DB reader has not received all policies yet, thus causing the publish function to believe the policy that needs publishing does not exist yet'
64 changes: 33 additions & 31 deletions packages/mds-policy-service/repository/index.ts
Original file line number Diff line number Diff line change
Expand Up @@ -14,7 +14,7 @@
* limitations under the License.
*/

import type { InsertReturning } from '@mds-core/mds-repository'
import type { DataSource, InsertReturning } from '@mds-core/mds-repository'
import { ReadWriteRepository, RepositoryError } from '@mds-core/mds-repository'
import type { Timestamp, UUID } from '@mds-core/mds-types'
import { ConflictError, hasAtLeastOneEntry, NotFoundError, now } from '@mds-core/mds-utils'
Expand Down Expand Up @@ -44,7 +44,8 @@ export const PolicyRepository = ReadWriteRepository.Create('policies', { entitie
const readPolicies = async (
params: ReadPolicyQueryParams = {},
presentationOptions: PresentationOptions = {},
timestamp: Timestamp = now()
timestamp: Timestamp = now(),
connectionParam?: DataSource
) => {
const {
policy_ids,
Expand Down Expand Up @@ -103,7 +104,7 @@ export const PolicyRepository = ReadWriteRepository.Create('policies', { entitie
const ALIAS = 'pe'

try {
const connection = await repository.connect('ro')
const connection = connectionParam ?? (await repository.connect('ro'))
const query = connection.getRepository(PolicyEntity).createQueryBuilder(ALIAS)

const pager = (() => {
Expand Down Expand Up @@ -249,9 +250,9 @@ export const PolicyRepository = ReadWriteRepository.Create('policies', { entitie
}
}

const isPolicyPublished = async (policy_id: UUID) => {
const isPolicyPublished = async (policy_id: UUID, connectionParam?: DataSource) => {
try {
const connection = await repository.connect('ro')
const connection = connectionParam ?? (await repository.connect('ro'))
const entity = await connection.getRepository(PolicyEntity).findOneOrFail({ where: { policy_id } })
if (!entity) {
return false
Expand Down Expand Up @@ -416,13 +417,19 @@ export const PolicyRepository = ReadWriteRepository.Create('policies', { entitie
try {
const { beforeCommit = async () => undefined } = options

if (await isPolicyPublished(policy_id)) {
const connection = await repository.connect('rw')
if (await isPolicyPublished(policy_id, connection)) {
throw new ConflictError('Cannot re-publish existing policy')
}

const {
policies: [policy]
} = await readPolicies({ policy_ids: [policy_id], get_unpublished: true, get_published: null })
} = await readPolicies(
{ policy_ids: [policy_id], get_unpublished: true, get_published: null },
undefined,
undefined,
connection
)

if (!policy) {
throw new NotFoundError('cannot publish nonexistent policy')
Expand All @@ -432,30 +439,25 @@ export const PolicyRepository = ReadWriteRepository.Create('policies', { entitie
throw new ConflictError('Policies cannot be published after their start_date')
}

try {
const connection = await repository.connect('rw')
const publishedPolicy = await connection.transaction(async manager => {
const {
raw: [updated]
} = await manager
.getRepository(PolicyEntity)
.createQueryBuilder()
.update()
.set({ published_date })
.where('policy_id = :policy_id', { policy_id })
.andWhere('published_date IS NULL')
.returning('*')
.execute()
const mappedPolicy = PolicyEntityToDomain.map(updated)
await beforeCommit(mappedPolicy)

return mappedPolicy
})

return publishedPolicy
} catch (error) {
throw RepositoryError(error)
}
const publishedPolicy = await connection.transaction(async manager => {
const {
raw: [updated]
} = await manager
.getRepository(PolicyEntity)
.createQueryBuilder()
.update()
.set({ published_date })
.where('policy_id = :policy_id', { policy_id })
.andWhere('published_date IS NULL')
.returning('*')
.execute()
const mappedPolicy = PolicyEntityToDomain.map(updated)
await beforeCommit(mappedPolicy)

return mappedPolicy
})

return publishedPolicy
} catch (error) {
throw RepositoryError(error)
}
Expand Down

0 comments on commit d98196b

Please sign in to comment.