Skip to content

Commit

Permalink
ui: Fix issue where relays with trailing slashes cannot be removed (#…
Browse files Browse the repository at this point in the history
…1531)

Summary
-------

This fixes the issue at Github #1531 where relays with trailing slashes cannot be removed.

The root cause (Identified by @fishcakeday) was that for a relay to be removed, a certain dictionary entry containing the relay url needed to be removed prior to sending the updated relay list. However those dictionary keys used `String` objects, which cannot tell that two URLs are the same with or without a trailing slash.

To fix the issue, I have used a dictionary with the type `[RelayURL: RelayInfo]`, and made the necessary protocol conformance implementations for RelayURL. This way, URLs are handled with higher accuracy (e.g. Trailing slashes do not matter, URLs that resolve to the same location will match no matter what).

This allows us to leverage the existing parsing and handling logic that comes with the `URL` type, instead of manually handling URL strings.

Generally speaking it is preferrable to work with higher level `URL` or `RelayURL` objects than handle URLs via `String`. There is an opportunity to refactor more code, but I intentionally kept the changes to `RelayURL` limited to the functionality in this issue, because otherwise the changeset becomes very big and risky.

Issue reproduction
------------------

**Device:** iPhone 14 Pro simulator
**iOS:** 17.0
**Damus:** Local build from `476f52562` with the following local change:
``` diff

Signed-off-by: William Casarin <[email protected]>
  • Loading branch information
danieldaquino authored and jb55 committed Oct 1, 2023
1 parent a2b7dfe commit 1607ee7
Show file tree
Hide file tree
Showing 9 changed files with 115 additions and 46 deletions.
4 changes: 4 additions & 0 deletions damus.xcodeproj/project.pbxproj
Original file line number Diff line number Diff line change
Expand Up @@ -427,6 +427,7 @@
D7870BC12AC4750B0080BA88 /* MentionView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7870BC02AC4750B0080BA88 /* MentionView.swift */; };
D7870BC32AC47EBC0080BA88 /* EventLoaderView.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7870BC22AC47EBC0080BA88 /* EventLoaderView.swift */; };
D7DEEF2F2A8C021E00E0C99F /* NostrEventTests.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7DEEF2E2A8C021E00E0C99F /* NostrEventTests.swift */; };
D7FF94002AC7AC5300FD969D /* RelayURL.swift in Sources */ = {isa = PBXBuildFile; fileRef = D7FF93FF2AC7AC5200FD969D /* RelayURL.swift */; };
E4FA1C032A24BB7F00482697 /* SearchSettingsView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E4FA1C022A24BB7F00482697 /* SearchSettingsView.swift */; };
E990020F2955F837003BBC5A /* EditMetadataView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E990020E2955F837003BBC5A /* EditMetadataView.swift */; };
E9E4ED0B295867B900DD7078 /* ThreadView.swift in Sources */ = {isa = PBXBuildFile; fileRef = E9E4ED0A295867B900DD7078 /* ThreadView.swift */; };
Expand Down Expand Up @@ -1109,6 +1110,7 @@
D7870BC02AC4750B0080BA88 /* MentionView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = MentionView.swift; sourceTree = "<group>"; };
D7870BC22AC47EBC0080BA88 /* EventLoaderView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EventLoaderView.swift; sourceTree = "<group>"; };
D7DEEF2E2A8C021E00E0C99F /* NostrEventTests.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = NostrEventTests.swift; sourceTree = "<group>"; };
D7FF93FF2AC7AC5200FD969D /* RelayURL.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = RelayURL.swift; sourceTree = "<group>"; };
E4FA1C022A24BB7F00482697 /* SearchSettingsView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = SearchSettingsView.swift; sourceTree = "<group>"; };
E990020E2955F837003BBC5A /* EditMetadataView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = EditMetadataView.swift; sourceTree = "<group>"; };
E9E4ED0A295867B900DD7078 /* ThreadView.swift */ = {isa = PBXFileReference; lastKnownFileType = sourcecode.swift; path = ThreadView.swift; sourceTree = "<group>"; };
Expand Down Expand Up @@ -1710,6 +1712,7 @@
50088DA029E8271A008A1FDF /* WebSocket.swift */,
4C28A4112A6D03D200C1A7A5 /* ReferencedId.swift */,
4C2B7BF12A71B6540049DEE7 /* Id.swift */,
D7FF93FF2AC7AC5200FD969D /* RelayURL.swift */,
);
path = Nostr;
sourceTree = "<group>";
Expand Down Expand Up @@ -2714,6 +2717,7 @@
3A23838E2A297DD200E5AA2E /* ZapButtonModel.swift in Sources */,
F71694F82A6983AF001F4053 /* GrayGradient.swift in Sources */,
4C1D4FB12A7958E60024F453 /* VersionInfo.swift in Sources */,
D7FF94002AC7AC5300FD969D /* RelayURL.swift in Sources */,
4C64305C2A945AFF00B0C0E9 /* MusicController.swift in Sources */,
5053ACA72A56DF3B00851AE3 /* DeveloperSettingsView.swift in Sources */,
F79C7FAD29D5E9620000F946 /* EditPictureControl.swift in Sources */,
Expand Down
21 changes: 12 additions & 9 deletions damus/Models/Contacts.swift
Original file line number Diff line number Diff line change
Expand Up @@ -184,8 +184,13 @@ func decode_json_relays(_ content: String) -> [String: RelayInfo]? {
return decode_json(content)
}

func remove_relay(ev: NostrEvent, current_relays: [RelayDescriptor], keypair: FullKeypair, relay: String) -> NostrEvent?{
func decode_json_relays(_ content: String) -> [RelayURL: RelayInfo]? {
return decode_json(content)
}

func remove_relay(ev: NostrEvent, current_relays: [RelayDescriptor], keypair: FullKeypair, relay: RelayURL) -> NostrEvent?{
var relays = ensure_relay_info(relays: current_relays, content: ev.content)

relays.removeValue(forKey: relay)

guard let content = encode_json(relays) else {
Expand All @@ -195,9 +200,10 @@ func remove_relay(ev: NostrEvent, current_relays: [RelayDescriptor], keypair: Fu
return NostrEvent(content: content, keypair: keypair.to_keypair(), kind: 3, tags: ev.tags.strings())
}

func add_relay(ev: NostrEvent, keypair: FullKeypair, current_relays: [RelayDescriptor], relay: String, info: RelayInfo) -> NostrEvent? {
func add_relay(ev: NostrEvent, keypair: FullKeypair, current_relays: [RelayDescriptor], relay: RelayURL, info: RelayInfo) -> NostrEvent? {
var relays = ensure_relay_info(relays: current_relays, content: ev.content)


guard relays.index(forKey: relay) == nil else {
return nil
}
Expand All @@ -211,11 +217,8 @@ func add_relay(ev: NostrEvent, keypair: FullKeypair, current_relays: [RelayDescr
return NostrEvent(content: content, keypair: keypair.to_keypair(), kind: 3, tags: ev.tags.strings())
}

func ensure_relay_info(relays: [RelayDescriptor], content: String) -> [String: RelayInfo] {
guard let relay_info = decode_json_relays(content) else {
return make_contact_relays(relays)
}
return relay_info
func ensure_relay_info(relays: [RelayDescriptor], content: String) -> [RelayURL: RelayInfo] {
return decode_json_relays(content) ?? make_contact_relays(relays)
}

func is_already_following(contacts: NostrEvent, follow: FollowRef) -> Bool {
Expand Down Expand Up @@ -245,8 +248,8 @@ func follow_with_existing_contacts(keypair: FullKeypair, our_contacts: NostrEven
return NostrEvent(content: our_contacts.content, keypair: keypair.to_keypair(), kind: kind, tags: tags)
}

func make_contact_relays(_ relays: [RelayDescriptor]) -> [String: RelayInfo] {
func make_contact_relays(_ relays: [RelayDescriptor]) -> [RelayURL: RelayInfo] {
return relays.reduce(into: [:]) { acc, relay in
acc[relay.url.url.absoluteString] = relay.info
acc[relay.url] = relay.info
}
}
2 changes: 1 addition & 1 deletion damus/Models/HomeModel.swift
Original file line number Diff line number Diff line change
Expand Up @@ -849,7 +849,7 @@ func load_our_relays(state: DamusState, m_old_ev: NostrEvent?, ev: NostrEvent) {
d[r] = .rw
}

guard let decoded = decode_json_relays(ev.content) else {
guard let decoded: [String: RelayInfo] = decode_json_relays(ev.content) else {
return
}

Expand Down
31 changes: 0 additions & 31 deletions damus/Nostr/RelayConnection.swift
Original file line number Diff line number Diff line change
Expand Up @@ -13,37 +13,6 @@ enum NostrConnectionEvent {
case nostr_event(NostrResponse)
}

public struct RelayURL: Hashable {
private(set) var url: URL

var id: String {
return url.absoluteString
}

init?(_ str: String) {
guard let last = str.last else { return nil }

var urlstr = str
if last == "/" {
urlstr = String(str.dropLast(1))
}

guard let url = URL(string: urlstr) else {
return nil
}

guard let scheme = url.scheme else {
return nil
}

guard scheme == "ws" || scheme == "wss" else {
return nil
}

self.url = url
}
}

final class RelayConnection: ObservableObject {
@Published private(set) var isConnected = false
@Published private(set) var isConnecting = false
Expand Down
87 changes: 87 additions & 0 deletions damus/Nostr/RelayURL.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,87 @@
//
// RelayURL.swift
// damus
//
// Created by Daniel D’Aquino on 2023-09-29.
//

import Foundation

public struct RelayURL: Hashable, Equatable, Codable, CodingKeyRepresentable {
private(set) var url: URL

var id: String {
return url.absoluteString
}

init?(_ str: String) {
guard let last = str.last else { return nil }

var urlstr = str
if last == "/" {
urlstr = String(str.dropLast(1))
}

guard let url = URL(string: urlstr) else {
return nil
}

guard let scheme = url.scheme else {
return nil
}

guard scheme == "ws" || scheme == "wss" else {
return nil
}

self.url = url
}

// MARK: - Codable
public init(from decoder: Decoder) throws {
let container = try decoder.singleValueContainer()
let urlString = try container.decode(String.self)
guard let instance = RelayURL(urlString) else {
throw DecodingError.dataCorruptedError(in: container, debugDescription: "Invalid URL string.")
}
self = instance
}

public func encode(to encoder: Encoder) throws {
var container = encoder.singleValueContainer()
try container.encode(url.absoluteString)
}

// MARK: - CodingKeyRepresentable
// CodingKeyRepresentable conformance is necessary to ensure that
// a dictionary with type "[RelayURL: T] where T: Codable" can be encoded into a keyed container
// e.g. `{<URL>: <VALUE>, <URL>: <VALUE>}` instead of `[<URL>, <VALUE>, <URL>, <VALUE>]`, which is Swift's default for non-string-keyed dictionaries

public var codingKey: CodingKey {
return StringKey(stringValue: self.url.absoluteString)
}

public init?<T>(codingKey: T) where T : CodingKey {
self.init(codingKey.stringValue)
}

// MARK: - Equatable
public static func == (lhs: RelayURL, rhs: RelayURL) -> Bool {
return lhs.url == rhs.url
}

// MARK: - Hashable
public func hash(into hasher: inout Hasher) {
hasher.combine(self.url)
}

}

private struct StringKey: CodingKey {
var stringValue: String
init(stringValue: String) {
self.stringValue = stringValue
}
var intValue: Int? { return nil }
init?(intValue: Int) { return nil }
}
4 changes: 3 additions & 1 deletion damus/Views/AddRelayView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -82,9 +82,11 @@ struct AddRelayView: View {
new_relay = "wss://" + new_relay
}

/*
if new_relay.hasSuffix("/") {
new_relay.removeLast();
}
*/

guard let url = RelayURL(new_relay),
let ev = state.contacts.event,
Expand All @@ -109,7 +111,7 @@ struct AddRelayView: View {

state.pool.connect(to: [new_relay])

guard let new_ev = add_relay(ev: ev, keypair: keypair, current_relays: state.pool.our_descriptors, relay: new_relay, info: info) else {
guard let new_ev = add_relay(ev: ev, keypair: keypair, current_relays: state.pool.our_descriptors, relay: url, info: info) else {
return
}

Expand Down
3 changes: 2 additions & 1 deletion damus/Views/Relays/RecommendedRelayView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -118,7 +118,8 @@ struct RecommendedRelayView: View {
guard let ev_before_add = damus.contacts.event else {
return
}
guard let ev_after_add = add_relay(ev: ev_before_add, keypair: keypair, current_relays: damus.pool.our_descriptors, relay: relay, info: .rw) else {
guard let relay_url = RelayURL(relay),
let ev_after_add = add_relay(ev: ev_before_add, keypair: keypair, current_relays: damus.pool.our_descriptors, relay: relay_url, info: .rw) else {
return
}
process_contact_event(state: damus, ev: ev_after_add)
Expand Down
6 changes: 4 additions & 2 deletions damus/Views/Relays/RelayDetailView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,8 @@ struct RelayDetailView: View {
}

let descriptors = state.pool.our_descriptors
guard let new_ev = remove_relay( ev: ev, current_relays: descriptors, keypair: keypair, relay: relay) else {
guard let relay_url = RelayURL(relay),
let new_ev = remove_relay( ev: ev, current_relays: descriptors, keypair: keypair, relay: relay_url) else {
return
}

Expand All @@ -71,7 +72,8 @@ struct RelayDetailView: View {
guard let ev_before_add = state.contacts.event else {
return
}
guard let ev_after_add = add_relay(ev: ev_before_add, keypair: keypair, current_relays: state.pool.our_descriptors, relay: relay, info: .rw) else {
guard let relay_url = RelayURL(relay),
let ev_after_add = add_relay(ev: ev_before_add, keypair: keypair, current_relays: state.pool.our_descriptors, relay: relay_url, info: .rw) else {
return
}
process_contact_event(state: state, ev: ev_after_add)
Expand Down
3 changes: 2 additions & 1 deletion damus/Views/Relays/RelayView.swift
Original file line number Diff line number Diff line change
Expand Up @@ -95,7 +95,8 @@ struct RelayView: View {

let descriptors = state.pool.our_descriptors
guard let keypair = state.keypair.to_full(),
let new_ev = remove_relay(ev: ev, current_relays: descriptors, keypair: keypair, relay: relay) else {
let relay_url = RelayURL(relay),
let new_ev = remove_relay(ev: ev, current_relays: descriptors, keypair: keypair, relay: relay_url) else {
return
}

Expand Down

0 comments on commit 1607ee7

Please sign in to comment.