Skip to content

Commit d16b51f

Browse files
davidvedvickdavidvedvick
authored
Merge pull request #515 from namehillsoftware/chore/consolidate-xml-parsing
[Chore] Consolidate XML Parsing
2 parents 10c5353 + 4c49dbe commit d16b51f

File tree

18 files changed

+228
-219
lines changed

18 files changed

+228
-219
lines changed

‎projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/browsing/files/properties/FilePropertiesHandler.kt‎

Lines changed: 0 additions & 35 deletions
This file was deleted.

‎projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/browsing/files/properties/FilePropertiesProvider.kt‎

Lines changed: 22 additions & 8 deletions
Original file line numberDiff line numberDiff line change
@@ -10,6 +10,7 @@ import com.lasthopesoftware.bluewater.client.connection.libraries.ProvideGuarant
1010
import com.lasthopesoftware.bluewater.shared.UrlKeyHolder
1111
import com.lasthopesoftware.exceptions.isOkHttpCanceled
1212
import com.lasthopesoftware.promises.extensions.preparePromise
13+
import com.lasthopesoftware.promises.extensions.toPromise
1314
import com.lasthopesoftware.resources.executors.ThreadPools
1415
import com.namehillsoftware.handoff.cancellation.CancellationSignal
1516
import com.namehillsoftware.handoff.promises.Promise
@@ -70,7 +71,7 @@ class FilePropertiesProvider(
7071
ImmediateResponse<Throwable, Unit>
7172
{
7273
private val cancellationProxy = CancellationProxy()
73-
private lateinit var response: Response
74+
private lateinit var responseString: String
7475

7576
private val urlKeyHolder
7677
get() = connectionProvider.urlProvider.baseUrl.let { UrlKeyHolder(it, serviceFile) }
@@ -94,28 +95,41 @@ class FilePropertiesProvider(
9495
}
9596
}
9697

97-
override fun promiseResponse(resolution: Response): Promise<Unit> {
98-
response = resolution
98+
override fun promiseResponse(response: Response): Promise<Unit> {
99+
responseString = response.body.use {
100+
if (cancellationProxy.isCancelled) {
101+
reject(FilePropertiesCancellationException(libraryId, serviceFile))
102+
return Unit.toPromise()
103+
}
104+
105+
it.string()
106+
}
107+
99108
return ThreadPools.compute.preparePromise(this)
100109
}
101110

102111
override fun prepareMessage(cancellationSignal: CancellationSignal) {
103-
if (cancellationProxy.isCancelled) {
104-
response.close()
112+
if (cancellationSignal.isCancelled) {
105113
reject(FilePropertiesCancellationException(libraryId, serviceFile))
106114
return
107115
}
108116

109117
resolve(
110-
response.body
111-
.use { body -> body.string() }
118+
responseString
112119
.let(Jsoup::parse)
113120
.let { xml ->
114121
xml
115122
.getElementsByTag("item")
116123
.firstOrNull()
117124
?.children()
118-
?.associateTo(HashMap()) { el ->Pair(el.attr("Name"), el.wholeOwnText()) }
125+
?.associateTo(HashMap()) { el ->
126+
if (cancellationSignal.isCancelled) {
127+
reject(FilePropertiesCancellationException(libraryId, serviceFile))
128+
return
129+
}
130+
131+
Pair(el.attr("Name"), el.wholeOwnText())
132+
}
119133
?: emptyMap()
120134
}
121135
.also { properties ->

‎projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/browsing/items/access/ItemProvider.kt‎

Lines changed: 27 additions & 16 deletions
Original file line numberDiff line numberDiff line change
@@ -4,13 +4,14 @@ import com.lasthopesoftware.bluewater.client.browsing.items.Item
44
import com.lasthopesoftware.bluewater.client.browsing.items.ItemId
55
import com.lasthopesoftware.bluewater.client.browsing.library.repository.LibraryId
66
import com.lasthopesoftware.bluewater.client.connection.libraries.ProvideGuaranteedLibraryConnections
7+
import com.lasthopesoftware.promises.extensions.keepPromise
78
import com.lasthopesoftware.promises.extensions.preparePromise
89
import com.lasthopesoftware.resources.executors.ThreadPools
10+
import com.lasthopesoftware.resources.io.promiseStringBody
11+
import com.lasthopesoftware.resources.io.promiseXmlDocument
912
import com.namehillsoftware.handoff.promises.Promise
1013
import com.namehillsoftware.handoff.promises.response.PromisedResponse
11-
import okhttp3.Response
12-
import org.jsoup.Jsoup
13-
import org.jsoup.parser.Parser
14+
import org.jsoup.nodes.Document
1415
import java.io.IOException
1516
import kotlin.coroutines.cancellation.CancellationException
1617

@@ -19,28 +20,38 @@ private const val browseLibraryParameter = "Browse/Children"
1920
class ItemProvider(private val connectionProvider: ProvideGuaranteedLibraryConnections) :
2021
ProvideItems,
2122
ProvideFreshItems,
22-
PromisedResponse<Response, List<Item>>
23+
PromisedResponse<Document, List<Item>>
2324
{
24-
override fun promiseItems(libraryId: LibraryId, itemId: ItemId?): Promise<List<Item>> =
25+
override fun promiseItems(libraryId: LibraryId, itemId: ItemId?): Promise<List<Item>> = Promise.Proxy { cp ->
2526
connectionProvider
2627
.promiseLibraryConnection(libraryId)
28+
.also(cp::doCancel)
2729
.eventually { connectionProvider ->
2830
connectionProvider
29-
.run {
31+
?.run {
3032
itemId
31-
?.run { promiseResponse(browseLibraryParameter, "ID=$id", "Version=2", "ErrorOnMissing=1") }
33+
?.run {
34+
promiseResponse(
35+
browseLibraryParameter,
36+
"ID=$id",
37+
"Version=2",
38+
"ErrorOnMissing=1"
39+
)
40+
}
3241
?: promiseResponse(browseLibraryParameter, "Version=2", "ErrorOnMissing=1")
3342
}
34-
}
35-
.eventually(this)
36-
37-
override fun promiseResponse(response: Response): Promise<List<Item>> = ThreadPools.compute.preparePromise { cs ->
38-
val bodyString = response.body.use { it.string() }
39-
if (cs.isCancelled) throw itemParsingCancelledException()
40-
41-
val xml = Jsoup.parse(bodyString, Parser.xmlParser())
43+
?.also(cp::doCancel)
44+
?.promiseStringBody()
45+
?.also(cp::doCancel)
46+
?.promiseXmlDocument()
47+
?.also(cp::doCancel)
48+
?.eventually(this)
49+
.keepPromise(emptyList())
50+
}
51+
}
4252

43-
val body = xml.getElementsByTag("Response").firstOrNull()
53+
override fun promiseResponse(document: Document): Promise<List<Item>> = ThreadPools.compute.preparePromise { cs ->
54+
val body = document.getElementsByTag("Response").firstOrNull()
4455
?: throw IOException("Response tag not found")
4556

4657
val status = body.attr("Status")

‎projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/browsing/items/playlists/PlaylistsStorage.kt‎

Lines changed: 29 additions & 26 deletions
Original file line numberDiff line numberDiff line change
@@ -3,40 +3,43 @@ package com.lasthopesoftware.bluewater.client.browsing.items.playlists
33
import com.lasthopesoftware.bluewater.client.browsing.files.ServiceFile
44
import com.lasthopesoftware.bluewater.client.browsing.library.repository.LibraryId
55
import com.lasthopesoftware.bluewater.client.connection.libraries.ProvideLibraryConnections
6-
import com.lasthopesoftware.bluewater.shared.StandardResponse
76
import com.lasthopesoftware.promises.extensions.keepPromise
87
import com.lasthopesoftware.promises.extensions.preparePromise
98
import com.lasthopesoftware.resources.executors.ThreadPools
9+
import com.lasthopesoftware.resources.io.promiseStandardResponse
10+
import com.lasthopesoftware.resources.io.promiseStringBody
11+
import com.lasthopesoftware.resources.io.promiseXmlDocument
1012
import com.namehillsoftware.handoff.promises.Promise
11-
import org.jsoup.Jsoup
12-
import org.jsoup.parser.Parser
1313

1414
class PlaylistsStorage(private val libraryConnections: ProvideLibraryConnections) : StorePlaylists {
1515
override fun promiseAudioPlaylistPaths(libraryId: LibraryId): Promise<List<String>> =
1616
libraryConnections
1717
.promiseLibraryConnection(libraryId)
1818
.eventually { connectionProvider ->
19-
connectionProvider
20-
?.promiseResponse("Playlists/List", "IncludeMediaTypes=1")
21-
?.then { response ->
22-
response.body
23-
.use { body -> Jsoup.parse(body.string(), Parser.xmlParser()) }
24-
.let { xml ->
25-
xml
26-
.getElementsByTag("Item")
27-
.mapNotNull { itemXml ->
28-
itemXml
29-
.takeIf {
30-
it.getElementsByTag("Field")
31-
.any { el -> el.attr("Name") == "MediaTypes" && el.ownText() == "Audio" }
32-
}
33-
?.getElementsByTag("Field")
34-
?.firstOrNull { el -> el.attr("Name") == "Path" }
35-
?.ownText()
36-
}
37-
}
38-
}
39-
.keepPromise(emptyList())
19+
Promise.Proxy { cp ->
20+
connectionProvider
21+
?.promiseResponse("Playlists/List", "IncludeMediaTypes=1")
22+
?.also(cp::doCancel)
23+
?.promiseStringBody()
24+
?.also(cp::doCancel)
25+
?.promiseXmlDocument()
26+
?.also(cp::doCancel)
27+
?.then { xml ->
28+
xml
29+
.getElementsByTag("Item")
30+
.mapNotNull { itemXml ->
31+
itemXml
32+
.takeIf {
33+
it.getElementsByTag("Field")
34+
.any { el -> el.attr("Name") == "MediaTypes" && el.ownText() == "Audio" }
35+
}
36+
?.getElementsByTag("Field")
37+
?.firstOrNull { el -> el.attr("Name") == "Path" }
38+
?.ownText()
39+
}
40+
}
41+
.keepPromise(emptyList())
42+
}
4043
}
4144

4245
override fun promiseStoredPlaylist(libraryId: LibraryId, playlistPath: String, playlist: List<ServiceFile>): Promise<*> =
@@ -45,8 +48,8 @@ class PlaylistsStorage(private val libraryConnections: ProvideLibraryConnections
4548
.eventually { connectionProvider ->
4649
connectionProvider?.run {
4750
promiseResponse("Playlists/Add", "Type=Playlist", "Path=$playlistPath", "CreateMode=Overwrite")
48-
.then { it -> it?.use { r -> r.body.byteStream().use(StandardResponse::fromInputStream) } }
49-
.then { it -> it?.items?.get("PlaylistID") }
51+
.promiseStandardResponse()
52+
.then { it -> it.items["PlaylistID"] }
5053
.eventually {
5154
it?.let { playlistId ->
5255
ThreadPools.compute

‎projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/browsing/library/revisions/RevisionStorage.kt‎

Lines changed: 4 additions & 5 deletions
Original file line numberDiff line numberDiff line change
@@ -1,8 +1,8 @@
11
package com.lasthopesoftware.bluewater.client.browsing.library.revisions
22

33
import com.lasthopesoftware.bluewater.client.connection.ProvideConnections
4-
import com.lasthopesoftware.bluewater.shared.StandardResponse
54
import com.lasthopesoftware.policies.caching.TimedExpirationPromiseCache
5+
import com.lasthopesoftware.resources.io.promiseStandardResponse
66
import com.namehillsoftware.handoff.promises.Promise
77
import org.joda.time.Duration
88

@@ -19,10 +19,9 @@ internal object RevisionStorage {
1919
return expiringRevisionCache.getOrAdd(urlProvider.baseUrl.toString()) {
2020
connectionProvider
2121
.promiseResponse("Library/GetRevision")
22-
.then { response ->
23-
response.body
24-
.use { body -> body.byteStream().use(StandardResponse::fromInputStream) }
25-
?.let { standardRequest -> standardRequest.items["Sync"] }
22+
.promiseStandardResponse()
23+
.then { standardRequest ->
24+
standardRequest.items["Sync"]
2625
?.takeIf { revisionValue -> revisionValue.isNotEmpty() }!!
2726
.toInt()
2827
}

‎projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/connection/authentication/ConnectionAuthenticationCheckingExtensions.kt‎

Lines changed: 6 additions & 9 deletions
Original file line numberDiff line numberDiff line change
@@ -1,19 +1,16 @@
11
package com.lasthopesoftware.bluewater.client.connection.authentication
22

33
import com.lasthopesoftware.bluewater.client.connection.ProvideConnections
4-
import com.lasthopesoftware.bluewater.shared.StandardResponse
54
import com.lasthopesoftware.promises.extensions.toPromise
5+
import com.lasthopesoftware.resources.io.promiseStandardResponse
66
import com.namehillsoftware.handoff.promises.Promise
77

88
private val nullConnectionProviderPromise by lazy { false.toPromise() }
99

1010
internal fun ProvideConnections?.promiseIsReadOnly(): Promise<Boolean> =
1111
this?.promiseResponse("Authenticate")
12-
?.then { r ->
13-
r.body
14-
.use { b -> b.byteStream().use(StandardResponse::fromInputStream) }
15-
?.let { sr -> sr.items["ReadOnly"]?.toInt() }
16-
?.let { ro -> ro != 0 }
17-
?: false
18-
}
19-
?: nullConnectionProviderPromise
12+
?.promiseStandardResponse()
13+
?.then { sr ->
14+
sr.items["ReadOnly"]?.toInt()?.let { ro -> ro != 0 } ?: false
15+
}
16+
?: nullConnectionProviderPromise

‎projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/connection/builder/lookup/ServerInfoXmlRequest.kt‎

Lines changed: 5 additions & 3 deletions
Original file line numberDiff line numberDiff line change
@@ -5,11 +5,11 @@ import com.lasthopesoftware.bluewater.client.browsing.library.repository.Library
55
import com.lasthopesoftware.bluewater.client.connection.HttpPromisedResponse
66
import com.lasthopesoftware.bluewater.client.connection.okhttp.ProvideOkHttpClients
77
import com.lasthopesoftware.promises.extensions.keepPromise
8+
import com.lasthopesoftware.resources.io.promiseStringBody
9+
import com.lasthopesoftware.resources.io.promiseXmlDocument
810
import com.namehillsoftware.handoff.promises.Promise
911
import okhttp3.Request
10-
import org.jsoup.Jsoup
1112
import org.jsoup.nodes.Document
12-
import org.jsoup.parser.Parser
1313

1414
class ServerInfoXmlRequest(private val libraryProvider: ILibraryProvider, private val clientFactory: ProvideOkHttpClients) : RequestServerInfoXml {
1515
override fun promiseServerInfoXml(libraryId: LibraryId): Promise<Document?> = Promise.Proxy { cp ->
@@ -21,7 +21,9 @@ class ServerInfoXmlRequest(private val libraryProvider: ILibraryProvider, privat
2121
.build()
2222
}
2323
?.let { request -> HttpPromisedResponse(clientFactory.getJriverCentralClient().newCall(request)).also(cp::doCancel) }
24-
?.then { r -> r.body.use { b -> Jsoup.parse(b.string(), Parser.xmlParser()) } }
24+
?.promiseStringBody()
25+
?.also(cp::doCancel)
26+
?.promiseXmlDocument()
2527
.keepPromise()
2628
}
2729
}

‎projectBlueWater/src/main/java/com/lasthopesoftware/bluewater/client/connection/testing/ConnectionTester.kt‎

Lines changed: 9 additions & 2 deletions
Original file line numberDiff line numberDiff line change
@@ -1,12 +1,15 @@
11
package com.lasthopesoftware.bluewater.client.connection.testing
22

33
import com.lasthopesoftware.bluewater.client.connection.ProvideConnections
4-
import com.lasthopesoftware.bluewater.shared.StandardResponse
4+
import com.lasthopesoftware.bluewater.shared.NonStandardResponseException
5+
import com.lasthopesoftware.bluewater.shared.StandardResponse.Companion.toStandardResponse
56
import com.lasthopesoftware.bluewater.shared.lazyLogger
67
import com.namehillsoftware.handoff.cancellation.CancellationSignal
78
import com.namehillsoftware.handoff.promises.Promise
89
import com.namehillsoftware.handoff.promises.propagation.CancellationProxy
910
import okhttp3.Response
11+
import org.jsoup.Jsoup
12+
import org.jsoup.parser.Parser
1013
import java.io.IOException
1114

1215
object ConnectionTester : TestConnections {
@@ -39,11 +42,15 @@ object ConnectionTester : TestConnections {
3942
if (cancellationSignal.isCancelled) return false
4043

4144
try {
42-
return body.byteStream().use(StandardResponse::fromInputStream)?.isStatus ?: false
45+
return body.string().let { Jsoup.parse(it, Parser.xmlParser()) }.toStandardResponse().isStatusOk
46+
} catch (e: NonStandardResponseException) {
47+
logger.warn("Non standard response received.", e)
4348
} catch (e: IOException) {
4449
logger.error("Error closing connection, device failure?", e)
4550
} catch (e: IllegalArgumentException) {
4651
logger.warn("Illegal argument passed in", e)
52+
} catch (t: Throwable) {
53+
logger.error("Unexpected error parsing response.", t)
4754
}
4855
}
4956
return false

0 commit comments

Comments
 (0)