Rename RetainableByteBufferPool to ByteBufferPool #9300

kohlschuetter · 2023-02-02T21:34:28Z

With issue #9166, ByteBufferPool was removed and replaced by RetainableByteBufferPool. Since ByteBufferPool was used by AbstractConnector, this change broke backwards compatibility with third-party connectors such as junixsocket-jetty.

Since there's no longer any other ByteBufferPool, rename the RetainableByteBufferPool interface, and thereby not only reinstate compatibility with existing third-party libraries but also save a few keystrokes.

#9284

Signed-off-by: Christian Kohlschütter [email protected]

joakime · 2023-02-02T21:41:16Z

Related to #9072

kohlschuetter · 2023-02-02T22:21:04Z

For what it's worth, I used the following commands to rename these references:

gsed -E -i"" 's/\bRetainableByteBufferPool/ByteBufferPool/g' $(find . -name "*.java")
gsed -E -i"" 's/\bretainableByteBufferPool/byteBufferPool/g' $(find . -name "*.java")
gsed -E -i"" 's/\bgetRetainableByteBufferPool/getByteBufferPool/g' $(find . -name "*.java")
gsed -E -i"" 's/\bsetRetainableByteBufferPool/setByteBufferPool/g' $(find . -name "*.java")
cd ./jetty-core/jetty-io/src/main/java/org/eclipse/jetty/io/
git mv RetainableByteBufferPool.java ByteBufferPool.java
cd -

I then temporarily added impsort-maven-plugin to pom.xml to reorganize the now-renamed imports (also see #9304):

Add the profile to pom.xml:

    <profile>
      <id>reformat</id>
      <properties>
        <skipTests>true</skipTests>
        <checkstyle.skip>true</checkstyle.skip>
        <enforcer.skip>true</enforcer.skip>
        <license.skip>true</license.skip>
        <spotbugs.skip>true</spotbugs.skip>
      </properties>
      <build>
        <plugins>
          <plugin>
            <groupId>net.revelc.code</groupId>
            <artifactId>impsort-maven-plugin</artifactId>
            <version>1.8.0</version>
            <configuration>
              <groups>java,*</groups>
              <staticGroups>*</staticGroups>
              <excludes>
                <exclude>**/module-info.java</exclude>
              </excludes>
              <compliance>14_PREVIEW</compliance>
              <staticAfter>true</staticAfter>
              <removeUnused>false</removeUnused>
            </configuration>
            <executions>
              <execution>
                <id>sort-imports</id>
                <goals>
                  <goal>sort</goal><!-- runs at process-sources phase by default -->
                </goals>
              </execution>
            </executions>
          </plugin>
        </plugins>
      </build>
    </profile>

Reorganize imports

mvn process-sources -Preformat

kohlschuetter · 2023-02-02T23:05:06Z

"502 Bad Gateway" in Java 19 unit tests, probably unrelated, since Java 17 tests pass

joakime · 2023-02-02T23:21:41Z

@olamy Seems that we are having DNS issues on our CI machine.

The 502 Bad Gateway is due to the proxy being unable to resolve "www.eclipse.org" ...

2023-02-02 22:49:08.262:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 2023-02-02 22:49:08.262:DEBUG:oejepP.JavadocTransparentProxy:qtp1196765369-30: 1187524360 rewriting: http://localhost:35951/proxy/current/ -> https://www.eclipse.org/jetty/javadoc/jetty-10/index.html?overview-summary.html/current/
2023-02-02 22:49:08.287:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 2023-02-02 22:49:08.287:DEBUG:oejepP.JavadocTransparentProxy:qtp1196765369-30: 1187524360 proxying to upstream:
2023-02-02 22:49:08.287:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: GET /proxy/current/ HTTP/1.1
2023-02-02 22:49:08.287:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: User-Agent: Jetty/12.0.0-SNAPSHOT
2023-02-02 22:49:08.287:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: Host: localhost:35951
2023-02-02 22:49:08.287:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: Accept-Encoding: gzip
2023-02-02 22:49:08.287:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 
2023-02-02 22:49:08.287:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: HttpRequest[GET /jetty/javadoc/jetty-10/index.html HTTP/1.1]@a83dae9
2023-02-02 22:49:08.287:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: User-Agent: Jetty/12.0.0-SNAPSHOT
2023-02-02 22:49:08.287:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: Accept-Encoding: gzip
2023-02-02 22:49:08.288:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: Host: www.eclipse.org
2023-02-02 22:49:08.288:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: Via: 1.1 jenkins-slave-6sczs
2023-02-02 22:49:08.288:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: X-Forwarded-For: 127.0.0.1
2023-02-02 22:49:08.288:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: X-Forwarded-Proto: http
2023-02-02 22:49:08.288:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: X-Forwarded-Host: localhost:35951
2023-02-02 22:49:08.288:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: X-Forwarded-Server: 127.0.0.1
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 2023-02-02 22:49:08.494:DEBUG:oejepP.JavadocTransparentProxy:qtp1196765369-31: 1187524360 proxying failed
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: java.net.UnknownHostException: www.eclipse.org: Name or service not known
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at java.base/java.net.Inet6AddressImpl.lookupAllHostAddr(Native Method)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at java.base/java.net.Inet6AddressImpl.lookupAllHostAddr(Inet6AddressImpl.java:52)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at java.base/java.net.InetAddress$PlatformResolver.lookupByName(InetAddress.java:1059)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at java.base/java.net.InetAddress.getAddressesFromNameService(InetAddress.java:1668)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at java.base/java.net.InetAddress$NameServiceAddresses.get(InetAddress.java:1003)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at java.base/java.net.InetAddress.getAllByName0(InetAddress.java:1658)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at java.base/java.net.InetAddress.getAllByName(InetAddress.java:1524)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at org.eclipse.jetty.util.SocketAddressResolver$Async.lambda$resolve$1(SocketAddressResolver.java:165)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at [email protected]/org.eclipse.jetty.util.thread.QueuedThreadPool.runJob(QueuedThreadPool.java:934)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at [email protected]/org.eclipse.jetty.util.thread.QueuedThreadPool$Runner.run(QueuedThreadPool.java:1078)
2023-02-02 22:49:08.494:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 	at java.base/java.lang.Thread.run(Thread.java:1589)
2023-02-02 22:49:08.508:INFO :oejth.JettyHomeTester:ConsoleStreamer/STDERR: 2023-02-02 22:49:08.508:DEBUG:oejepP.JavadocTransparentProxy:qtp1196765369-31: 1187524360 proxying complete

olamy · 2023-02-02T23:50:50Z

that's weird we can resolve github.com there is no reason we can't resolve eclipse.org

olamy · 2023-02-02T23:51:29Z

well but on the other side being dependant of external service is not good for unit test ;)

gregw · 2023-02-04T22:35:48Z

This is now clashing with the JPMS fixes that have been merged.

With issue jetty#9166, ByteBufferPool was removed and replaced by RetainableByteBufferPool. Since ByteBufferPool was used by AbstractConnector, this change broke backwards compatibility with third-party connectors such as junixsocket-jetty. Since there's no longer any other ByteBufferPool, rename the RetainableByteBufferPool interface, and thereby not only reinstate compatibility with existing third-party libraries but also save a few keystrokes. jetty#9284 Signed-off-by: Christian Kohlschütter <[email protected]>

kohlschuetter · 2023-02-04T22:41:11Z

@gregw done. When do you think can this be merged?

gregw · 2023-02-06T01:18:47Z

@sbordet @lorban will defer to you guys on this one.

ArrayRetainableByteBufferPool -> ArrayByteBufferPool. Updated field names. Updates tests. Updated *.mod files to not reference RetainableByteBufferPool. Updated javadocs. Signed-off-by: Simone Bordet <[email protected]>

sbordet · 2023-02-06T09:34:02Z

Approved.
I also changed all references to RetainableByteBufferPool in fields, tests, javadocs, documentation, etc.

…x-documentation-operations-logging * upstream/jetty-12.0.x: (42 commits) fixed style cleanup TODOs for decoration Issue jetty#9300 - Rename RetainableByteBufferPool to ByteBufferPool Removed TODOs that will not be done. Rename Handler Nested & Collection (jetty#9305) fix surefire jpms configuration fix merge fix merge Bump maven.surefire.plugin.version from 3.0.0-M5 to 3.0.0-M8 (jetty#9255) Rename RetainableByteBufferPool to ByteBufferPool Fixed merge Fix jetty#9285 use possibly wrapper response for redirection (jetty#9286) Issue jetty#9293 - Jetty 12 - Relax JPMS dependencies (quic) (jetty#9307) Issue jetty#9293 - Jetty 12 - Relax JPMS dependencies (fcgi) (jetty#9306) Jetty 10 - Configurable Unsafe Host Header (jetty#9283) Issue jetty#9293 - Jetty 12 - Relax JPMS dependencies. (jetty#9296) Issue jetty#9293 - Jetty 12 - Relax JPMS dependencies. (jetty#9299) fix dependency add used dependency this dependency is used in test scope ...

joakime linked an issue Feb 2, 2023 that may be closed by this pull request

Jetty 12: Removal of ByteBufferPool breaks AbstractConnector subclasses #9284

Closed

joakime added Build Jetty 12 labels Feb 2, 2023

joakime added this to the 12.0.x milestone Feb 2, 2023

gregw requested review from sbordet and lorban February 2, 2023 21:43

kohlschuetter force-pushed the issue/9284 branch from 8b2c0b8 to 0a0fd8d Compare February 3, 2023 18:49

kohlschuetter force-pushed the issue/9284 branch from 0a0fd8d to b8ab66f Compare February 4, 2023 22:40

sbordet removed this from the 12.0.x milestone Feb 6, 2023

sbordet merged commit 44980eb into jetty:jetty-12.0.x Feb 6, 2023

kohlschuetter mentioned this pull request Feb 6, 2023

Add Maven profile to automatically reformat code #9304

Closed

sbordet mentioned this pull request Feb 15, 2023

Jetty 12: Removal of ByteBufferPool breaks AbstractConnector subclasses #9284

Closed

Provide feedback

Saved searches

Use saved searches to filter your results more quickly

Rename RetainableByteBufferPool to ByteBufferPool #9300

Rename RetainableByteBufferPool to ByteBufferPool #9300

kohlschuetter commented Feb 2, 2023

joakime commented Feb 2, 2023

kohlschuetter commented Feb 2, 2023 •

edited

Loading

kohlschuetter commented Feb 2, 2023 •

edited

Loading

joakime commented Feb 2, 2023

olamy commented Feb 2, 2023

olamy commented Feb 2, 2023

gregw commented Feb 4, 2023

kohlschuetter commented Feb 4, 2023

gregw commented Feb 6, 2023

sbordet commented Feb 6, 2023

Rename RetainableByteBufferPool to ByteBufferPool #9300

Rename RetainableByteBufferPool to ByteBufferPool #9300

Conversation

kohlschuetter commented Feb 2, 2023

joakime commented Feb 2, 2023

kohlschuetter commented Feb 2, 2023 • edited Loading

kohlschuetter commented Feb 2, 2023 • edited Loading

joakime commented Feb 2, 2023

olamy commented Feb 2, 2023

olamy commented Feb 2, 2023

gregw commented Feb 4, 2023

kohlschuetter commented Feb 4, 2023

gregw commented Feb 6, 2023

sbordet commented Feb 6, 2023

kohlschuetter commented Feb 2, 2023 •

edited

Loading

kohlschuetter commented Feb 2, 2023 •

edited

Loading