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.

Sign up for GitHub

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

138 Image level tags #277

Merged
merged 11 commits into from
Dec 3, 2024
Merged

138 Image level tags #277

Show file tree
Hide file tree
Changes from 8 commits
Commits
Show all changes
11 commits
Select commit Hold shift + click to select a range
8e31d77
feat(138.b): add project tag schema and update project schema
lessej Oct 23, 2024
52b1676
feat(138.b): add mutation for creating project tag
lessej Oct 24, 2024
4ea5bd5
feat(138.b): add delete project tag resolver and type defs
lessej Oct 25, 2024
f851495
feat(138.b): add resolver and model methods for update tag
lessej Oct 25, 2024
8e83fd5
fix: remove unneeded code
lessej Oct 25, 2024
3cb8057
feat(138): add resolvers, schema, type defs for creating and deleting…
lessej Oct 31, 2024
97cf121
fix: fix filter
lessej Nov 1, 2024
8bc0d2c
feat(138): implement cascading delete
lessej Nov 2, 2024
eb13d89
chore: merge main and resolve conflicts
lessej Nov 17, 2024
cc107cc
fix: fix bulk delete of project tag from iamges
lessej Nov 26, 2024
21d51c7
feat(138): add max for number of images that can be modified when rem…
lessej Dec 1, 2024
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
74 changes: 74 additions & 0 deletions src/@types/graphql.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -162,6 +162,11 @@ export type CreateImagePayload = {
imageAttempt?: Maybe<ImageAttempt>;
};

export type CreateImageTagInput = {
imageId: Scalars['ID']['input'];
tagId: Scalars['ID']['input'];
};

export type CreateInternalLabelInput = {
bbox: Array<Scalars['Float']['input']>;
conf?: InputMaybe<Scalars['Float']['input']>;
Expand Down Expand Up @@ -215,6 +220,11 @@ export type CreateProjectLabelInput = {
reviewerEnabled?: InputMaybe<Scalars['Boolean']['input']>;
};

export type CreateProjectTagInput = {
color: Scalars['String']['input'];
name: Scalars['String']['input'];
};

export type CreateUploadInput = {
originalFile: Scalars['String']['input'];
partCount?: InputMaybe<Scalars['Int']['input']>;
Expand Down Expand Up @@ -256,6 +266,11 @@ export type DeleteImageCommentInput = {
imageId: Scalars['ID']['input'];
};

export type DeleteImageTagInput = {
imageId: Scalars['ID']['input'];
tagId: Scalars['ID']['input'];
};

export type DeleteImagesInput = {
imageIds?: InputMaybe<Array<Scalars['ID']['input']>>;
};
Expand Down Expand Up @@ -283,6 +298,10 @@ export type DeleteProjectLabelInput = {
_id: Scalars['ID']['input'];
};

export type DeleteProjectTagInput = {
_id: Scalars['ID']['input'];
};

export type DeleteViewInput = {
viewId: Scalars['ID']['input'];
};
Expand Down Expand Up @@ -416,6 +435,7 @@ export type Image = {
path?: Maybe<Scalars['String']['output']>;
projectId: Scalars['String']['output'];
reviewed?: Maybe<Scalars['Boolean']['output']>;
tags?: Maybe<Array<Scalars['ID']['output']>>;
timezone: Scalars['String']['output'];
userSetData?: Maybe<Scalars['JSONObject']['output']>;
};
Expand Down Expand Up @@ -483,6 +503,11 @@ export type ImageMetadata = {
timezone?: Maybe<Scalars['String']['output']>;
};

export type ImageTagsPayload = {
__typename?: 'ImageTagsPayload';
tags?: Maybe<Array<Scalars['ID']['output']>>;
};

export type ImagesConnection = {
__typename?: 'ImagesConnection';
images: Array<Image>;
Expand Down Expand Up @@ -559,20 +584,24 @@ export type Mutation = {
createImage?: Maybe<CreateImagePayload>;
createImageComment?: Maybe<ImageCommentsPayload>;
createImageError?: Maybe<ImageError>;
createImageTag?: Maybe<ImageTagsPayload>;
createInternalLabels?: Maybe<StandardPayload>;
createLabels?: Maybe<StandardPayload>;
createObjects?: Maybe<StandardPayload>;
createProject?: Maybe<ProjectPayload>;
createProjectLabel?: Maybe<ProjectLabelPayload>;
createProjectTag?: Maybe<ProjectTagsPayload>;
createUpload?: Maybe<CreateUploadPayload>;
createUser?: Maybe<StandardPayload>;
createView?: Maybe<CreateViewPayload>;
deleteDeployment?: Maybe<Task>;
deleteImageComment?: Maybe<ImageCommentsPayload>;
deleteImageTag?: Maybe<ImageTagsPayload>;
deleteImages?: Maybe<StandardErrorPayload>;
deleteLabels?: Maybe<StandardPayload>;
deleteObjects?: Maybe<StandardPayload>;
deleteProjectLabel?: Maybe<StandardPayload>;
deleteProjectTag?: Maybe<ProjectTagsPayload>;
deleteView?: Maybe<DeleteViewPayload>;
redriveBatch?: Maybe<StandardPayload>;
registerCamera?: Maybe<RegisterCameraPayload>;
Expand All @@ -587,6 +616,7 @@ export type Mutation = {
updateObjects?: Maybe<StandardPayload>;
updateProject?: Maybe<ProjectPayload>;
updateProjectLabel?: Maybe<ProjectLabelPayload>;
updateProjectTag?: Maybe<ProjectTagsPayload>;
updateUser?: Maybe<StandardPayload>;
updateView?: Maybe<UpdateViewPayload>;
};
Expand Down Expand Up @@ -632,6 +662,11 @@ export type MutationCreateImageErrorArgs = {
};


export type MutationCreateImageTagArgs = {
input: CreateImageTagInput;
};


export type MutationCreateInternalLabelsArgs = {
input: CreateInternalLabelsInput;
};
Expand All @@ -657,6 +692,11 @@ export type MutationCreateProjectLabelArgs = {
};


export type MutationCreateProjectTagArgs = {
input: CreateProjectTagInput;
};


export type MutationCreateUploadArgs = {
input: CreateUploadInput;
};
Expand All @@ -682,6 +722,11 @@ export type MutationDeleteImageCommentArgs = {
};


export type MutationDeleteImageTagArgs = {
input: DeleteImageTagInput;
};


export type MutationDeleteImagesArgs = {
input: DeleteImagesInput;
};
Expand All @@ -702,6 +747,11 @@ export type MutationDeleteProjectLabelArgs = {
};


export type MutationDeleteProjectTagArgs = {
input: DeleteProjectTagInput;
};


export type MutationDeleteViewArgs = {
input: DeleteViewInput;
};
Expand Down Expand Up @@ -772,6 +822,11 @@ export type MutationUpdateProjectLabelArgs = {
};


export type MutationUpdateProjectTagArgs = {
input: UpdateProjectTagInput;
};


export type MutationUpdateUserArgs = {
input: UpdateUserInput;
};
Expand Down Expand Up @@ -844,6 +899,7 @@ export type Project = {
description?: Maybe<Scalars['String']['output']>;
labels?: Maybe<Array<ProjectLabel>>;
name: Scalars['String']['output'];
tags?: Maybe<Array<ProjectTag>>;
timezone: Scalars['String']['output'];
views: Array<View>;
};
Expand Down Expand Up @@ -874,6 +930,18 @@ export type ProjectRegistration = {
projectId: Scalars['String']['output'];
};

export type ProjectTag = {
__typename?: 'ProjectTag';
_id: Scalars['ID']['output'];
color: Scalars['String']['output'];
name: Scalars['String']['output'];
};

export type ProjectTagsPayload = {
__typename?: 'ProjectTagsPayload';
tags?: Maybe<Array<ProjectTag>>;
};

export type Query = {
__typename?: 'Query';
batches?: Maybe<BatchesConnection>;
Expand Down Expand Up @@ -1147,6 +1215,12 @@ export type UpdateProjectLabelInput = {
reviewerEnabled?: InputMaybe<Scalars['Boolean']['input']>;
};

export type UpdateProjectTagInput = {
_id: Scalars['ID']['input'];
color: Scalars['String']['input'];
name: Scalars['String']['input'];
};

export type UpdateUserInput = {
roles: Array<UserRole>;
username: Scalars['String']['input'];
Expand Down
2 changes: 2 additions & 0 deletions src/api/auth/roles.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,7 @@ const WRITE_DEPLOYMENTS_ROLES = [MANAGER];
const WRITE_AUTOMATION_RULES_ROLES = [MANAGER];
const WRITE_CAMERA_REGISTRATION_ROLES = [MANAGER];
const WRITE_CAMERA_SERIAL_NUMBER_ROLES = [MANAGER];
const WRITE_TAGS_ROLES = [MANAGER, MEMBER];

export {
READ_TASKS_ROLES,
Expand All @@ -30,4 +31,5 @@ export {
WRITE_AUTOMATION_RULES_ROLES,
WRITE_CAMERA_REGISTRATION_ROLES,
WRITE_CAMERA_SERIAL_NUMBER_ROLES,
WRITE_TAGS_ROLES,
};
76 changes: 75 additions & 1 deletion src/api/db/models/Image.ts
View file
Open in desktop
Original file line number Diff line number Diff line change
Expand Up @@ -9,7 +9,7 @@ import GraphQLError, {
NotFoundError,
} from '../../errors.js';
import { BulkWriteResult } from 'mongodb';
import mongoose, { HydratedDocument } from 'mongoose';
import mongoose, { HydratedDocument, UpdateWriteOpResult } from 'mongoose';
import MongoPaging, { AggregationOutput } from 'mongo-cursor-pagination';
import { TaskModel } from './Task.js';
import { ObjectSchema } from '../schemas/shared/index.js';
Expand All @@ -28,6 +28,7 @@ import {
WRITE_IMAGES_ROLES,
WRITE_COMMENTS_ROLES,
EXPORT_DATA_ROLES,
WRITE_TAGS_ROLES,
} from '../../auth/roles.js';
import {
buildPipeline,
Expand Down Expand Up @@ -475,6 +476,69 @@ export class ImageModel {
}
}

static async createTag(
input: gql.CreateImageTagInput,
context: Pick<Context, 'user'>,
): Promise<{ tags: mongoose.Types.ObjectId[] }> {
try {
const image = await ImageModel.queryById(input.imageId, context);

if (!image.tags) {
image.tags = [] as any as mongoose.Types.DocumentArray<mongoose.Types.ObjectId>;
}

image.tags.push(new mongoose.Types.ObjectId(input.tagId));
await image.save();

return { tags: image.tags };
} catch (err) {
if (err instanceof GraphQLError) throw err;
throw new InternalServerError(err as string);
}
}

static async deleteTag(
input: gql.DeleteImageTagInput,
context: Pick<Context, 'user'>,
): Promise<{ tags: mongoose.Types.ObjectId[] }> {
try {
const image = await ImageModel.queryById(input.imageId, context);

const tag = image.tags?.filter((t) => idMatch(t, input.tagId))[0];
if (!tag) throw new NotFoundError('Tag not found on image');

image.tags = image.tags.filter(
(t) => !idMatch(t, input.tagId),
) as mongoose.Types.ObjectId[];

await image.save();

return { tags: image.tags };
} catch (err) {
if (err instanceof GraphQLError) throw err;
throw new InternalServerError(err as string);
}
}

static async deleteProjectTag(
input: { tagId: string },
context: Pick<Context, 'user'>,
): Promise<UpdateWriteOpResult> {
try {
const res = await Image.updateMany({
"tags": input.tagId
}, {
"$pull": {
"tags": input.tagId
}
});
Copy link
Member

Choose a reason for hiding this comment

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

Have you stress tested this yet? How many tags can we remove on images within the 30 second time out? I see we capped it at 500 images but is that just arbitrary? I'm guessing that because it's a single updateMany call it is probably pretty efficient, but we should understand what the limit is. We have some projects with ~2 million images so you can imagine at that volume we might run into timeouts.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

Hey @nathanielrindlaub sorry I forgot to get back to fixing this. I put up a bug fix because it seems the deletion wasn't working as I expected. I did some quick checks of performance with as many images as possible in the current project. Please see below

Type Image Count Duration
Cold Start 1014 4118.89ms
Cold Start 1014 4150.08ms
Cold Start 1014 4233.56ms
Cold Start 1014 4402.07ms
Cold Start 1014 4036.09ms
Cold Start 1014 3992.30ms
Cold Start 1 3900.83ms
Cold Start 1 3837.03ms
Cold Start 1 4230.82ms
Cold Start 1 3829.27ms
Cold Start 1 3930.13ms
Cold Start 1 4083.47ms
Warm 1014 805.65ms
Warm 1014 886.84ms
Warm 1014 819.05ms
Warm 1014 786.62ms
Warm 1014 781.23ms
Warm 1014 870.32ms
Warm 1 653.31ms
Warm 1 643.08ms
Warm 1 634.07ms
Warm 1 622.23ms
Warm 1 619.41ms
Warm 1 701.56ms

These were taken on the local serverless offline lambda so I don't know if they will be useful. I used the shell in Compass to add a tag to all images in the default project and then removed them all using the UI which calls the local API.

I believe the 500 image limit is a limit for removing images with a specific label (object label). Tags is uncapped at the moment.

This data is likely not conclusive by any means so if you have a better way to stress test it closer to 2 million images at once, please let me know!

Copy link
Member

Choose a reason for hiding this comment

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

I believe the 500 image limit is a limit for removing images with a specific label (object label). Tags is uncapped at the moment.

@JesseLeung97, sorry I'm not 100% tracking what you mean here. Mind elaborating a bit?

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nathanielrindlaub Sorry for the confusion. I was responding to this

How many tags can we remove on images within the 30 second time out? I see we capped it at 500 images but is that just arbitrary?

This 500 image removal limit is not for this feature (image level tags). I believe it's for object labels.

Copy link
Member

Choose a reason for hiding this comment

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

@JesseLeung97 oh sorry I was confused by the MAX_TAG_DELETE variable in this line, but I just searched and it doesn't seem to be used anywhere.

Copy link
Collaborator Author

Choose a reason for hiding this comment

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

@nathanielrindlaub In testing on dev with 52k images:

Image Count Duration
51889 10.878s
51889 10.973s
51889 10.548s
51889 11.230s
51889 11.120s

I did not observe any timeouts when removing the tag from 52k images and the max runtime was just under 12s. I arbitrarily set the upper limit for deleting at 50,000 given that's a round number near the max that we've been able to test. If we can add more images, I'm happy to continue raising the limit until it breaks.

return res;
} catch (err) {
if (err instanceof GraphQLError) throw err;
throw new InternalServerError(err as string);
}
}

/**
* Finds Image records and creates new Object subdocuments on them
* It's used by frontend when creating new empty objects and when adding
Expand Down Expand Up @@ -1113,6 +1177,16 @@ export default class AuthedImageModel extends BaseAuthedModel {
return ImageModel.queryByFilter(...args);
}

@roleCheck(WRITE_TAGS_ROLES)
createTag(...args: MethodParams<typeof ImageModel.createTag>) {
return ImageModel.createTag(...args);
}

@roleCheck(WRITE_TAGS_ROLES)
deleteTag(...args: MethodParams<typeof ImageModel.deleteTag>) {
return ImageModel.deleteTag(...args);
}

@roleCheck(WRITE_COMMENTS_ROLES)
createComment(...args: MethodParams<typeof ImageModel.createComment>) {
return ImageModel.createComment(...args);
Expand Down
Loading
Loading