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

Add support for arbitrary single-letter tags in Filter #154

Merged
merged 5 commits into from
May 19, 2024
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
113 changes: 94 additions & 19 deletions Sources/NostrSDK/Filter.swift
Original file line number Diff line number Diff line change
Expand Up @@ -19,13 +19,10 @@ public struct Filter: Codable, Hashable, Equatable {

/// a list of a kind numbers
public let kinds: [Int]?

/// a list of event ids that are referenced in an "e" tag
public let events: [String]?

/// a list of pubkeys that are referenced in a "p" tag
public let pubkeys: [String]?


/// a list of tag values that are referenced by single English-alphabet letter tag names
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// a list of tag values that are referenced by single English-alphabet letter tag names
/// a list of tag values that are referenced by single Latin-script letter tag names

https://en.wikipedia.org/wiki/Latin_script

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to "basic Latin letter" instead.
https://en.wikipedia.org/wiki/ISO_basic_Latin_alphabet

public let tags: [Character: [String]]?

/// an integer unix timestamp, events must be newer than this to pass
public let since: Int?

Expand All @@ -36,16 +33,28 @@ public struct Filter: Codable, Hashable, Equatable {
public let limit: Int?

private enum CodingKeys: String, CodingKey {
case ids = "ids"
case authors = "authors"
case kinds = "kinds"
case events = "#e"
case pubkeys = "#p"
case since = "since"
case until = "until"
case limit = "limit"
case ids
case authors
case kinds
case since
case until
case limit
}


private struct TagFilterName: CodingKey {
var stringValue: String
tyiu marked this conversation as resolved.
Show resolved Hide resolved

init(stringValue: String) {
self.stringValue = stringValue
}

var intValue: Int? { nil }

init?(intValue: Int) {
Copy link
Collaborator

Choose a reason for hiding this comment

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

What's the purpose of an initializer that always returns nil?

Copy link
Contributor Author

@tyiu tyiu May 19, 2024

Choose a reason for hiding this comment

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

Its declaration is required by the CodingKey protocol. The compiler doesn't like it if it's not there. Per the documentation of this initializer, it says

    /// Creates a new instance from the specified integer.
    ///
    /// If the value passed as `intValue` does not correspond to any instance of
    /// this type, the result is `nil`.
    ///
    /// - parameter intValue: The integer value of the desired key.
    init?(intValue: Int)

return nil
}
}

/// Creates and returns a filter with the specified parameters.
///
/// - Parameters:
Expand All @@ -54,19 +63,85 @@ public struct Filter: Codable, Hashable, Equatable {
/// - kinds: a list of a kind numbers
/// - events: a list of event ids that are referenced in an "e" tag
/// - pubkeys: a list of pubkeys that are referenced in a "p" tag
/// - tags: a list of tag values that are referenced by single English-alphabet letter tag names
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// - tags: a list of tag values that are referenced by single English-alphabet letter tag names
/// - tags: a list of tag values that are referenced by single Latin-script letter tag names

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to "basic Latin letter" instead.
https://en.wikipedia.org/wiki/ISO_basic_Latin_alphabet

/// - since: an integer unix timestamp, events must be newer than this to pass
/// - until: an integer unix timestamp, events must be older than this to pass
/// - limit: maximum number of events to be returned in the initial query
///
/// If `tags` contains an `e` tag and `events` is also provided, `events` takes precedence.
bryanmontz marked this conversation as resolved.
Show resolved Hide resolved
/// If `tags` contains a `p` tag and `pubkeys` is also provided, `pubkeys` takes precedence.
///
/// Returns `nil` if `tags` contains tag names that are not in the English-alphabet of A-Z or a-z.
Copy link
Collaborator

Choose a reason for hiding this comment

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

Suggested change
/// Returns `nil` if `tags` contains tag names that are not in the English-alphabet of A-Z or a-z.
/// Returns `nil` if `tags` contains tag names that are not in the Latin script of A-Z or a-z.

Copy link
Contributor Author

Choose a reason for hiding this comment

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

Changed it to "basic Latin letter" instead.
https://en.wikipedia.org/wiki/ISO_basic_Latin_alphabet

///
/// > Important: Event ids and pubkeys should be in the 32-byte hexadecimal format, not the `note...` and `npub...` formats.
public init(ids: [String]? = nil, authors: [String]? = nil, kinds: [Int]? = nil, events: [String]? = nil, pubkeys: [String]? = nil, since: Int? = nil, until: Int? = nil, limit: Int? = nil) {
public init?(ids: [String]? = nil, authors: [String]? = nil, kinds: [Int]? = nil, events: [String]? = nil, pubkeys: [String]? = nil, tags: [Character: [String]]? = nil, since: Int? = nil, until: Int? = nil, limit: Int? = nil) {
self.ids = ids
self.authors = authors
self.kinds = kinds
self.events = events
self.pubkeys = pubkeys
self.since = since
self.until = until
self.limit = limit

if let tags {
guard tags.keys.allSatisfy({ $0.isEnglishLetter }) else {
return nil
}
}

var tagsBuilder: [Character: [String]] = tags ?? [:]
if let events {
tagsBuilder["e"] = events
}
if let pubkeys {
tagsBuilder["p"] = pubkeys
}
self.tags = tagsBuilder
}

public init(from decoder: Decoder) throws {
let container = try decoder.container(keyedBy: CodingKeys.self)

ids = try container.decodeIfPresent([String].self, forKey: .ids)
authors = try container.decodeIfPresent([String].self, forKey: .authors)
kinds = try container.decodeIfPresent([Int].self, forKey: .kinds)
since = try container.decodeIfPresent(Int.self, forKey: .since)
until = try container.decodeIfPresent(Int.self, forKey: .until)
limit = try container.decodeIfPresent(Int.self, forKey: .limit)

if let tagsContainer = try? decoder.container(keyedBy: TagFilterName.self) {
var decodedTags: [Character: [String]] = [:]
for key in tagsContainer.allKeys {
let tagName = key.stringValue

if tagName.count == 2 && tagName.first == "#", let tagCharacter = tagName.last, tagCharacter.isEnglishLetter {
decodedTags[tagCharacter] = try tagsContainer.decode([String].self, forKey: key)
}
}
tags = decodedTags.isEmpty ? nil : decodedTags
} else {
tags = nil
}
}

public func encode(to encoder: Encoder) throws {
var container = encoder.container(keyedBy: CodingKeys.self)

try container.encodeIfPresent(self.ids, forKey: .ids)
try container.encodeIfPresent(self.authors, forKey: .authors)
try container.encodeIfPresent(self.kinds, forKey: .kinds)
try container.encodeIfPresent(self.since, forKey: .since)
try container.encodeIfPresent(self.until, forKey: .until)
try container.encodeIfPresent(self.limit, forKey: .limit)

var tagsContainer = encoder.container(keyedBy: TagFilterName.self)
try self.tags?.forEach {
try tagsContainer.encode($0.value, forKey: TagFilterName(stringValue: "#\($0.key)"))
}
}
}

private extension Character {
var isEnglishLetter: Bool {
Copy link
Collaborator

Choose a reason for hiding this comment

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

IMO we should generally use "Latin script" or "Latin alphabet" rather than "English letter" because it is more accurate. There are many languages that use the Latin alphabet of A-Z.

Copy link
Contributor Author

@tyiu tyiu May 19, 2024

Choose a reason for hiding this comment

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

I think "Latin script" might be too broad.

The Latin script is the basis of the International Phonetic Alphabet, and the 26 most widespread letters are the letters contained in the ISO basic Latin alphabet, which are the same letters as the English alphabet.

Maybe isBasicLatinLetter is better. I chose Letter over Alphabet or Script because Swift already has an isLetter property, so making the naming consistent to that makes more sense.

(self >= "A" && self <= "Z") || (self >= "a" && self <= "z")
}
}
45 changes: 0 additions & 45 deletions Tests/NostrSDKTests/FilterEncodingTests.swift

This file was deleted.

78 changes: 78 additions & 0 deletions Tests/NostrSDKTests/FilterTests.swift
Original file line number Diff line number Diff line change
@@ -0,0 +1,78 @@
//
// FilterTests.swift
//
//
// Created by Joel Klabo on 5/26/23.
//

import NostrSDK
import XCTest

final class FilterTests: XCTestCase, FixtureLoading, JSONTesting {

func testFilterEncoding() throws {
let filter = Filter(authors: ["d9fa34214aa9d151c4f4db843e9c2af4f246bab4205137731f91bcfa44d66a62"],
kinds: [3],
limit: 1)

let expected = try loadFixtureString("filter")

let encoder = JSONEncoder()
let result = try encoder.encode(filter)
let resultString = String(decoding: result, as: UTF8.self)

XCTAssertTrue(areEquivalentJSONObjectStrings(expected, resultString))
}

func testFilterWithAllFieldsEncoding() throws {
let filter = Filter(ids: ["pubkey1"],
authors: ["author1", "author2"],
kinds: [1, 2, 3],
events: ["event1", "event2"],
pubkeys: ["referencedPubkey1"],
tags: ["t": ["hashtag"], "e": ["thisEventFilterIsDiscarded"], "p": ["thisPubkeyFilterIsDiscarded"]],
since: 1234,
until: 12345,
limit: 5)

let expected = try loadFixtureString("filter_all_fields")

let encoder = JSONEncoder()
let result = try encoder.encode(filter)
let resultString = String(decoding: result, as: UTF8.self)

XCTAssertTrue(areEquivalentJSONObjectStrings(expected, resultString))
}

func testFilterWithInvalidTagsEncoding() throws {
XCTAssertNil(Filter(tags: ["*": []]))
}

func testFilterDecoding() throws {
let expectedFilter = Filter(ids: ["pubkey1"],
authors: ["author1", "author2"],
kinds: [1, 2, 3],
events: ["event1", "event2"],
pubkeys: ["referencedPubkey1"],
tags: ["t": ["hashtag"]],
since: 1234,
until: 12345,
limit: 5)
let filter: Filter = try decodeFixture(filename: "filter_all_fields")
XCTAssertEqual(expectedFilter, filter)
}

func testFilterWithExtraFieldsDecoding() throws {
let expectedFilter = Filter(ids: ["pubkey1"],
authors: ["author1", "author2"],
kinds: [1, 2, 3],
events: ["event1", "event2"],
pubkeys: ["referencedPubkey1"],
tags: ["t": ["hashtag"]],
since: 1234,
until: 12345,
limit: 5)
let filter: Filter = try decodeFixture(filename: "filter_with_extra_fields")
XCTAssertEqual(expectedFilter, filter)
}
}
1 change: 1 addition & 0 deletions Tests/NostrSDKTests/Fixtures/filter_all_fields.json
Original file line number Diff line number Diff line change
Expand Up @@ -4,6 +4,7 @@
"kinds": [1, 2, 3],
"#e": ["event1", "event2"],
"#p": ["referencedPubkey1"],
"#t": ["hashtag"],
"since": 1234,
"until": 12345,
"limit": 5
Expand Down
13 changes: 13 additions & 0 deletions Tests/NostrSDKTests/Fixtures/filter_with_extra_fields.json
Original file line number Diff line number Diff line change
@@ -0,0 +1,13 @@
{
"ids": ["pubkey1"],
"authors": ["author1", "author2"],
"kinds": [1, 2, 3],
"#e": ["event1", "event2"],
"#p": ["referencedPubkey1"],
"#t": ["hashtag"],
"since": 1234,
"until": 12345,
"limit": 5,
"#unrecognized": ["unrecognized"],
"unrecognized": 123
}
38 changes: 22 additions & 16 deletions Tests/NostrSDKTests/RelayRequestEncodingTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -35,14 +35,17 @@ final class RelayRequestEncodingTests: XCTestCase, FixtureLoading, JSONTesting {
}

func testEncodeCount() throws {
let filter = Filter(ids: nil,
authors: ["some-pubkey"],
kinds: [1, 7],
events: nil,
pubkeys: nil,
since: nil,
until: nil,
limit: nil)
let filter = try XCTUnwrap(
Filter(ids: nil,
authors: ["some-pubkey"],
kinds: [1, 7],
events: nil,
pubkeys: nil,
since: nil,
until: nil,
limit: nil
)
)

let request = try XCTUnwrap(RelayRequest.count(subscriptionId: "some-subscription-id", filter: filter), "failed to encode request")
let expected = try loadFixtureString("count_request")
Expand All @@ -51,14 +54,17 @@ final class RelayRequestEncodingTests: XCTestCase, FixtureLoading, JSONTesting {
}

func testEncodeReq() throws {
let filter = Filter(ids: nil,
authors: ["some-pubkey"],
kinds: [1, 7],
events: nil,
pubkeys: nil,
since: nil,
until: nil,
limit: nil)
let filter = try XCTUnwrap(
Filter(ids: nil,
authors: ["some-pubkey"],
kinds: [1, 7],
events: nil,
pubkeys: nil,
since: nil,
until: nil,
limit: nil
)
)

let request = try XCTUnwrap(RelayRequest.request(subscriptionId: "some-subscription-id", filter: filter), "failed to encode request")
let expected = try loadFixtureString("req")
Expand Down
9 changes: 6 additions & 3 deletions Tests/NostrSDKTests/RelayTests.swift
Original file line number Diff line number Diff line change
Expand Up @@ -44,7 +44,8 @@ final class RelayTests: XCTestCase {

wait(for: [connectExpectation!], timeout: 10)

let subscriptionId = try relay.subscribe(with: Filter(kinds: [1], limit: 1))
let filter = try XCTUnwrap(Filter(kinds: [1], limit: 1))
let subscriptionId = try relay.subscribe(with: filter)

relay.events
.sink { [unowned relay] _ in
Expand All @@ -65,7 +66,8 @@ final class RelayTests: XCTestCase {

func testSubscribeWithoutConnection() throws {
let relay = try Relay(url: RelayTests.RelayURL)
XCTAssertThrowsError(try relay.subscribe(with: Filter(kinds: [1], limit: 1))) {
let filter = try XCTUnwrap(Filter(kinds: [1], limit: 1))
XCTAssertThrowsError(try relay.subscribe(with: filter)) {
XCTAssertEqual($0 as? RelayRequestError, RelayRequestError.notConnected)
}
}
Expand All @@ -88,7 +90,8 @@ final class RelayTests: XCTestCase {

wait(for: [connectExpectation!], timeout: 10)

let subscriptionId = try relay.subscribe(with: Filter(kinds: [1], limit: 1))
let filter = try XCTUnwrap(Filter(kinds: [1], limit: 1))
let subscriptionId = try relay.subscribe(with: filter)

wait(for: [receiveExpectation!], timeout: 10)

Expand Down
Loading