Skip to content

Dead code #1927

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

Merged
merged 10 commits into from
Jul 31, 2020
Merged
Show file tree
Hide file tree
Changes from 1 commit
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
Prev Previous commit
Next Next commit
Make providers endpoint-agnostic
A provider can now be registered on multiple endpoints (or potentially
moved if needed).
  • Loading branch information
code-asher committed Jul 27, 2020
commit e8f6d3005539761f2849d131ed5c843cdb636cd0
4 changes: 2 additions & 2 deletions src/node/app/proxy.ts
Original file line number Diff line number Diff line change
Expand Up @@ -24,7 +24,7 @@ export class ProxyHttpProvider extends HttpProvider {
const port = route.base.replace(/^\//, "")
return {
proxy: {
base: `${this.options.base}/${port}`,
base: `${route.providerBase}/${port}`,
port,
},
}
Expand All @@ -35,7 +35,7 @@ export class ProxyHttpProvider extends HttpProvider {
const port = route.base.replace(/^\//, "")
return {
proxy: {
base: `${this.options.base}/${port}`,
base: `${route.providerBase}/${port}`,
port,
},
}
Expand Down
2 changes: 1 addition & 1 deletion src/node/app/vscode.ts
Original file line number Diff line number Diff line change
Expand Up @@ -131,7 +131,7 @@ export class VscodeHttpProvider extends HttpProvider {
if (!this.isRoot(route)) {
throw new HttpError("Not found", HttpCode.NotFound)
} else if (!this.authenticated(request)) {
return { redirect: "/login", query: { to: this.options.base } }
return { redirect: "/login", query: { to: route.providerBase } }
}
try {
return await this.getRoot(request, route)
Expand Down
2 changes: 1 addition & 1 deletion src/node/entry.ts
Original file line number Diff line number Diff line change
Expand Up @@ -71,7 +71,7 @@ const main = async (args: Args, cliArgs: Args, configArgs: Args): Promise<void>
}

const httpServer = new HttpServer(options)
httpServer.registerHttpProvider("/", VscodeHttpProvider, args)
httpServer.registerHttpProvider(["/", "/vscode"], VscodeHttpProvider, args)
httpServer.registerHttpProvider("/update", UpdateHttpProvider, false)
httpServer.registerHttpProvider("/proxy", ProxyHttpProvider)
httpServer.registerHttpProvider("/login", LoginHttpProvider, args.config!, envPassword)
Expand Down
60 changes: 36 additions & 24 deletions src/node/http.ts
Original file line number Diff line number Diff line change
Expand Up @@ -79,9 +79,8 @@ export interface HttpResponse<T = string | Buffer | object> {
*/
mime?: string
/**
* Redirect to this path. Will rewrite against the base path but NOT the
* provider endpoint so you must include it. This allows redirecting outside
* of your endpoint.
* Redirect to this path. This is constructed against the site base (not the
* provider's base).
*/
redirect?: string
/**
Expand Down Expand Up @@ -133,12 +132,16 @@ export interface HttpServerOptions {

export interface Route {
/**
* Base path part (in /test/path it would be "/test").
* Provider base path part (for /provider/base/path it would be /provider).
*/
providerBase: string
/**
* Base path part (for /provider/base/path it would be /base).
*/
base: string
/**
* Remaining part of the route (in /test/path it would be "/path"). It can be
* blank.
* Remaining part of the route after factoring out the base and provider base
* (for /provider/base/path it would be /path). It can be blank.
*/
requestPath: string
/**
Expand All @@ -161,7 +164,6 @@ interface ProviderRoute extends Route {

export interface HttpProviderOptions {
readonly auth: AuthType
readonly base: string
readonly commit: string
readonly password?: string
}
Expand Down Expand Up @@ -518,41 +520,51 @@ export class HttpServer {
/**
* Register a provider for a top-level endpoint.
*/
public registerHttpProvider<T extends HttpProvider>(endpoint: string, provider: HttpProvider0<T>): T
public registerHttpProvider<A1, T extends HttpProvider>(endpoint: string, provider: HttpProvider1<A1, T>, a1: A1): T
public registerHttpProvider<T extends HttpProvider>(endpoint: string | string[], provider: HttpProvider0<T>): T
public registerHttpProvider<A1, T extends HttpProvider>(
endpoint: string | string[],
provider: HttpProvider1<A1, T>,
a1: A1,
): T
public registerHttpProvider<A1, A2, T extends HttpProvider>(
endpoint: string,
endpoint: string | string[],
provider: HttpProvider2<A1, A2, T>,
a1: A1,
a2: A2,
): T
public registerHttpProvider<A1, A2, A3, T extends HttpProvider>(
endpoint: string,
endpoint: string | string[],
provider: HttpProvider3<A1, A2, A3, T>,
a1: A1,
a2: A2,
a3: A3,
): T
// eslint-disable-next-line @typescript-eslint/no-explicit-any
public registerHttpProvider(endpoint: string, provider: any, ...args: any[]): any {
endpoint = endpoint.replace(/^\/+|\/+$/g, "")
if (this.providers.has(`/${endpoint}`)) {
throw new Error(`${endpoint} is already registered`)
}
if (/\//.test(endpoint)) {
throw new Error(`Only top-level endpoints are supported (got ${endpoint})`)
}
public registerHttpProvider(endpoint: string | string[], provider: any, ...args: any[]): void {
const p = new provider(
{
auth: this.options.auth || AuthType.None,
base: `/${endpoint}`,
commit: this.options.commit,
password: this.options.password,
},
...args,
)
this.providers.set(`/${endpoint}`, p)
return p
const endpoints = (typeof endpoint === "string" ? [endpoint] : endpoint).map((e) => e.replace(/^\/+|\/+$/g, ""))
endpoints.forEach((endpoint) => {
if (/\//.test(endpoint)) {
throw new Error(`Only top-level endpoints are supported (got ${endpoint})`)
}
const existingProvider = this.providers.get(`/${endpoint}`)
this.providers.set(`/${endpoint}`, p)
if (existingProvider) {
logger.debug(`Overridding existing /${endpoint} provider`)
// If the existing provider isn't registered elsewhere we can dispose.
if (!Array.from(this.providers.values()).find((p) => p === existingProvider)) {
logger.debug(`Disposing existing /${endpoint} provider`)
existingProvider.dispose()
}
}
})
}

/**
Expand Down Expand Up @@ -759,15 +771,15 @@ export class HttpServer {
// that by shifting the next base out of the request path.
let provider = this.providers.get(base)
if (base !== "/" && provider) {
return { ...parse(requestPath), fullPath, query: parsedUrl.query, provider, originalPath }
return { ...parse(requestPath), providerBase: base, fullPath, query: parsedUrl.query, provider, originalPath }
}

// Fall back to the top-level provider.
provider = this.providers.get("/")
if (!provider) {
throw new Error(`No provider for ${base}`)
}
return { base, fullPath, requestPath, query: parsedUrl.query, provider, originalPath }
return { base, providerBase: "/", fullPath, requestPath, query: parsedUrl.query, provider, originalPath }
}

/**
Expand Down
1 change: 0 additions & 1 deletion test/update.test.ts
Original file line number Diff line number Diff line change
Expand Up @@ -51,7 +51,6 @@ describe("update", () => {
_provider = new UpdateHttpProvider(
{
auth: AuthType.None,
base: "/update",
commit: "test",
},
true,
Expand Down