fix: tagged string subrange calculation crash (#661)

* Tag parsing algorithm improvements: support of nested tags, unexpected extra tags, single tags. 
* Add convenient functions for extracting tagged/untagged substrings
* Add documentation for TaggedString
* Add exhaustive TaggedString unit tests

Author: VladislavFitz

Sources/AlgoliaSearchClient/Models/Search/Response/SearchResponse/Auxiliary/Highlighting/TaggedString.swift

@@ -7,20 +7,43 @@
 
 import Foundation
 
+/// Structure embedding the calculation of tagged substring in the input string
 public struct TaggedString {
 
+  /// Input string
   public let input: String
+  
+  /// Opening tag
   public let preTag: String
+  
+  /// Closing tag
   public let postTag: String
+  
+  /// String comparison options for tags parsing
   public let options: String.CompareOptions
 
   private lazy var storage: (String, [Range<String.Index>]) = TaggedString.computeRanges(for: input, preTag: preTag, postTag: postTag, options: options)
 
+  /// Output string cleansed of the opening and closing tags
   public private(set) lazy var output: String = { storage.0 }()
+  
+  /// List of ranges of tagged substrings in the output string
   public private(set) lazy var taggedRanges: [Range<String.Index>] = { storage.1 }()
+  
+  /// List of ranges of untagged substrings in the output string
   public private(set) lazy var untaggedRanges: [Range<String.Index>] = TaggedString.computeInvertedRanges(for: output, with: taggedRanges)
 
-  public init(string: String, preTag: String, postTag: String, options: String.CompareOptions = []) {
+  /**
+  - Parameters:
+    - string: Input string
+    - preTag: Opening tag
+    - postTag: Closing tag
+    - options: String comparison options for tags parsing
+   */
+  public init(string: String,
+              preTag: String,
+              postTag: String,
+              options: String.CompareOptions = []) {
     // This string reconstruction is here to avoid a potential problems due to string encoding
     // Check unit test TaggedStringTests -> testWithDecodedString
     let string = String(string.indices.map { string [$0] })
@@ -29,45 +52,107 @@ public struct TaggedString {
     self.postTag = postTag
     self.options = options
   }
+  
+  /// - Returns: The list of tagged substrings of the output string
+  public mutating func taggedSubstrings() -> [Substring] {
+    return taggedRanges.map { output[$0] }
+  }
+  
+  /// - Returns: The list of untagged substrings of the output string
+  public mutating func untaggedSubstrings() -> [Substring] {
+    return untaggedRanges.map { output[$0] }
+  }
 
   private static func computeRanges(for string: String, preTag: String, postTag: String, options: String.CompareOptions) -> (output: String, ranges: [Range<String.Index>]) {
+    var output = string
+    var preStack: [R] = []
+    var rangesToHighlight = [R]()
+    
+    typealias R = Range<String.Index>
 
-    var newText = string
-    var rangesToHighlight = [Range<String.Index>]()
-
-    while let matchBegin = newText.range(of: preTag, options: options) {
-      newText.removeSubrange(matchBegin)
-      let tailRange = matchBegin.lowerBound..<newText.endIndex
-      guard let matchEnd = newText.range(of: postTag, options: options, range: tailRange) else { continue }
-      newText.removeSubrange(matchEnd)
-      rangesToHighlight.append(.init(uncheckedBounds: (matchBegin.lowerBound, matchEnd.lowerBound)))
+    enum Tag {
+      case pre(R), post(R)
     }
-
-    return (newText, rangesToHighlight)
-
+    
+    func nextTag(in string: String) -> Tag? {
+      switch (string.range(of: preTag, options: options), string.range(of: postTag, options: options)) {
+      case (.some(let pre), .some(let post)) where pre.lowerBound < post.lowerBound:
+        return .pre(pre)
+      case (.some, .some(let post)):
+        return .post(post)
+      case (.some(let pre), .none):
+        return .pre(pre)
+      case (.none, .some(let post)):
+        return .post(post)
+      case (.none, .none):
+        return nil
+      }
+    }
+        
+    while let nextTag = nextTag(in: output) {
+      switch nextTag {
+      case .pre(let preRange):
+        preStack.append(preRange)
+        output.removeSubrange(preRange)
+      case .post(let postRange):
+        if let lastPre = preStack.last {
+          preStack.removeLast()
+          rangesToHighlight.append(.init(uncheckedBounds: (lastPre.lowerBound, postRange.lowerBound)))
+        }
+        output.removeSubrange(postRange)
+      }
+    }
+        
+    return (output, mergeOverlapping(rangesToHighlight))
   }
-
+  
   private static func computeInvertedRanges(for string: String, with ranges: [Range<String.Index>]) -> [Range<String.Index>] {
-
+    
+    if ranges.isEmpty {
+      return ([string.startIndex..<string.endIndex])
+    }
+    
     var lowerBound = string.startIndex
 
     var invertedRanges: [Range<String.Index>] = []
 
-    for range in ranges {
+    for range in ranges where range.lowerBound >= lowerBound {
       if lowerBound != range.lowerBound {
         let invertedRange = lowerBound..<range.lowerBound
         invertedRanges.append(invertedRange)
       }
       lowerBound = range.upperBound
     }
 
-    if lowerBound != string.endIndex {
+    if lowerBound != string.endIndex, lowerBound != string.startIndex {
       invertedRanges.append(lowerBound..<string.endIndex)
     }
 
     return invertedRanges
 
   }
+  
+  private static func mergeOverlapping<T: Comparable>(_ input: [Range<T>]) -> [Range<T>] {
+    var output: [Range<T>] = []
+    let sortedRanges = input.sorted(by: { $0.lowerBound < $1.lowerBound })
+    guard let head = sortedRanges.first else { return output }
+    let tail = sortedRanges.suffix(from: sortedRanges.index(after: sortedRanges.startIndex))
+    var (lower, upper) = (head.lowerBound, head.upperBound)
+    for range in tail {
+      if range.lowerBound <= upper {
+        if range.upperBound > upper {
+          upper = range.upperBound
+        } else {
+          continue
+        }
+      } else {
+        output.append(.init(uncheckedBounds: (lower: lower, upper: upper)))
+        (lower, upper) = (range.lowerBound, range.upperBound)
+      }
+    }
+    output.append(.init(uncheckedBounds: (lower: lower, upper: upper)))
+    return output
+  }
 
 }
 

Tests/AlgoliaSearchClientTests/Misc/HighlightedHTML.json

@@ -0,0 +1,4 @@
+{
+  "source": "<div class=\"c-entry-content\">\n<p id=\"4B922A\">Supercell’s mobile games, <em>Clash of Clans</em> and <em>Clash Royale,</em> are massive: together, the pair has been downloaded more than 3 billion times, according to the developer. Now, the Finnish studio is bringing them together with a new <em>Clash</em> animated short, a 12-minute piece called <em>Lost &amp; Crowned</em>. It’s gorgeous and funny but also part of a broader strategy to make the overarching fantasy world something bigger and more enticing. “When it comes to storytelling, we feel it’s important to expand beyond what’s in the games,” says Christina Lee, who oversees story and franchise development at Supercell.</p>\n<p id=\"WVPh8p\">This isn’t technically the first animated short set in the <em>Clash of Clans</em> universe. <a href=\"https://www.theverge.com/2016/12/23/14053766/clash-a-rama-animated-series-the-simpsons-supercell-clash-of-clans\">That distinction goes to <em>Clash-A-Rama</em></a>, a YouTube series penned by a trio of <em>Simpsons</em> writers that debuted back in 2016. Lee says that the series was a big success, particularly when it came to expanding the fictional world of <em>Clash</em>. The games themselves have very little narrative, instead focusing almost entirely on combat. But <em>Clash-A-Rama</em> showed what the characters did in their off-time, adding a newfound depth to that world. It made Supercell want to do something even bigger.</p>\n<div class=\"c-float-right\"></div>\n<p id=\"z2satY\">“While <em>Clash-A-Rama</em> was a big success, we always saw it as exceptional fan fiction, and decided that it was time to tell Supercell’s take on the world and the characters, and that’s why we set out to do this animated short,” says Lee. That original series featured a bright and approachable 2D art style, whereas <em>Lost &amp; Crowned</em> has a 3D look more in-keeping with the games and CG commercials and trailers. One look, and it’s clear it’s set in the <em>Clash</em> universe.</p>\n<p id=\"wQIGYs\">It was developed in collaboration with animation and design studio Psyops. The studio has previously worked with Riot on <a href=\"https://www.theverge.com/2020/7/24/21336218/league-of-legends-yone-anime-cinematic-riot-games\">a <em>League of Legends</em> short</a>, and it even turned a Travel Oregon commercial into <a href=\"https://www.youtube.com/watch?v=doVV1a7XgyQ\">a Studio Ghibli-style anime clip</a> — and Lee says the main goal, aside from bridging the world of <em>Clash Royale</em> and <em>Clash of Clans</em>, was to focus on a character-driven narrative. “One of the driving principles for our team is to make our characters feel like real people, particularly people you can relate to,” she says. “That relatability is really key for us.”</p>\n<p id=\"TyGasf\">This kind of storytelling is something of a trend among popular multiplayer games. Titles like <em>Overwatch</em> and <em>League of Legends</em> don’t feature much storytelling inside of the actual game, but there are copious animated shorts, comic books, and other content that flesh out their fictional worlds. Lee heads up a team of three focused entirely on this kind of external storytelling for <em>Clash</em>. And while Supercell isn’t ready to announce anything beyond <em>Lost &amp; Crowned</em>, it’s not hard to foresee a future where more animated clips, or even things like comics, further flesh out <em>Clash</em>’s fantasy realm. “We’re in the process of evaluating other opportunities for storytelling,” says Lee.</p>\n</div>",
+  "markup": "<div class=\"c-entry-content\">\n<p id=\"4B922A\">Supercell’s mobile games, <em>Clash of Clans</em> <em>a</em>nd <em>Clash Royale,</em> <em>a</em>re massive: together, the pair has been downloaded more than 3 billion times, <em>a</em>ccording to the developer. Now, the Finnish studio is bringing them together with <em>a</em> new <em>Clash</em> <em>a</em>nimated short, <em>a</em> 12-minute piece called <em>Lost &amp; Crowned</em>. It’s gorgeous <em>a</em>nd funny but <em>a</em>lso part of <em>a</em> broader strategy to make the overarching fantasy world something bigger <em>a</em>nd more enticing. “When it comes to storytelling, we feel it’s important to expand beyond what’s in the games,” says Christina Lee, who oversees story <em>a</em>nd franchise development <em>a</em>t Supercell.</p>\n<p id=\"WVPh8p\">This isn’t technically the first <em>a</em>nimated short set in the <em>Clash of Clans</em> universe. <a href=\"https://www.theverge.com/2016/12/23/14053766/clash-a-rama-animated-series-the-simpsons-supercell-clash-of-clans\">That distinction goes to <em>Clash-<em>A</em>-Rama</em></a>, <em>a</em> YouTube series penned by <em>a</em> trio of <em>Simpsons</em> writers that debuted back in 2016. Lee says that the series was <em>a</em> big success, particularly when it came to expanding the fictional world of <em>Clash</em>. The games themselves have very little narrative, instead focusing <em>a</em>lmost entirely on combat. But <em>Clash-<em>A</em>-Rama</em> showed what the characters did in their off-time, <em>a</em>dding <em>a</em> newfound depth to that world. It made Supercell want to do something even bigger.</p>\n<div class=\"c-float-right\"></div>\n<p id=\"z2satY\">“While <em>Clash-<em>A</em>-Rama</em> was <em>a</em> big success, we <em>a</em>lways saw it <em>a</em>s exceptional fan fiction, <em>a</em>nd decided that it was time to tell Supercell’s take on the world <em>a</em>nd the characters, <em>a</em>nd that’s why we set out to do this <em>a</em>nimated short,” says Lee. That original series featured <em>a</em> bright <em>a</em>nd <em>a</em>pproachable 2D <em>a</em>rt style, whereas <em>Lost &amp; Crowned</em> has <em>a</em> 3D look more in-keeping with the games <em>a</em>nd CG commercials <em>a</em>nd trailers. One look, <em>a</em>nd it’s clear it’s set in the <em>Clash</em> universe.</p>\n<p id=\"wQIGYs\">It was developed in collaboration with <em>a</em>nimation <em>a</em>nd design studio Psyops. The studio has previously worked with Riot on <a href=\"https://www.theverge.com/2020/7/24/21336218/league-of-legends-yone-anime-cinematic-riot-games\"><em>a</em> <em>League of Legends</em> short</a>, <em>a</em>nd it even turned <em>a</em> Travel Oregon commercial into <a href=\"https://www.youtube.com/watch?v=doVV1a7XgyQ\"><em>a</em> Studio Ghibli-style <em>a</em>nime clip</a> — <em>a</em>nd Lee says the main goal, <em>a</em>side from bridging the world of <em>Clash Royale</em> <em>a</em>nd <em>Clash of Clans</em>, was to focus on <em>a</em> character-driven narrative. “One of the driving principles for our team is to make our characters feel like real people, particularly people you can relate to,” she says. “That relatability is really key for us.”</p>\n<p id=\"TyGasf\">This kind of storytelling is something of <em>a</em> trend <em>a</em>mong popular multiplayer games. Titles like <em>Overwatch</em> <em>a</em>nd <em>League of Legends</em> don’t feature much storytelling inside of the <em>a</em>ctual game, but there <em>a</em>re copious <em>a</em>nimated shorts, comic books, <em>a</em>nd other content that flesh out their fictional worlds. Lee heads up <em>a</em> team of three focused entirely on this kind of external storytelling for <em>Clash</em>. <em>A</em>nd while Supercell isn’t ready to <em>a</em>nnounce <em>a</em>nything beyond <em>Lost &amp; Crowned</em>, it’s not hard to foresee <em>a</em> future where more <em>a</em>nimated clips, or even things like comics, further flesh out <em>Clash</em>’s fantasy realm. “We’re in the process of evaluating other opportunities for storytelling,” says Lee.</p>\n</div>"
+}

Tests/AlgoliaSearchClientTests/Unit/HighlightedStringTests.swift

@@ -55,6 +55,17 @@ class HighlightedStringTests: XCTestCase {
 
   }
 
+    struct MarkupString: Codable {
+      let source: String
+      let markup: HighlightedString
+    }
+  
+    func testWithHTMLString() throws {
+      let data = try Data(filename: "HighlightedHTML.json")
+      let searchResponse = try JSONDecoder().decode(MarkupString.self, from: data)
+      let searchResponseJSON = try JSON(searchResponse)
+      print(searchResponseJSON)
+    }
 
 }