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

Allow multiple projects per repo #18774

Merged
merged 1 commit into from
Sep 25, 2023
Merged
Show file tree
Hide file tree
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
Original file line number Diff line number Diff line change
Expand Up @@ -29,8 +29,7 @@ export const NewProjectRepoList: FC<Props> = ({ filteredRepos, noReposAvailable,
<div className="flex-grow">
<div
className={
"text-base text-gray-900 dark:text-gray-50 font-medium rounded-xl whitespace-nowrap" +
(r.inUse ? " text-gray-400 dark:text-gray-500" : "text-gray-700")
"text-base text-gray-900 dark:text-gray-50 font-medium rounded-xl whitespace-nowrap text-gray-700"
}
>
{toSimpleName(r)}
Expand All @@ -39,17 +38,9 @@ export const NewProjectRepoList: FC<Props> = ({ filteredRepos, noReposAvailable,
</div>
<div className="flex justify-end">
<div className="h-full my-auto flex self-center opacity-0 group-hover:opacity-100 items-center mr-2 text-right">
{!r.inUse ? (
<Button onClick={() => onRepoSelected(r)} loading={isCreating}>
Select
</Button>
) : (
<p className="text-gray-500">
Project already
<br />
exists.
</p>
)}
<Button onClick={() => onRepoSelected(r)} loading={isCreating}>
Select
gtsiolis marked this conversation as resolved.
Show resolved Hide resolved
</Button>
Copy link
Member Author

Choose a reason for hiding this comment

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

we no longer forbid creating a project if one for that repo already exists

</div>
</div>
</div>
Expand Down
2 changes: 2 additions & 0 deletions components/dashboard/src/workspaces/CreateWorkspacePage.tsx
Original file line number Diff line number Diff line change
Expand Up @@ -237,6 +237,7 @@ export function CreateWorkspacePage() {
const result = await createWorkspaceMutation.createWorkspace({
contextUrl: contextURL,
organizationId,
projectId: selectedProjectID,
...opts,
});
await storeAutoStartOptions();
Expand All @@ -263,6 +264,7 @@ export function CreateWorkspacePage() {
selectedIde,
useLatestIde,
createWorkspaceMutation,
selectedProjectID,
storeAutoStartOptions,
history,
autostart,
Expand Down
3 changes: 1 addition & 2 deletions components/gitpod-db/src/project-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -10,8 +10,7 @@ import { TransactionalDB } from "./typeorm/transactional-db-impl";
export const ProjectDB = Symbol("ProjectDB");
export interface ProjectDB extends TransactionalDB<ProjectDB> {
findProjectById(projectId: string): Promise<Project | undefined>;
findProjectByCloneUrl(cloneUrl: string): Promise<Project | undefined>;
findProjectsByCloneUrls(cloneUrls: string[]): Promise<(Project & { teamOwners?: string[] })[]>;
findProjectsByCloneUrl(cloneUrl: string): Promise<Project[]>;
Copy link
Member Author

Choose a reason for hiding this comment

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

we still need a method like this because we need to look up the project(s) when a webhook event comes in.

findProjects(orgID: string): Promise<Project[]>;
findProjectsBySearchTerm(
offset: number,
Expand Down
44 changes: 4 additions & 40 deletions components/gitpod-db/src/typeorm/project-db-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -7,7 +7,7 @@
import { PartialProject, Project, ProjectEnvVar, ProjectEnvVarWithValue, ProjectUsage } from "@gitpod/gitpod-protocol";
import { EncryptionService } from "@gitpod/gitpod-protocol/lib/encryption/encryption-service";
import { inject, injectable, optional } from "inversify";
import { EntityManager, Repository } from "typeorm";
import { EntityManager, FindConditions, Repository } from "typeorm";
import { v4 as uuidv4 } from "uuid";
import { ProjectDB } from "../project-db";
import { DBProject } from "./entity/db-project";
Expand Down Expand Up @@ -58,46 +58,10 @@ export class ProjectDBImpl extends TransactionalDBImpl<ProjectDB> implements Pro
return repo.findOne({ id: projectId, markedDeleted: false });
}

public async findProjectByCloneUrl(cloneUrl: string): Promise<Project | undefined> {
public async findProjectsByCloneUrl(cloneUrl: string): Promise<Project[]> {
const repo = await this.getRepo();
return repo.findOne({ cloneUrl, markedDeleted: false });
}

public async findProjectsByCloneUrls(cloneUrls: string[]): Promise<(Project & { teamOwners?: string[] })[]> {
if (cloneUrls.length === 0) {
return [];
}
const repo = await this.getRepo();
const q = repo
.createQueryBuilder("project")
.where("project.markedDeleted = false")
.andWhere(`project.cloneUrl in (${cloneUrls.map((u) => `'${u}'`).join(", ")})`);
const projects = await q.getMany();

const teamIds = Array.from(new Set(projects.map((p) => p.teamId).filter((id) => !!id)));

const teamIdsAndOwners =
teamIds.length === 0
? []
: ((await (
await this.getEntityManager()
).query(`
SELECT member.teamId AS teamId, user.name AS owner FROM d_b_user AS user
LEFT JOIN d_b_team_membership AS member ON (user.id = member.userId)
WHERE member.teamId IN (${teamIds.map((id) => `'${id}'`).join(", ")})
AND member.deleted = 0
AND member.role = 'owner'
`)) as { teamId: string; owner: string }[]);

const result: (Project & { teamOwners?: string[] })[] = [];
for (const project of projects) {
result.push({
...project,
teamOwners: teamIdsAndOwners.filter((i) => i.teamId === project.teamId).map((i) => i.owner),
});
}

return result;
const conditions: FindConditions<DBProject> = { cloneUrl, markedDeleted: false };
return repo.find(conditions);
}

public async findProjects(orgId: string): Promise<Project[]> {
Expand Down
47 changes: 12 additions & 35 deletions components/gitpod-db/src/typeorm/workspace-db-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -57,6 +57,7 @@ import {
} from "./metrics";
import { TransactionalDBImpl } from "./transactional-db-impl";
import { TypeORM } from "./typeorm";
import { ApplicationError, ErrorCodes } from "@gitpod/gitpod-protocol/lib/messaging/error";

type RawTo<T> = (instance: WorkspaceInstance, ws: Workspace) => T;
interface OrderBy {
Expand Down Expand Up @@ -710,16 +711,19 @@ export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl<WorkspaceDB> imp

// Find the (last triggered) prebuild for a given commit
public async findPrebuiltWorkspaceByCommit(
cloneURL: string,
projectId: string,
Copy link
Member Author

Choose a reason for hiding this comment

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

A cloneURL was used across the board to identify the project. Since we now can have multiple projects per cloneURL this is no longer the case, so we pass on the projected instead. This is the majority of this changeset.

commit: string,
): Promise<PrebuiltWorkspace | undefined> {
if (!commit || !cloneURL) {
return undefined;
if (!commit || !projectId) {
throw new ApplicationError(ErrorCodes.INTERNAL_SERVER_ERROR, "Illegal arguments", { projectId, commit });
Copy link
Member Author

Choose a reason for hiding this comment

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

we should not silently ignore such a state

Copy link
Member

Choose a reason for hiding this comment

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

Agree, but how comes that this is necessary in the persistence layer given the mandatory arguments. Let's check the call sites, too.

}
const repo = await this.getPrebuiltWorkspaceRepo();
return await repo
.createQueryBuilder("pws")
.where("pws.cloneURL = :cloneURL AND pws.commit LIKE :commit", { cloneURL, commit: commit + "%" })
.where("pws.projectId = :projectId AND pws.commit LIKE :commit", {
projectId,
commit: commit + "%",
})
.orderBy("pws.creationTime", "DESC")
.innerJoinAndMapOne(
"pws.workspace",
Expand Down Expand Up @@ -770,19 +774,12 @@ export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl<WorkspaceDB> imp
const repo = await this.getPrebuiltWorkspaceRepo();
return await repo.findOne(pwsid);
}
public async countRunningPrebuilds(cloneURL: string): Promise<number> {
const repo = await this.getPrebuiltWorkspaceRepo();
return await repo
.createQueryBuilder("pws")
.where('pws.cloneURL = :cloneURL AND state = "building"', { cloneURL })
.getCount();
}

public async findPrebuildsWithWorkpace(cloneURL: string): Promise<PrebuildWithWorkspace[]> {
public async findPrebuildsWithWorkspace(projectId: string): Promise<PrebuildWithWorkspace[]> {
const repo = await this.getPrebuiltWorkspaceRepo();

let query = repo.createQueryBuilder("pws");
query = query.where("pws.cloneURL = :cloneURL", { cloneURL });
query = query.where("pws.projectId = :projectId", { projectId });
query = query.orderBy("pws.creationTime", "DESC");
query = query.innerJoinAndMapOne("pws.workspace", DBWorkspace, "ws", "pws.buildWorkspaceId = ws.id");
query = query.andWhere("ws.deleted = false");
Expand All @@ -798,37 +795,17 @@ export class TypeORMWorkspaceDBImpl extends TransactionalDBImpl<WorkspaceDB> imp
});
}

public async countUnabortedPrebuildsSince(cloneURL: string, date: Date): Promise<number> {
public async countUnabortedPrebuildsSince(projectId: string, date: Date): Promise<number> {
const abortedState: PrebuiltWorkspaceState = "aborted";
const repo = await this.getPrebuiltWorkspaceRepo();

let query = repo.createQueryBuilder("pws");
query = query.where("pws.cloneURL = :cloneURL", { cloneURL });
query = query.where("pws.projectId != :projectId", { projectId });
Copy link
Member

Choose a reason for hiding this comment

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

should be a match, correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes 🙈

Copy link
Member

Choose a reason for hiding this comment

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

fixing via a840064 in parallel. Shall we create a separate PR?

Copy link
Member Author

Choose a reason for hiding this comment

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

fine with me to put it into your pr

query = query.andWhere("pws.creationTime >= :time", { time: date.toISOString() });
query = query.andWhere("pws.state != :state", { state: abortedState });
return query.getCount();
}

public async findQueuedPrebuilds(cloneURL?: string): Promise<PrebuildWithWorkspace[]> {
const repo = await this.getPrebuiltWorkspaceRepo();

let query = await repo.createQueryBuilder("pws");
query = query.where('state = "queued"');
if (cloneURL) {
query = query.andWhere("pws.cloneURL = :cloneURL", { cloneURL });
}
query = query.orderBy("pws.creationTime", "ASC");
query = query.innerJoinAndMapOne("pws.workspace", DBWorkspace, "ws", "pws.buildWorkspaceId = ws.id");

const res = await query.getMany();
return res.map((r) => {
const withWorkspace: PrebuiltWorkspace & { workspace: Workspace } = r as any;
return {
prebuild: r,
workspace: withWorkspace.workspace,
};
});
}
public async attachUpdatableToPrebuild(pwsid: string, update: PrebuiltWorkspaceUpdatable): Promise<void> {
const repo = await this.getPrebuiltWorkspaceUpdatableRepo();
await repo.save(update);
Expand Down
18 changes: 17 additions & 1 deletion components/gitpod-db/src/workspace-db.spec.db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@ import { TypeORM } from "./typeorm/typeorm";
import { DBPrebuiltWorkspace } from "./typeorm/entity/db-prebuilt-workspace";
import { secondsBefore } from "@gitpod/gitpod-protocol/lib/util/timeutil";
import { resetDB } from "./test/reset-db";
import { v4 } from "uuid";

@suite
class WorkspaceDBSpec {
Expand Down Expand Up @@ -508,6 +509,7 @@ class WorkspaceDBSpec {
public async testCountUnabortedPrebuildsSince() {
const now = new Date();
const cloneURL = "https://github.com/gitpod-io/gitpod";
const projectId = v4();

await Promise.all([
// Created now, and queued
Expand All @@ -516,6 +518,7 @@ class WorkspaceDBSpec {
buildWorkspaceId: "apples",
creationTime: now.toISOString(),
cloneURL: cloneURL,
projectId,
commit: "",
state: "queued",
statusVersion: 0,
Expand All @@ -526,6 +529,7 @@ class WorkspaceDBSpec {
buildWorkspaceId: "bananas",
creationTime: now.toISOString(),
cloneURL: cloneURL,
projectId,
commit: "",
state: "aborted",
statusVersion: 0,
Expand All @@ -536,14 +540,26 @@ class WorkspaceDBSpec {
buildWorkspaceId: "oranges",
creationTime: secondsBefore(now.toISOString(), 62),
cloneURL: cloneURL,
projectId,
commit: "",
state: "available",
statusVersion: 0,
}),
// different project now and queued
this.storePrebuiltWorkspace({
id: "prebuild123-other",
buildWorkspaceId: "apples",
creationTime: now.toISOString(),
cloneURL: cloneURL,
projectId: "other-projectId",
commit: "",
state: "queued",
statusVersion: 0,
}),
]);

const minuteAgo = secondsBefore(now.toISOString(), 60);
const unabortedCount = await this.db.countUnabortedPrebuildsSince(cloneURL, new Date(minuteAgo));
const unabortedCount = await this.db.countUnabortedPrebuildsSince(projectId, new Date(minuteAgo));
expect(unabortedCount).to.eq(1);
}

Expand Down
9 changes: 3 additions & 6 deletions components/gitpod-db/src/workspace-db.ts
Original file line number Diff line number Diff line change
Expand Up @@ -133,7 +133,6 @@ export interface WorkspaceDB {
findInstancesByPhase(phases: string[]): Promise<WorkspaceInstance[]>;

getWorkspaceCount(type?: String): Promise<Number>;
getWorkspaceCountByCloneURL(cloneURL: string, sinceLastDays?: number, type?: string): Promise<number>;
getInstanceCount(type?: string): Promise<number>;

findRegularRunningInstances(userId?: string): Promise<WorkspaceInstance[]>;
Expand All @@ -158,17 +157,15 @@ export interface WorkspaceDB {
updateSnapshot(snapshot: DeepPartial<Snapshot> & Pick<Snapshot, "id">): Promise<void>;

storePrebuiltWorkspace(pws: PrebuiltWorkspace): Promise<PrebuiltWorkspace>;
findPrebuiltWorkspaceByCommit(cloneURL: string, commit: string): Promise<PrebuiltWorkspace | undefined>;
findPrebuiltWorkspaceByCommit(projectId: string, commit: string): Promise<PrebuiltWorkspace | undefined>;
findActivePrebuiltWorkspacesByBranch(
projectId: string,
branch: string,
): Promise<PrebuildWithWorkspaceAndInstances[]>;
findPrebuildsWithWorkpace(cloneURL: string): Promise<PrebuildWithWorkspace[]>;
findPrebuildsWithWorkspace(projectId: string): Promise<PrebuildWithWorkspace[]>;
findPrebuildByWorkspaceID(wsid: string): Promise<PrebuiltWorkspace | undefined>;
findPrebuildByID(pwsid: string): Promise<PrebuiltWorkspace | undefined>;
countRunningPrebuilds(cloneURL: string): Promise<number>;
countUnabortedPrebuildsSince(cloneURL: string, date: Date): Promise<number>;
findQueuedPrebuilds(cloneURL?: string): Promise<PrebuildWithWorkspace[]>;
countUnabortedPrebuildsSince(projectId: string, date: Date): Promise<number>;
attachUpdatableToPrebuild(pwsid: string, update: PrebuiltWorkspaceUpdatable): Promise<void>;
findUpdatablesForPrebuild(pwsid: string): Promise<PrebuiltWorkspaceUpdatable[]>;
markUpdatableResolved(updatableId: string): Promise<void>;
Expand Down
3 changes: 1 addition & 2 deletions components/gitpod-protocol/src/gitpod-service.ts
Original file line number Diff line number Diff line change
Expand Up @@ -323,8 +323,6 @@ export interface ProviderRepository {
updatedAt?: string;
installationId?: number;
installationUpdatedAt?: string;

inUse?: { userName: string };
}

export interface ClientHeaderFields {
Expand Down Expand Up @@ -416,6 +414,7 @@ export namespace GitpodServer {
export interface CreateWorkspaceOptions extends StartWorkspaceOptions {
contextUrl: string;
organizationId: string;
projectId?: string;

// whether running workspaces on the same context should be ignored. If false (default) users will be asked.
//TODO(se) remove this option and let clients do that check if they like. The new create workspace page does it already
Expand Down
Loading
Loading