Conversation
Add custom class loader which will load legacy db code.
Remove parent class loader to make sure classes are only loaded from provided jar. Do not recreate git repo all the time. Add git checkout progress bar.
…test # Conflicts: # tests/src/test/java/com/jetbrains/youtrackdb/auto/BaseDBCompatibilityTest.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/code/CommonDownloader.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/code/JarDownloader.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/code/github/GithubJarBuilder.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/code/github/GithubRepoDownloader.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/code/github/MavenBuilder.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/code/maven/MavenJarDownloader.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/data/CommonDbDownloader.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/data/DbDownloader.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/data/FileDbDownloader.java # tests/src/test/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/data/HttpDbDownloader.java
Remove parent class loader to make sure classes are only loaded from provided jar. Do not recreate git repo all the time. Add git checkout progress bar. Add ability to exclude properties from comparison.(Password hashes does not match sometimes:) )
…will be introduced)
…rom terminal. Adjust sample config to not contain my own folder structure. Create docker file for testing. Add shadow jar plugin to not play with dependencies.
There was a problem hiding this comment.
Pull Request Overview
This PR introduces Docker support for binary compatibility testing in YouTrackDB. It creates a new binary-compat module that allows users to run binary compatibility tests on their private databases in isolated Docker environments.
Key changes:
- Added a new
binary-compatmodule with comprehensive binary compatibility testing framework - Enhanced core database comparison functionality to support property exclusion
- Created Docker infrastructure for running tests in isolated environments
Reviewed Changes
Copilot reviewed 26 out of 26 changed files in this pull request and generated 3 comments.
Show a summary per file
| File | Description |
|---|---|
| binary-compat/pom.xml | New module POM with dependencies for Maven resolution, Git operations, and archive handling |
| binary-compat/docker/Dockerfile | Docker configuration for running binary compatibility tests |
| binary-compat/src/main/java/com/jetbrains/youtrackdb/auto/BaseDBCompatibilityChecker.java | Main test orchestration class handling database exports, imports, and comparisons across versions |
| binary-compat/src/main/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/code/*.java | Code fetching infrastructure for Maven and Git-based artifacts |
| binary-compat/src/main/java/com/jetbrains/youtrackdb/auto/binarycompat/fetcher/data/*.java | Database downloading and preparation utilities |
| core/src/main/java/com/jetbrains/youtrackdb/internal/core/record/impl/EntityHelper.java | Enhanced entity comparison to support excluded properties |
| core/src/main/java/com/jetbrains/youtrackdb/internal/core/db/tool/DatabaseCompare.java | Updated database comparison to use property exclusion |
| tests/pom.xml | Added dependencies for binary compatibility testing |
| pom.xml | Added binary-compat module reference |
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,18 @@ | |||
| # This module is inteded to test binary compatibility of YouTrackDB releases. | |||
There was a problem hiding this comment.
Corrected spelling of 'inteded' to 'intended'.
| # This module is inteded to test binary compatibility of YouTrackDB releases. | |
| # This module is intended to test binary compatibility of YouTrackDB releases. |
|
|
||
| @Override | ||
| public void update(int completed) { | ||
| // this method it too verbose, we don't need it |
There was a problem hiding this comment.
Corrected grammar from 'it too verbose' to 'is too verbose'.
| // this method it too verbose, we don't need it | |
| // this method is too verbose, we don't need it |
| // this class loader "sees", all the classes from the current class loader which could issues, | ||
| // in case some classes are removed in the newer version of the code, but still present in the older one | ||
| // but without it, I will need a mechanism to load all the db dependencies, | ||
| // without specifying them manually one more time in the db compatibility test | ||
| // this is an open question for now. | ||
| // to deliver test sooner I will leave it as is with this comment |
There was a problem hiding this comment.
Corrected grammar from 'which could issues' to 'which could cause issues'.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 3 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,32 @@ | |||
| package com.jetbrains.youtrackdb.auto.binarycompat.fetcher.code.maven; | |||
|
|
|||
| import com.jetbrains.youtrackdb.auto.binarycompat.fetcher.code.github.MavenBuilder; | |||
There was a problem hiding this comment.
The import references a MavenBuilder interface from the github package, but this class is in the maven package. Consider moving the MavenBuilder interface to a more appropriate shared package or the maven package to avoid confusing package organization.
| import com.jetbrains.youtrackdb.auto.binarycompat.fetcher.code.github.MavenBuilder; | |
| import com.jetbrains.youtrackdb.auto.binarycompat.fetcher.code.maven.MavenBuilder; |
| try (var is = new BufferedInputStream(conn.getInputStream())) { | ||
| outputDir = untar(is); | ||
| } finally { | ||
| conn.disconnect(); | ||
| } |
There was a problem hiding this comment.
The HTTP connection doesn't validate content type or implement size limits. This could allow processing of malicious content. Consider adding content-type validation and maximum download size limits to prevent resource exhaustion attacks.
| // without specifying them manually one more time in the db compatibility test | ||
| // this is an open question for now. | ||
| // to deliver test sooner I will leave it as is with this comment | ||
| var loader = new URLClassLoader(new URL[]{importJars.toURI().toURL()}); |
There was a problem hiding this comment.
Creating a URLClassLoader with dynamically loaded JARs from external sources poses security risks. The loaded classes have full access to the JVM. Consider implementing security restrictions or validating JAR signatures before loading.
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 2 comments.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| FROM azul/zulu-openjdk:8 | ||
|
|
||
| MAINTAINER OrientDB LTD (info@orientdb.com) | ||
|
|
||
| RUN mkdir /orientdb |
There was a problem hiding this comment.
The removal of the MAINTAINER line is correct as it's deprecated, but the remaining OrientDB references (line 8) suggest this Dockerfile may not be properly updated for YouTrackDB usage.
| this.mavenBuilders = Map.of( | ||
| OS.LINUX, unixMavenBuilder, | ||
| OS.MAC, unixMavenBuilder, | ||
| OS.WINDOWS, new WindowsMavenBuilder() |
There was a problem hiding this comment.
Extra space before 'OS.WINDOWS' creates inconsistent indentation compared to other map entries.
| OS.WINDOWS, new WindowsMavenBuilder() | |
| OS.WINDOWS, new WindowsMavenBuilder() |
There was a problem hiding this comment.
Pull Request Overview
Copilot reviewed 28 out of 28 changed files in this pull request and generated 1 comment.
Tip: Customize your code reviews with copilot-instructions.md. Create the file or learn how to get started.
| @@ -0,0 +1,18 @@ | |||
| # This module is inteded to test binary compatibility of YouTrackDB releases. | |||
There was a problem hiding this comment.
Corrected spelling of 'inteded' to 'intended'.
| # This module is inteded to test binary compatibility of YouTrackDB releases. | |
| # This module is intended to test binary compatibility of YouTrackDB releases. |
This is a continuation of #459
The main goal of this PR is to introduce docker file which will allow users to run binary compatibility tests on their private databases in isolated environments.