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

fix: reverse http handler calling order #1227

Merged
merged 1 commit into from
May 16, 2023
Merged

Conversation

0utplay
Copy link
Member

@0utplay 0utplay commented May 14, 2023

Motivation

The priority is documented as Handlers with a high priority will get called before handlers with a low priority. But the current implementation does not follow that documentation therefore we need to reverse the comparator to achieve a correct call chain.

Modification

Reversed the sorting comperator and inverted the priority in the test cases and the in the node rest handler to ensure that no behavior changes.

Result

The call chain is as documented

@0utplay 0utplay added v: 4.X This pull should be included in the 4.0 release in: driver An issue/pull request releated to the driver module code t: fix A pull request introducing a fix for a bug. in: module An issue/pull request releated to one of the internal modules labels May 14, 2023
@0utplay 0utplay added this to the 4.0.0-RC9 milestone May 14, 2023
@0utplay 0utplay requested a review from derklaro May 14, 2023 19:47
@0utplay 0utplay self-assigned this May 14, 2023
@github-actions
Copy link

Test Results

  47 files  ±0    47 suites  ±0   1m 11s ⏱️ -5s
321 tests ±0  321 ✔️ ±0  0 💤 ±0  0 ±0 
651 runs  ±0  651 ✔️ ±0  0 💤 ±0  0 ±0 

Results for commit c176679. ± Comparison against base commit 05236d2.

This pull request removes 12 and adds 12 tests. Note that renamed tests count towards both.
eu.cloudnetservice.driver.document.DocumentSerialisationTest ‑ [4] {"b":1,"s":2,"i":3,"l":4,"f":5.0,"d":6.0,"c":"/","string":"Hello, World!","bol":true,"cloud":["Ben?","Yes","No","HoHoHoHo"],"world":{"insane":"!","hello":"world","this":"is"}}, PRETTY
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [10] 15203, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [2] JM5AN2QTX5S3VNTNBYPVBM2FF, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [3] -276558892, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [3] Lobbyhello156:5d4759e3-8365-42ec-b0c3-70c57cd39a39
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [4] 2394853681375571828, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [4] ServiceInfoSnapshot(creationTime=1683653189636, address=127.0.1.1:99, processSnapshot=ProcessSnapshot[pid=2146, cpuUsage=50.877192982456144, systemCpuUsage=100.0, maxHeapMemory=536870912, heapUsageMemory=32564064, noHeapUsageMemory=58354080, unloadedClassCount=0, totalLoadedClassCount=8529, currentLoadedClassCount=8529, threads=[ThreadSnapshot[id=1, priority=5, daemon=false, name=Test worker, threadState=RUNNABLE], ThreadSnapshot[id=2, priority=10, daemon=true, name=Reference Handler, threadState=RUNNABLE]…
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [5] 0.7950877, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [6] 0.00968161763839448, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [7] true, null
…
eu.cloudnetservice.driver.document.DocumentSerialisationTest ‑ [4] {"b":1,"s":2,"i":3,"l":4,"f":5.0,"d":6.0,"c":"/","string":"Hello, World!","bol":true,"cloud":["Ben?","Yes","No","HoHoHoHo"],"world":{"this":"is","hello":"world","insane":"!"}}, PRETTY
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [10] 27648, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [2] KNG6W2EODL17XUKM253XLDWLK, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [3] -817261151, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [3] Lobbyhello156:6c3fc38a-e9a7-41ad-a1e8-8e917768f7c2
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [4] -7763562976841092541, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [4] ServiceInfoSnapshot(creationTime=1684093520939, address=127.0.1.1:99, processSnapshot=ProcessSnapshot[pid=2288, cpuUsage=86.66666666666667, systemCpuUsage=97.82608695652173, maxHeapMemory=536870912, heapUsageMemory=33399208, noHeapUsageMemory=58185144, unloadedClassCount=1, totalLoadedClassCount=8526, currentLoadedClassCount=8525, threads=[ThreadSnapshot[id=1, priority=5, daemon=false, name=Test worker, threadState=RUNNABLE], ThreadSnapshot[id=2, priority=10, daemon=true, name=Reference Handler, threadStat…
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [5] 0.9570593, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [6] 0.2944598860620822, null
eu.cloudnetservice.driver.network.rpc.object.DefaultObjectMapperTest ‑ [7] false, null
…

@derklaro derklaro merged commit 3d1d02a into nightly May 16, 2023
@derklaro derklaro deleted the reverse-http-handler-order branch May 16, 2023 11:43
derklaro pushed a commit that referenced this pull request May 16, 2023
#1228)

### Motivation
The bridge rest handler has routes that can collide in some situations.
To prevent that we have to set a higher priority on the literal only
routes.

### Modification
Set a high priority on literal only routes to prevent collisions.

### Result
No collisions on bridge rest routes.

##### Other context
Waits for #1227 

Fixes #1226
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
in: driver An issue/pull request releated to the driver module code in: module An issue/pull request releated to one of the internal modules t: fix A pull request introducing a fix for a bug. v: 4.X This pull should be included in the 4.0 release
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants