Skip to content
Open
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
13 changes: 9 additions & 4 deletions packages/binding-http/src/http-client-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -28,20 +28,24 @@ export default class HttpClientFactory implements ProtocolClientFactory {
public readonly scheme: string = "http";
private config: HttpConfig | null = null;
private oAuthManager: OAuthManager = new OAuthManager();
private readonly clients = new Set<ProtocolClient>();

Copy link
Copy Markdown
Member

Choose a reason for hiding this comment

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

I have a question about why using Set instead of an array. I wonder whether it can happen that a HttpClient may be treated as duplicate... hence creating for example 2 HttpClient instances but having just one entry in the set...

Copy link
Copy Markdown
Author

Choose a reason for hiding this comment

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

@danielpeintner , I used a Set to avoid accidentally tracking the same instance more than once, but I’m happy to use an array for consistency with MqttClientFactory if you prefer.


constructor(config: HttpConfig | null = null) {
this.config = config;
}

public getClient(): ProtocolClient {
let client: HttpClient;
// HTTP over HTTPS proxy requires HttpsClient
if (this.config && this.config.proxy && this.config.proxy.href && this.config.proxy.href.startsWith("https:")) {
warn("HttpClientFactory creating client for 'https' due to secure proxy configuration");
return new HttpClient(this.config, true, this.oAuthManager);
client = new HttpClient(this.config, true, this.oAuthManager);
} else {
debug(`HttpClientFactory creating client for '${this.scheme}'`);
return new HttpClient(this.config);
client = new HttpClient(this.config);
}
this.clients.add(client);
return client;
}

public init(): boolean {
Expand All @@ -51,8 +55,9 @@ export default class HttpClientFactory implements ProtocolClientFactory {
}

public destroy(): boolean {
// info(`HttpClientFactory for '${HttpClientFactory.scheme}' destroyed`);
// TODO uncomment info if something is executed here
debug(`HttpClientFactory stopping all clients for '${this.scheme}'`);
this.clients.forEach((client) => void client.stop());
this.clients.clear();
return true;
}
}
4 changes: 4 additions & 0 deletions packages/binding-http/src/http-client-impl.ts
Original file line number Diff line number Diff line change
Expand Up @@ -183,6 +183,7 @@ export default class HttpClient implements ProtocolClient {
this.activeSubscriptions.set(form.href, internalSubscription);
return new Subscription(() => {
internalSubscription.close();
this.activeSubscriptions.delete(form.href);
});
}

Expand Down Expand Up @@ -226,6 +227,7 @@ export default class HttpClient implements ProtocolClient {

if (internalSub) {
internalSub.close();
this.activeSubscriptions.delete(form.href);
} else {
warn(`HttpClient cannot unlink ${form.href} no subscription found`);
}
Expand All @@ -249,6 +251,8 @@ export default class HttpClient implements ProtocolClient {
}

public async stop(): Promise<void> {
this.activeSubscriptions.forEach((subscription) => subscription.close());
this.activeSubscriptions.clear();
// When running in browser mode, Agent.destroy() might not exist.
this.agent?.destroy?.();
}
Expand Down
13 changes: 9 additions & 4 deletions packages/binding-http/src/https-client-factory.ts
Original file line number Diff line number Diff line change
Expand Up @@ -26,20 +26,24 @@ const { debug, warn } = createLoggers("binding-http", "https-client-factory");
export default class HttpsClientFactory implements ProtocolClientFactory {
public readonly scheme: string = "https";
private config: HttpConfig | null = null;
private readonly clients = new Set<ProtocolClient>();

constructor(config: HttpConfig | null = null) {
this.config = config;
}

public getClient(): ProtocolClient {
let client: HttpClient;
// HTTPS over HTTP proxy requires HttpClient
if (this.config && this.config.proxy && this.config.proxy.href && this.config.proxy.href.startsWith("http:")) {
warn("HttpsClientFactory creating client for 'http' due to insecure proxy configuration");
return new HttpClient(this.config);
client = new HttpClient(this.config);
} else {
debug(`HttpsClientFactory creating client for '${this.scheme}'`);
return new HttpClient(this.config, true);
client = new HttpClient(this.config, true);
}
this.clients.add(client);
return client;
}

public init(): boolean {
Expand All @@ -49,8 +53,9 @@ export default class HttpsClientFactory implements ProtocolClientFactory {
}

public destroy(): boolean {
// info(`HttpsClientFactory for '${HttpsClientFactory.scheme}' destroyed`);
// TODO uncomment info if something is executed here
debug(`HttpsClientFactory stopping all clients for '${this.scheme}'`);
this.clients.forEach((client) => void client.stop());
this.clients.clear();
return true;
}
}
83 changes: 83 additions & 0 deletions packages/binding-http/test/http-client-factory-test.ts
Original file line number Diff line number Diff line change
@@ -0,0 +1,83 @@
/********************************************************************************
* Copyright (c) 2026 Contributors to the Eclipse Foundation
*
* See the NOTICE file(s) distributed with this work for additional
* information regarding copyright ownership.
*
* This program and the accompanying materials are made available under the
* terms of the Eclipse Public License v. 2.0 which is available at
* http://www.eclipse.org/legal/epl-2.0, or the W3C Software Notice and
* Document License (2015-05-13) which is available at
* https://www.w3.org/Consortium/Legal/2015/copyright-software-and-document.
*
* SPDX-License-Identifier: EPL-2.0 OR W3C-20150513
********************************************************************************/

import { suite, test } from "@testdeck/mocha";
import chai, { expect, should } from "chai";
import spies from "chai-spies";

import HttpClientFactory from "../src/http-client-factory";
import HttpsClientFactory from "../src/https-client-factory";
import HttpClient from "../src/http-client";

should();
chai.use(spies);

@suite("HTTP client lifecycle")
class HttpClientLifecycleTest {
@test "HTTP factory destruction stops every created client"() {
const factory = new HttpClientFactory();
const firstClient = factory.getClient();
const secondClient = factory.getClient();
const firstStop = chai.spy.on(firstClient, "stop");
const secondStop = chai.spy.on(secondClient, "stop");

expect(factory.destroy()).to.equal(true);

firstStop.should.have.been.called.once;
secondStop.should.have.been.called.once;
}

@test "HTTPS factory destruction stops every created client"() {
const factory = new HttpsClientFactory();
const firstClient = factory.getClient();
const secondClient = factory.getClient();
const firstStop = chai.spy.on(firstClient, "stop");
const secondStop = chai.spy.on(secondClient, "stop");

expect(factory.destroy()).to.equal(true);

firstStop.should.have.been.called.once;
secondStop.should.have.been.called.once;
}

@test async "client stop closes and clears active subscriptions"() {
const client = new HttpClient();
const close = chai.spy();
client["activeSubscriptions"].set("http://example.test/observation", {
open: async () => undefined,
close,
});

await client.stop();

close.should.have.been.called.once;
expect(client["activeSubscriptions"].size).to.equal(0);
}

@test async "unlink closes and forgets the active subscription"() {
const client = new HttpClient();
const close = chai.spy();
const href = "http://example.test/observation";
client["activeSubscriptions"].set(href, {
open: async () => undefined,
close,
});

await client.unlinkResource({ href });

close.should.have.been.called.once;
expect(client["activeSubscriptions"].has(href)).to.equal(false);
}
}
Loading