From 62cdf5e9864afa654ee942a91b530bee9ddba3da Mon Sep 17 00:00:00 2001 From: svenvc Date: Fri, 26 Apr 2024 13:02:57 +0200 Subject: [PATCH] make ZnTooManyRedirects more intelligent (add a trail of followed URLs and two resume behaviors) --- .../executeWithRedirectsRemaining..st | 14 -------- .../executeWithRedirectsRemaining.trail..st | 24 ++++++++++++++ .../instance/executeWithRetriesRemaining..st | 4 ++- .../ZnTooManyRedirects.class/README.md | 2 ++ .../instance/defaultResumeValue.st | 3 ++ .../instance/trail..st | 3 ++ .../instance/trail.st | 3 ++ .../ZnTooManyRedirects.class/properties.json | 6 ++-- .../monticello.meta/categories.st | 9 ++++- .../instance/testRedirect.st | 2 +- .../instance/testRedirectLoopAndTrail.st | 33 +++++++++++++++++++ 11 files changed, 84 insertions(+), 19 deletions(-) delete mode 100644 repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRedirectsRemaining..st create mode 100644 repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRedirectsRemaining.trail..st create mode 100644 repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/defaultResumeValue.st create mode 100644 repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/trail..st create mode 100644 repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/trail.st create mode 100644 repository/Zinc-Tests.package/ZnClientTest.class/instance/testRedirectLoopAndTrail.st diff --git a/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRedirectsRemaining..st b/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRedirectsRemaining..st deleted file mode 100644 index 0481cb11..00000000 --- a/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRedirectsRemaining..st +++ /dev/null @@ -1,14 +0,0 @@ -private - protocol -executeWithRedirectsRemaining: redirectCount - self getConnectionAndExecute. - response isRedirect - ifTrue: [ - (redirectCount > 0 and: [ self followRedirects ]) - ifTrue: [ - self - prepareRedirect; - executeWithRedirectsRemaining: redirectCount - 1 ] - ifFalse: [ - self followRedirects - ifTrue: [ ZnTooManyRedirects signal ] ] ]. - ^ self handleResponse \ No newline at end of file diff --git a/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRedirectsRemaining.trail..st b/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRedirectsRemaining.trail..st new file mode 100644 index 00000000..d4ac6243 --- /dev/null +++ b/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRedirectsRemaining.trail..st @@ -0,0 +1,24 @@ +private - protocol +executeWithRedirectsRemaining: redirectCount trail: collectionOfUrls + self getConnectionAndExecute. + response isRedirect + ifTrue: [ + (redirectCount > 0 and: [ self followRedirects ]) + ifTrue: [ + self prepareRedirect. + collectionOfUrls add: self request url. + self + executeWithRedirectsRemaining: redirectCount - 1 + trail: collectionOfUrls ] + ifFalse: [ + self followRedirects + ifTrue: [ | exception | + (exception := ZnTooManyRedirects new) + trail: collectionOfUrls. + exception signal = exception defaultResumeValue + ifTrue: [ + "when resumed with default resume value, start over" + self + executeWithRedirectsRemaining: self maxNumberOfRedirects + trail: collectionOfUrls ] ] ] ]. + ^ self handleResponse \ No newline at end of file diff --git a/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRetriesRemaining..st b/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRetriesRemaining..st index 376ebf5a..69d5c256 100644 --- a/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRetriesRemaining..st +++ b/repository/Zinc-HTTP.package/ZnClient.class/instance/executeWithRetriesRemaining..st @@ -1,6 +1,8 @@ private - protocol executeWithRetriesRemaining: retryCount - ^ [ self executeWithRedirectsRemaining: self maxNumberOfRedirects ] + ^ [ self + executeWithRedirectsRemaining: self maxNumberOfRedirects + trail: OrderedCollection new ] on: self retryExceptionSet do: [ :exception | retryCount > 0 diff --git a/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/README.md b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/README.md index e4bdf29e..24979ac0 100644 --- a/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/README.md +++ b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/README.md @@ -1,3 +1,5 @@ ZnTooManyRedirects is signalled when an HTTP client has been following more redirects than allowed. +The default resume behavior is to retry, signal with any other value to give up just return the redirect. + Part of Zinc HTTP Components. \ No newline at end of file diff --git a/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/defaultResumeValue.st b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/defaultResumeValue.st new file mode 100644 index 00000000..1cceb5da --- /dev/null +++ b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/defaultResumeValue.st @@ -0,0 +1,3 @@ +private +defaultResumeValue + ^ #retry \ No newline at end of file diff --git a/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/trail..st b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/trail..st new file mode 100644 index 00000000..94ed2b2a --- /dev/null +++ b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/trail..st @@ -0,0 +1,3 @@ +accessing +trail: aCollectionOfUrls + trail := aCollectionOfUrls \ No newline at end of file diff --git a/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/trail.st b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/trail.st new file mode 100644 index 00000000..2ae9b5e9 --- /dev/null +++ b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/instance/trail.st @@ -0,0 +1,3 @@ +accessing +trail + ^ trail \ No newline at end of file diff --git a/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/properties.json b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/properties.json index 2e871fa2..72f3c6cb 100644 --- a/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/properties.json +++ b/repository/Zinc-HTTP.package/ZnTooManyRedirects.class/properties.json @@ -1,11 +1,13 @@ { - "commentStamp" : "", + "commentStamp" : "", "super" : "Error", "category" : "Zinc-HTTP-Exceptions", "classinstvars" : [ ], "pools" : [ ], "classvars" : [ ], - "instvars" : [ ], + "instvars" : [ + "trail" + ], "name" : "ZnTooManyRedirects", "type" : "normal" } \ No newline at end of file diff --git a/repository/Zinc-HTTP.package/monticello.meta/categories.st b/repository/Zinc-HTTP.package/monticello.meta/categories.st index b4484d8a..f7e23f08 100644 --- a/repository/Zinc-HTTP.package/monticello.meta/categories.st +++ b/repository/Zinc-HTTP.package/monticello.meta/categories.st @@ -1 +1,8 @@ -self packageOrganizer ensurePackage: #'Zinc-HTTP' withTags: #(#'Client-Server' #Core #Exceptions #Logging #Streaming #Support #Variables)! +SystemOrganization addCategory: #'Zinc-HTTP'! +SystemOrganization addCategory: #'Zinc-HTTP-Client-Server'! +SystemOrganization addCategory: #'Zinc-HTTP-Core'! +SystemOrganization addCategory: #'Zinc-HTTP-Exceptions'! +SystemOrganization addCategory: #'Zinc-HTTP-Logging'! +SystemOrganization addCategory: #'Zinc-HTTP-Streaming'! +SystemOrganization addCategory: #'Zinc-HTTP-Support'! +SystemOrganization addCategory: #'Zinc-HTTP-Variables'! diff --git a/repository/Zinc-Tests.package/ZnClientTest.class/instance/testRedirect.st b/repository/Zinc-Tests.package/ZnClientTest.class/instance/testRedirect.st index 20dcd055..eb8265f6 100644 --- a/repository/Zinc-Tests.package/ZnClientTest.class/instance/testRedirect.st +++ b/repository/Zinc-Tests.package/ZnClientTest.class/instance/testRedirect.st @@ -16,5 +16,5 @@ testRedirect beOneShot; maxNumberOfRedirects: 0; get: target; - response ] on: ZnTooManyRedirects do: [ :exception | exception resume ]. + response ] on: ZnTooManyRedirects do: [ :exception | exception resume: #doNotRetry ]. self assert: response isRedirect \ No newline at end of file diff --git a/repository/Zinc-Tests.package/ZnClientTest.class/instance/testRedirectLoopAndTrail.st b/repository/Zinc-Tests.package/ZnClientTest.class/instance/testRedirectLoopAndTrail.st new file mode 100644 index 00000000..44157997 --- /dev/null +++ b/repository/Zinc-Tests.package/ZnClientTest.class/instance/testRedirectLoopAndTrail.st @@ -0,0 +1,33 @@ +testing +testRedirectLoopAndTrail + self withServerDo: [ :server | | client count | + server onRequestRespond: [ :request | + request uri firstPathSegment = 'follow' + ifTrue: [ ZnResponse redirect: 'follow' ] ]. + + (client := ZnClient new) + url: server localUrl; addPath: 'follow'; + maxNumberOfRedirects: 10. + self should: [ client get ] raise: ZnTooManyRedirects. + client close. + + (client := ZnClient new) + url: server localUrl; addPath: 'follow'; + maxNumberOfRedirects: 10. + [ client get ] on: ZnTooManyRedirects do: [ :exception | + self assert: exception isResumable. + self assert: exception trail size equals: 10. + self assert: (exception trail allSatisfy: [ :each | each = (server localUrl / 'follow') ]) ]. + client close. + + (client := ZnClient new) + url: server localUrl; addPath: 'follow'; + maxNumberOfRedirects: 10. + count := 0. + [ client get ] on: ZnTooManyRedirects do: [ :exception | + count := count + 1. + exception trail size <= 30 + ifTrue: [ exception resume ] + ifFalse: [ exception resume: #doNotRetry ] ]. + self assert: count equals: 4. + client close ] \ No newline at end of file