Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

feat(core): drop the closure table pls #3900

Merged
merged 4 commits into from
Feb 7, 2025
Merged
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
6 changes: 2 additions & 4 deletions packages/server/modules/activitystream/tests/activity.spec.js
Original file line number Diff line number Diff line change
@@ -61,8 +61,7 @@ const {
const { getServerInfoFactory } = require('@/modules/core/repositories/server')
const { createObjectFactory } = require('@/modules/core/services/objects/management')
const {
storeSingleObjectIfNotFoundFactory,
storeClosuresIfNotFoundFactory
storeSingleObjectIfNotFoundFactory
} = require('@/modules/core/repositories/objects')
const { getEventBus } = require('@/modules/shared/services/eventBus')

@@ -121,8 +120,7 @@ const createPersonalAccessToken = createPersonalAccessTokenFactory({
storePersonalApiToken: storePersonalApiTokenFactory({ db })
})
const createObject = createObjectFactory({
storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }),
storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db })
storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db })
})

let server
4 changes: 1 addition & 3 deletions packages/server/modules/cli/commands/download/commit.ts
Original file line number Diff line number Diff line change
@@ -16,7 +16,6 @@ import {
import {
getObjectFactory,
getStreamObjectsFactory,
storeClosuresIfNotFoundFactory,
storeSingleObjectIfNotFoundFactory
} from '@/modules/core/repositories/objects'
import {
@@ -187,8 +186,7 @@ const command: CommandModule<
const createObject = createObjectFactory({
storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({
db: projectDb
}),
storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDb })
})
})
const getUser = getUserFactory({ db })
const getStreamCollaborators = getStreamCollaboratorsFactory({ db })
4 changes: 1 addition & 3 deletions packages/server/modules/cli/commands/download/project.ts
Original file line number Diff line number Diff line change
@@ -19,7 +19,6 @@ import { createCommitByBranchIdFactory } from '@/modules/core/services/commit/ma
import {
getObjectFactory,
getStreamObjectsFactory,
storeClosuresIfNotFoundFactory,
storeSingleObjectIfNotFoundFactory
} from '@/modules/core/repositories/objects'
import {
@@ -224,8 +223,7 @@ const command: CommandModule<
const createObject = createObjectFactory({
storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({
db: projectDb
}),
storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDb })
})
})
const getStreamCollaborators = getStreamCollaboratorsFactory({ db })
const getStreamBranchByName = getStreamBranchByNameFactory({ db: projectDb })
6 changes: 2 additions & 4 deletions packages/server/modules/comments/tests/comments.graph.spec.js
Original file line number Diff line number Diff line change
@@ -53,8 +53,7 @@ const {
} = require('@/modules/core/repositories/streams')
const {
getObjectFactory,
storeSingleObjectIfNotFoundFactory,
storeClosuresIfNotFoundFactory
storeSingleObjectIfNotFoundFactory
} = require('@/modules/core/repositories/objects')
const {
legacyCreateStreamFactory,
@@ -219,8 +218,7 @@ const createUser = createUserFactory({
emitEvent: getEventBus().emit
})
const createObject = createObjectFactory({
storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }),
storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db })
storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db })
})

function buildCommentInputFromString(textString) {
6 changes: 2 additions & 4 deletions packages/server/modules/comments/tests/comments.spec.ts
Original file line number Diff line number Diff line change
@@ -69,8 +69,7 @@ import {
} from '@/modules/core/repositories/branches'
import {
getObjectFactory,
storeSingleObjectIfNotFoundFactory,
storeClosuresIfNotFoundFactory
storeSingleObjectIfNotFoundFactory
} from '@/modules/core/repositories/objects'
import {
legacyCreateStreamFactory,
@@ -255,8 +254,7 @@ const createUser = createUserFactory({
emitEvent: getEventBus().emit
})
const createObject = createObjectFactory({
storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db }),
storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db })
storeSingleObjectIfNotFoundFactory: storeSingleObjectIfNotFoundFactory({ db })
})

function buildCommentInputFromString(textString?: string) {
9 changes: 1 addition & 8 deletions packages/server/modules/core/domain/objects/operations.ts
Original file line number Diff line number Diff line change
@@ -2,8 +2,7 @@ import { Logger } from '@/logging/logging'
import {
InsertableSpeckleObject,
RawSpeckleObject,
SpeckleObject,
SpeckleObjectClosureEntry
SpeckleObject
} from '@/modules/core/domain/objects/types'
import { BatchedSelectOptions } from '@/modules/shared/helpers/dbHelper'
import { MaybeNullOrUndefined, Nullable, Optional } from '@speckle/shared'
@@ -45,10 +44,6 @@ export type StoreObjectsIfNotFound = (
objects: Array<SpeckleObject | InsertableSpeckleObject>
) => Promise<void>

export type StoreClosuresIfNotFound = (
closures: SpeckleObjectClosureEntry[]
) => Promise<void>

export type GetObjectChildrenStream = (params: {
streamId: string
objectId: string
@@ -117,8 +112,6 @@ type CreateObjectsParams = {
logger?: Logger
}

export type CreateObjectsBatched = (params: CreateObjectsParams) => Promise<boolean>

export type CreateObjectsBatchedAndNoClosures = (
params: CreateObjectsParams
) => Promise<string[]>
4 changes: 1 addition & 3 deletions packages/server/modules/core/domain/objects/types.ts
Original file line number Diff line number Diff line change
@@ -1,9 +1,7 @@
import { ObjectChildrenClosureRecord, ObjectRecord } from '@/modules/core/helpers/types'
import { ObjectRecord } from '@/modules/core/helpers/types'
import { Nullable, NullableKeysToOptional } from '@speckle/shared'
import { OverrideProperties, SetOptional } from 'type-fest'

export type SpeckleObjectClosureEntry = ObjectChildrenClosureRecord

export type SpeckleObject = ObjectRecord

/**
4 changes: 1 addition & 3 deletions packages/server/modules/core/graph/resolvers/objects.ts
Original file line number Diff line number Diff line change
@@ -5,7 +5,6 @@ import {
getObjectChildrenFactory,
getObjectChildrenQueryFactory,
getObjectFactory,
storeClosuresIfNotFoundFactory,
storeObjectsIfNotFoundFactory
} from '@/modules/core/repositories/objects'
import { createObjectsFactory } from '@/modules/core/services/objects/management'
@@ -109,8 +108,7 @@ export = {
projectId: args.objectInput.streamId
})
const createObjects = createObjectsFactory({
storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db: projectDB }),
storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDB })
storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({ db: projectDB })
})
const ids = await createObjects({
streamId: args.objectInput.streamId,
7 changes: 0 additions & 7 deletions packages/server/modules/core/helpers/types.ts
Original file line number Diff line number Diff line change
@@ -136,13 +136,6 @@ export type ObjectRecord = {
streamId: string
}

export type ObjectChildrenClosureRecord = {
parent: string
child: string
minDepth: number
streamId: string
}

export type InvalidTokenResult = {
valid: false
}
Original file line number Diff line number Diff line change
@@ -0,0 +1,26 @@
import { Knex } from 'knex'

export async function up(knex: Knex): Promise<void> {
await knex.schema.dropTableIfExists('object_children_closure')
}

export async function down(knex: Knex): Promise<void> {
await knex.raw(`
-- Table Definition
CREATE TABLE "object_children_closure" (
"parent" varchar(255) NOT NULL,
"child" varchar(255) NOT NULL,
"minDepth" int4 NOT NULL DEFAULT 1,
"streamId" varchar(10) NOT NULL,
CONSTRAINT "object_children_closure_streamid_foreign" FOREIGN KEY ("streamId") REFERENCES "public"."streams"("id") ON DELETE CASCADE
);
`)

await knex.schema.alterTable('object_children_closure', (table) => {
table.index(['streamId', 'parent'])
table.index(['streamId', 'child'])
table.index(['streamId', 'minDepth'])
table.unique(['streamId', 'parent', 'child'], 'obj_parent_child_index')
table.index(['streamId', 'parent', 'minDepth'], 'full_pcd_index')
})
}
26 changes: 3 additions & 23 deletions packages/server/modules/core/repositories/objects.ts
Original file line number Diff line number Diff line change
@@ -1,6 +1,6 @@
import { Optional } from '@speckle/shared'
import { buildTableHelper, knex, Objects } from '@/modules/core/dbSchema'
import { ObjectChildrenClosureRecord, ObjectRecord } from '@/modules/core/helpers/types'
import { knex, Objects } from '@/modules/core/dbSchema'
import { ObjectRecord } from '@/modules/core/helpers/types'
import {
BatchedSelectOptions,
executeBatchedSelect
@@ -16,7 +16,6 @@ import {
GetObjectsStream,
GetStreamObjects,
HasObjects,
StoreClosuresIfNotFound,
StoreObjects,
StoreObjectsIfNotFound,
StoreSingleObjectIfNotFound
@@ -25,17 +24,8 @@ import { SpeckleObject } from '@/modules/core/domain/objects/types'
import { SetOptional } from 'type-fest'
import { get, set, toNumber } from 'lodash'

const ObjectChildrenClosure = buildTableHelper('object_children_closure', [
'parent',
'child',
'minDepth',
'streamId'
])

const tables = {
objects: (db: Knex) => db<ObjectRecord>(Objects.name),
objectChildrenClosure: (db: Knex) =>
db<ObjectChildrenClosureRecord>(ObjectChildrenClosure.name)
objects: (db: Knex) => db<ObjectRecord>(Objects.name)
}

export const getStreamObjectsFactory =
@@ -125,16 +115,6 @@ export const storeObjectsIfNotFoundFactory =
.ignore()
}

export const storeClosuresIfNotFoundFactory =
(deps: { db: Knex }): StoreClosuresIfNotFound =>
async (closuresBatch) => {
await tables
.objectChildrenClosure(deps.db)
.insert(closuresBatch)
.onConflict()
.ignore()
}

export const getObjectChildrenStreamFactory =
(deps: { db: Knex }): GetObjectChildrenStream =>
async ({ streamId, objectId }) => {
33 changes: 8 additions & 25 deletions packages/server/modules/core/rest/upload.ts
Original file line number Diff line number Diff line change
@@ -1,29 +1,19 @@
import zlib from 'zlib'
import { corsMiddleware } from '@/modules/core/configs/cors'
import Busboy from 'busboy'
import {
getFeatureFlags,
maximumObjectUploadFileSizeMb
} from '@/modules/shared/helpers/envHelper'
import { maximumObjectUploadFileSizeMb } from '@/modules/shared/helpers/envHelper'
import { ObjectHandlingError } from '@/modules/core/errors/object'
import { estimateStringMegabyteSize } from '@/modules/core/utils/chunking'
import { toMegabytesWith1DecimalPlace } from '@/modules/core/utils/formatting'
import { Router } from 'express'
import {
createObjectsBatchedAndNoClosuresFactory,
createObjectsBatchedFactory
} from '@/modules/core/services/objects/management'
import {
storeClosuresIfNotFoundFactory,
storeObjectsIfNotFoundFactory
} from '@/modules/core/repositories/objects'
import { createObjectsBatchedAndNoClosuresFactory } from '@/modules/core/services/objects/management'
import { storeObjectsIfNotFoundFactory } from '@/modules/core/repositories/objects'
import { validatePermissionsWriteStreamFactory } from '@/modules/core/services/streams/auth'
import { authorizeResolver, validateScopes } from '@/modules/shared'
import { getProjectDbClient } from '@/modules/multiregion/utils/dbSelector'
import { ExecuteHooks } from '@/modules/core/hooks'

const MAX_FILE_SIZE = maximumObjectUploadFileSizeMb() * 1024 * 1024
const { FF_NO_CLOSURE_WRITES } = getFeatureFlags()

export default (app: Router, { executeHooks }: { executeHooks: ExecuteHooks }) => {
const validatePermissionsWriteStream = validatePermissionsWriteStreamFactory({
@@ -70,18 +60,11 @@ export default (app: Router, { executeHooks }: { executeHooks: ExecuteHooks }) =

const projectDb = await getProjectDbClient({ projectId: req.params.streamId })

const objectInsertionService = FF_NO_CLOSURE_WRITES
? createObjectsBatchedAndNoClosuresFactory({
storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({
db: projectDb
})
})
: createObjectsBatchedFactory({
iainsproat marked this conversation as resolved.
Show resolved Hide resolved
storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({
db: projectDb
}),
storeClosuresIfNotFound: storeClosuresIfNotFoundFactory({ db: projectDb })
})
const objectInsertionService = createObjectsBatchedAndNoClosuresFactory({
storeObjectsIfNotFoundFactory: storeObjectsIfNotFoundFactory({
db: projectDb
})
})

let busboy
try {
Loading

Unchanged files with check annotations Beta

# google-chrome-stable is only available for amd64 so we have to fix the platform
# hadolint ignore=DL3029
FROM --platform=linux/amd64 node:18-bookworm-slim@sha256:408f8cbbb7b33a5bb94bdb8862795a94d2b64c2d516856824fd86c4a5594a443 AS node

Check warning on line 50 in packages/preview-service/Dockerfile

GitHub Actions / Build Preview Service

FROM --platform flag should not use a constant value

FromPlatformFlagConstDisallowed: FROM --platform flag should not use constant value "linux/amd64" More info: https://docs.docker.com/go/dockerfile/rule/from-platform-flag-const-disallowed/
SHELL ["/bin/bash", "-o", "pipefail", "-c"]
# Install tini and fonts