git » sdk » commit c84690c

Make Channel#bookmark use Client#publishWithOptions

author Stephen Paul Weber
2026-09-30 20:23:11 UTC
committer Stephen Paul Weber
2026-09-30 20:23:11 UTC
parent f3ad18fdb5b8de1782dc9674d76cc5c0230b964c

Make Channel#bookmark use Client#publishWithOptions

Fixes the nesting problem and is just better

borogove/Chat.hx +4 -33
test/TestChat.hx +120 -0

diff --git a/borogove/Chat.hx b/borogove/Chat.hx
index 4082999..1a4bc8c 100644
--- a/borogove/Chat.hx
+++ b/borogove/Chat.hx
@@ -2528,17 +2528,16 @@ class Channel extends Chat {
 	@HaxeCBridge.noemit // on superclass as abstract
 	public function bookmark() {
 		if (uiState == Invited) uiState = Open;
-		stream.sendIq(
+
+		client.publishWithOptions(
 			new Stanza("iq", { type: "set" })
 				.tag("pubsub", { xmlns: "http://jabber.org/protocol/pubsub" })
 				.tag("publish", { node: "urn:xmpp:bookmarks:1" })
 				.tag("item", { id: chatId })
 				.tag("conference", { xmlns: "urn:xmpp:bookmarks:1", name: getDisplayName(), autojoin: uiState == Closed || uiState == Invited ? "false" : "true" })
 				.textTag("nick", client.displayName()) // Redundant but some other clients want it
-				.addChild(extensions)
-				.up().up()
-				.tag("publish-options")
-				.tag("x", { xmlns: "jabber:x:data", type: "submit" })
+				.addChild(extensions),
+			new Stanza("x", { xmlns: "jabber:x:data", type: "submit" })
 				.tag("field", { "var": "FORM_TYPE", type: "hidden" }).textTag("value", "http://jabber.org/protocol/pubsub#publish-options").up()
 				.tag("field", { "var": "pubsub#persist_items" }).textTag("value", "true").up()
 				.tag("field", { "var": "pubsub#max_items" }).textTag("value", "max").up()
@@ -2546,34 +2545,6 @@ class Channel extends Chat {
 				.tag("field", { "var": "pubsub#access_model" }).textTag("value", "whitelist").up()
 				.tag("field", { "var": "pubsub#notify_delete" }).textTag("value", "true").up()
 				.tag("field", { "var": "pubsub#notify_retract" }).textTag("value", "true").up()
-				.up().up().up().up(),
-			(response) -> {
-				if (response.attr.get("type") == "error") {
-					final preconditionError = response.getChild("error")?.getChild("precondition-not-met", "http://jabber.org/protocol/pubsub#errors");
-					if (preconditionError != null) {
-						// publish options failed, so force them to be right, what a silly workflow
-						stream.sendIq(
-							new Stanza("iq", { type: "set" })
-								.tag("pubsub", { xmlns: "http://jabber.org/protocol/pubsub#owner" })
-								.tag("configure", { node: "urn:xmpp:bookmarks:1" })
-								.tag("x", { xmlns: "jabber:x:data", type: "submit" })
-								.tag("field", { "var": "FORM_TYPE", type: "hidden" }).textTag("value", "http://jabber.org/protocol/pubsub#publish-options").up()
-								.tag("field", { "var": "pubsub#persist_items" }).textTag("value", "true").up()
-								.tag("field", { "var": "pubsub#max_items" }).textTag("value", "max").up()
-								.tag("field", { "var": "pubsub#send_last_published_item" }).textTag("value", "never").up()
-								.tag("field", { "var": "pubsub#access_model" }).textTag("value", "whitelist").up()
-								.tag("field", { "var": "pubsub#notify_delete" }).textTag("value", "true").up()
-								.tag("field", { "var": "pubsub#notify_retract" }).textTag("value", "true").up()
-								.up().up().up(),
-							(response) -> {
-								if (response.attr.get("type") == "result") {
-									bookmark();
-								}
-							}
-						);
-					}
-				}
-			}
 		);
 	}
 
diff --git a/test/TestChat.hx b/test/TestChat.hx
index e7758a0..615a8b6 100644
--- a/test/TestChat.hx
+++ b/test/TestChat.hx
@@ -14,6 +14,7 @@ import borogove.Caps.Identity;
 import borogove.Member;
 import borogove.Role;
 import thenshim.Promise;
+using Lambda;
 
 @:access(borogove)
 class TestChat extends utest.Test {
@@ -55,6 +56,125 @@ class TestChat extends utest.Test {
 		chat.bookmark();
 	}
 
+	public function testChannelBookmark(async: Async) {
+		final persistence = new Dummy();
+		final client = new Client("test@example.com", persistence);
+		final channel = new Channel(client, client.stream, persistence, "channel@example.com");
+
+		client.stream.on("sendStanza", (stanza: Stanza) -> {
+			if (stanza.name == "iq" && stanza.attr.get("type") == "set") {
+				final pubsub = stanza.getChild("pubsub", "http://jabber.org/protocol/pubsub");
+				if (pubsub != null) {
+					final publish = pubsub.getChild("publish");
+					Assert.notNull(publish);
+					Assert.equals("urn:xmpp:bookmarks:1", publish.attr.get("node"));
+					Assert.isNull(publish.getChild("publish-options"), "publish-options must not be a child of publish");
+
+					final item = publish.getChild("item");
+					Assert.notNull(item);
+					Assert.equals("channel@example.com", item.attr.get("id"));
+
+					final conference = item.getChild("conference", "urn:xmpp:bookmarks:1");
+					Assert.notNull(conference);
+					Assert.equals("channel@example.com", conference.attr.get("name"));
+					Assert.equals("true", conference.attr.get("autojoin"));
+					Assert.equals("test", conference.getChildText("nick"));
+
+					final publishOptions = pubsub.getChild("publish-options");
+					Assert.notNull(publishOptions, "publish-options must be a child of pubsub");
+
+					final x = publishOptions.getChild("x", "jabber:x:data");
+					Assert.notNull(x);
+					Assert.equals("submit", x.attr.get("type"));
+
+					final fields = x.allTags("field");
+					final formType = fields.find(f -> f.attr.get("var") == "FORM_TYPE");
+					Assert.notNull(formType);
+					Assert.equals("http://jabber.org/protocol/pubsub#publish-options", formType.getChildText("value"));
+
+					final persistItems = fields.find(f -> f.attr.get("var") == "pubsub#persist_items");
+					Assert.notNull(persistItems);
+					Assert.equals("true", persistItems.getChildText("value"));
+
+					final maxItems = fields.find(f -> f.attr.get("var") == "pubsub#max_items");
+					Assert.notNull(maxItems);
+					Assert.equals("max", maxItems.getChildText("value"));
+
+					final sendLast = fields.find(f -> f.attr.get("var") == "pubsub#send_last_published_item");
+					Assert.notNull(sendLast);
+					Assert.equals("never", sendLast.getChildText("value"));
+
+					final accessModel = fields.find(f -> f.attr.get("var") == "pubsub#access_model");
+					Assert.notNull(accessModel);
+					Assert.equals("whitelist", accessModel.getChildText("value"));
+
+					final notifyDelete = fields.find(f -> f.attr.get("var") == "pubsub#notify_delete");
+					Assert.notNull(notifyDelete);
+					Assert.equals("true", notifyDelete.getChildText("value"));
+
+					final notifyRetract = fields.find(f -> f.attr.get("var") == "pubsub#notify_retract");
+					Assert.notNull(notifyRetract);
+					Assert.equals("true", notifyRetract.getChildText("value"));
+
+					async.done();
+					return EventHandled;
+				}
+			}
+			return EventUnhandled;
+		});
+
+		channel.bookmark();
+	}
+
+	public function testChannelBookmarkPreconditionNotMet(async: Async) {
+		final persistence = new Dummy();
+		final client = new Client("test@example.com", persistence);
+		final channel = new Channel(client, client.stream, persistence, "channel@example.com");
+
+		var step = 0;
+		client.stream.on("sendStanza", (stanza: Stanza) -> {
+			if (stanza.name == "iq" && stanza.attr.get("type") == "set") {
+				if (step == 0) {
+					final pubsub = stanza.getChild("pubsub", "http://jabber.org/protocol/pubsub");
+					if (pubsub != null && pubsub.getChild("publish") != null) {
+						step = 1;
+						final id = stanza.attr.get("id");
+						client.stream.onStanza(
+							new Stanza("iq", { xmlns: "jabber:client", type: "error", id: id })
+								.tag("error", { type: "cancel" })
+								.tag("precondition-not-met", { xmlns: "http://jabber.org/protocol/pubsub#errors" })
+								.up().up()
+						);
+						return EventHandled;
+					}
+				} else if (step == 1) {
+					final owner = stanza.getChild("pubsub", "http://jabber.org/protocol/pubsub#owner");
+					if (owner != null) {
+						step = 2;
+						final configure = owner.getChild("configure");
+						Assert.notNull(configure);
+						Assert.equals("urn:xmpp:bookmarks:1", configure.attr.get("node"));
+						Assert.notNull(configure.getChild("x", "jabber:x:data"));
+						final id = stanza.attr.get("id");
+						client.stream.onStanza(
+							new Stanza("iq", { xmlns: "jabber:client", type: "result", id: id })
+						);
+						return EventHandled;
+					}
+				} else if (step == 2) {
+					final pubsub = stanza.getChild("pubsub", "http://jabber.org/protocol/pubsub");
+					if (pubsub != null && pubsub.getChild("publish") != null) {
+						async.done();
+						return EventHandled;
+					}
+				}
+			}
+			return EventUnhandled;
+		});
+
+		channel.bookmark();
+	}
+
 	public function testGetMessagesBeforeNull(async: Async) {
 		final persistence = new Dummy();
 		final client = new Client("test@example.com", persistence);