Skip to content

WIP: Simplify UriParser #555

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.

Sign up for GitHub

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

Jump to bottom
Closed
raboof wants to merge 1 commit into from
Closed

WIP: Simplify UriParser #555

raboof wants to merge 1 commit into from

Conversation

@raboof
Copy link
Member

@raboof raboof commented Jun 24, 2024

(not for review yet, intermediate PR to test on CI)

@raboof raboof force-pushed the simplify-UriParser branch from b050911 to d124916 Compare June 25, 2024 08:02
@raboof raboof mentioned this pull request Jun 25, 2024
Closed
@garydgregory
Copy link
Member

garydgregory commented Jun 25, 2024 •
edited
Loading

Maybe one of the SSH tests is hung:

[INFO] Running org.apache.commons.vfs2.provider.sftp.SftpPutChannelTestCase
Exception in thread "pool-34-thread-1" java.lang.AssertionError
	at org.apache.sshd.common.session.AbstractSession.decode(AbstractSession.java:519)
	at org.apache.sshd.common.session.AbstractSession.messageReceived(AbstractSession.java:236)
	at org.apache.sshd.common.AbstractSessionIoHandler.messageReceived(AbstractSessionIoHandler.java:58)
	at org.apache.mina.core.filterchain.DefaultIoFilterChain$TailFilter.messageReceived(DefaultIoFilterChain.java:1015)
	at org.apache.mina.core.filterchain.DefaultIoFilterChain.callNextMessageReceived(DefaultIoFilterChain.java:650)
	at org.apache.mina.core.filterchain.DefaultIoFilterChain.access$1300(DefaultIoFilterChain.java:49)
	at org.apache.mina.core.filterchain.DefaultIoFilterChain$EntryImpl$1.messageReceived(DefaultIoFilterChain.java:1128)
	at org.apache.mina.core.filterchain.IoFilterAdapter.messageReceived(IoFilterAdapter.java:122)
	at org.apache.mina.core.filterchain.DefaultIoFilterChain.callNextMessageReceived(DefaultIoFilterChain.java:650)
	at org.apache.mina.core.filterchain.DefaultIoFilterChain.fireMessageReceived(DefaultIoFilterChain.java:643)
	at org.apache.mina.core.polling.AbstractPollingIoProcessor.read(AbstractPollingIoProcessor.java:539)
	at org.apache.mina.core.polling.AbstractPollingIoProcessor.access$1200(AbstractPollingIoProcessor.java:68)
	at org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.process(AbstractPollingIoProcessor.java:1224)
	at org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.process(AbstractPollingIoProcessor.java:1213)
	at org.apache.mina.core.polling.AbstractPollingIoProcessor$Processor.run(AbstractPollingIoProcessor.java:683)
	at org.apache.mina.util.NamePreservingRunnable.run(NamePreservingRunnable.java:64)
	at java.util.concurrent.ThreadPoolExecutor.runWorker(ThreadPoolExecutor.java:1149)
	at java.util.concurrent.ThreadPoolExecutor$Worker.run(ThreadPoolExecutor.java:624)
	at java.lang.Thread.run(Thread.java:750)

We are on an antique version of Mina, which I've tried to update in the past, but didn't manage to get working...

@raboof
WIP: Simplify UriParser
3ce7a7e 
(intermediate commit to test on CI)
@raboof raboof force-pushed the simplify-UriParser branch from d124916 to 3ce7a7e Compare June 25, 2024 11:18
@raboof
Copy link
Member Author

raboof commented Jun 25, 2024

retriggered by force-pushing

@garydgregory
Copy link
Member

garydgregory commented Jun 25, 2024

FYI @raboof All tests passed.

raboof added a commit to raboof/commons-vfs that referenced this pull request Jun 26, 2024
@raboof
Simplify UriParser
cd8458b 
The reverts apache#396 and related
changes and implements the same in a simpler way by replacing the
encoded characters already in `fixSeparators`.

This approach has a slightly higher risk at breaking existing behaviour,
but a lower risk of remaining problems in this part of the codebase. All
testcases still succeed.

This PR is intended to replace apache#543 and apache#555

This reverts commit cb45c94.
This reverts commit 5399c76.
@japplis
Copy link
Contributor

japplis commented Jun 26, 2024

I think this PR would also remove the need to merge #547

raboof added a commit to raboof/commons-vfs that referenced this pull request Jun 26, 2024
@raboof @japplis
Simplify UriParser
87149c1 
The reverts apache#396 and related
changes and implements the same in a simpler way by replacing the
encoded characters already in `fixSeparators`.

This approach has a slightly higher risk at breaking existing behaviour,
but a lower risk of remaining problems in this part of the codebase. All
testcases still succeed.

This PR is intended to replace apache#543 and apache#555. It includes the testcases
from apache#543, adapted to the behaviour before apache#396.

This reverts commit cb45c94.
This reverts commit 5399c76.

Co-Authored-By: Anthony Goubard <anthony.goubard@japplis.com>
@raboof raboof mentioned this pull request Jun 26, 2024
Merged
raboof added a commit to raboof/commons-vfs that referenced this pull request Jun 26, 2024
@raboof @japplis
Simplify UriParser
4b02af2 
The reverts apache#396 and related
changes and implements the same in a simpler way by replacing the
encoded characters already in `fixSeparators`.

This approach has a slightly higher risk at breaking existing behaviour,
but a lower risk of remaining problems in this part of the codebase. All
testcases still succeed.

This PR is intended to replace apache#543 and apache#555. It includes the testcases
from apache#543, adapted to the behaviour before apache#396.

This reverts commit cb45c94.
This reverts commit 5399c76.

Co-Authored-By: Anthony Goubard <anthony.goubard@japplis.com>
@raboof
Copy link
Member Author

raboof commented Jul 6, 2024

closing in favor of #558

@raboof raboof closed this Jul 6, 2024
raboof added a commit to raboof/commons-vfs that referenced this pull request Jul 25, 2024
@raboof @japplis
Simplify UriParser
4ac8c9d 
The reverts apache#396 and related
changes and implements the same in a simpler way by replacing the
encoded characters already in `fixSeparators`.

This approach has a slightly higher risk at breaking existing behaviour,
but a lower risk of remaining problems in this part of the codebase. All
testcases still succeed.

This PR is intended to replace apache#543 and apache#555. It includes the testcases
from apache#543, adapted to the behaviour before apache#396.

This reverts commit cb45c94.
This reverts commit 5399c76.

Co-Authored-By: Anthony Goubard <anthony.goubard@japplis.com>
garydgregory pushed a commit that referenced this pull request Jul 27, 2024
@raboof @japplis
Simplify UriParser (#558)
83d815a 
* Simplify UriParser

The reverts #396 and related
changes and implements the same in a simpler way by replacing the
encoded characters already in `fixSeparators`.

This approach has a slightly higher risk at breaking existing behaviour,
but a lower risk of remaining problems in this part of the codebase. All
testcases still succeed.

This PR is intended to replace #543 and #555. It includes the testcases
from #543, adapted to the behaviour before #396.

This reverts commit cb45c94.
This reverts commit 5399c76.

Co-Authored-By: Anthony Goubard <anthony.goubard@japplis.com>

* Add benchmark for UriParser

---------

Co-authored-by: Anthony Goubard <anthony.goubard@japplis.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment

Reviewers

No reviews

Assignees

No one assigned

Labels

None yet

Projects

None yet

Milestone

No milestone

Development

Successfully merging this pull request may close these issues.

3 participants

@raboof @garydgregory @japplis