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

fixed #182: stable and system independent checksum calculation #183

Merged
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
2 changes: 1 addition & 1 deletion .mvn/wrapper/maven-wrapper.properties
Original file line number Diff line number Diff line change
Expand Up @@ -14,5 +14,5 @@
# KIND, either express or implied. See the License for the
# specific language governing permissions and limitations
# under the License.
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.8.5/apache-maven-3.8.5-bin.zip
distributionUrl=https://repo.maven.apache.org/maven2/org/apache/maven/apache-maven/3.9.0/apache-maven-3.9.0-bin.zip
wrapperUrl=https://repo.maven.apache.org/maven2/org/apache/maven/wrapper/maven-wrapper/3.1.0/maven-wrapper-3.1.0.jar
2 changes: 2 additions & 0 deletions README.md
Original file line number Diff line number Diff line change
Expand Up @@ -194,6 +194,7 @@ Elasticsearch-Evolution can be configured to your needs:
- **historyMaxQuerySize** (default=1000): The maximum query size while validating already executed scripts. This query size have to be higher than the total count of your migration scripts.
- **validateOnMigrate** (default=true): Whether to fail when a previously applied migration script has been modified after it was applied.
- **baselineVersion** (default=1.0): Version to use as a baseline. versions lower than it will not be applied.
- **lineSeparator** (default=\n): Line separator, used only temporary between reading raw migration file line-by-line and parsing it later. Only needed for backward compatibility / checksum stability! Should be one of `\n`, `\r` or `\r\n`

### 5.1 Spring Boot

Expand Down Expand Up @@ -290,6 +291,7 @@ ElasticsearchEvolution.configure()

### v0.4.2-SNAPSHOT

- bugfix ([#182](https://github.com/senacor/elasticsearch-evolution/issues/182)): checksum calculation was based on system dependent line separators which lead to different checksums on different operating systems (e.g. windows vs linux). The default is now `\n`. For backward compatibility you can set other line separator via `lineSeparator` config property.
- version updates (spring-boot 2.7.8)
- spring boot 3 compatibility + tests
- added Opensearch 2.5 compatibility tests
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -149,7 +149,8 @@ protected MigrationScriptReader createMigrationScriptReader() {
getConfig().getLocations(),
getConfig().getEncoding(),
getConfig().getEsMigrationPrefix(),
getConfig().getEsMigrationSuffixes());
getConfig().getEsMigrationSuffixes(),
getConfig().getLineSeparator());
}

protected HistoryRepository createHistoryRepository() {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -36,6 +36,17 @@ public class ElasticsearchEvolutionConfig {
*/
private Charset encoding = StandardCharsets.UTF_8;

/**
* Line separator, used only temporary between reading raw migration file line-by-line and parsing it later.
* Only needed for backward compatibility / checksum stability!
* <p>
* Should be one of
* - '\n' (LF - Linux/Unix/OS X)
* - '\r' (CR - Classic MAC OS)
* - '\r\n' (CRLF - Windows)
*/
private String lineSeparator = "\n";

/**
* This content type will be used as default if no contentType header is specified in the header section of a migration script.
* If no charset is defined, the {@link #encoding} charset is used.
Expand Down Expand Up @@ -172,6 +183,15 @@ public ElasticsearchEvolutionConfig setEncoding(Charset encoding) {
return this;
}

public String getLineSeparator() {
return lineSeparator;
}

public ElasticsearchEvolutionConfig setLineSeparator(String lineSeparator) {
this.lineSeparator = lineSeparator;
return this;
}

public String getDefaultContentType() {
return defaultContentType;
}
Expand Down Expand Up @@ -277,6 +297,7 @@ public String toString() {
"enabled=" + enabled +
", locations=" + locations +
", encoding=" + encoding +
", lineSeparator='" + lineSeparator.replace("\n", "\\n").replace("\r", "\\r") + '\'' +
", defaultContentType='" + defaultContentType + '\'' +
", esMigrationPrefix='" + esMigrationPrefix + '\'' +
", esMigrationSuffixes=" + esMigrationSuffixes +
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,22 +45,26 @@ public class MigrationScriptReaderImpl implements MigrationScriptReader {
private final Charset encoding;
private final String esMigrationPrefix;
private final List<String> esMigrationSuffixes;
private final String lineSeparator;

/**
* @param locations Locations of migrations scripts, e.g classpath:es/migration or file:/home/migration
* @param encoding migrations scripts encoding
* @param esMigrationFilePrefix File name prefix for ES migrations.
* @param esMigrationFileSuffixes File name suffix for ES migrations.
* @param lineSeparator Line separator. should be '\n' per default and only something else for backward compatibility / hash stability
*/

public MigrationScriptReaderImpl(List<String> locations,
Charset encoding,
String esMigrationFilePrefix,
List<String> esMigrationFileSuffixes) {
List<String> esMigrationFileSuffixes,
String lineSeparator) {
this.locations = locations;
this.encoding = encoding;
this.esMigrationPrefix = esMigrationFilePrefix;
this.esMigrationSuffixes = esMigrationFileSuffixes;
this.lineSeparator = lineSeparator;
}

/**
Expand Down Expand Up @@ -162,7 +166,8 @@ private Stream<RawMigrationScript> readScriptsFromClassPath(String location) {

private Stream<RawMigrationScript> read(BufferedReader reader, String filename) {
String content = reader.lines()
.collect(Collectors.joining(System.lineSeparator()));
// use static line separator ('\n' per default) to get predictable and system independent checksum later
.collect(Collectors.joining(lineSeparator));
if (content.isEmpty()) {
return Stream.empty();
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -32,7 +32,7 @@ void nonJarFile() {
singletonList("classpath:scriptreader"),
StandardCharsets.UTF_8,
"c",
singletonList(".http"));
singletonList(".http"), "\n");
List<RawMigrationScript> actual = reader.read();
assertThat(actual)
.containsExactlyInAnyOrder(
Expand All @@ -46,7 +46,7 @@ void inJarFile() {
singletonList("classpath:META-INF"),
StandardCharsets.UTF_8,
"MANIFEST",
singletonList(".MF"));
singletonList(".MF"), "\n");

List<RawMigrationScript> res = reader.read();

Expand All @@ -61,7 +61,7 @@ void invalidClasspath() {
singletonList(classpath),
StandardCharsets.UTF_8,
"c",
singletonList(".http")) {
singletonList(".http"), "\n") {
@Override
protected Stream<RawMigrationScript> readFromLocation(String location) throws URISyntaxException, IOException {
throw new URISyntaxException("input", "reason");
Expand All @@ -79,7 +79,7 @@ void multipleSuffixes() {
singletonList("classpath:scriptreader"),
StandardCharsets.UTF_8,
"c",
Arrays.asList(".http", ".other"));
Arrays.asList(".http", ".other"), "\n");
List<RawMigrationScript> actual = reader.read();
assertThat(actual)
.containsExactlyInAnyOrder(
Expand All @@ -94,7 +94,7 @@ void handlingDuplicates() {
Arrays.asList("classpath:scriptreader", "classpath:scriptreader"),
StandardCharsets.UTF_8,
"c",
Arrays.asList(".http", ".other"));
Arrays.asList(".http", ".other"), "\n");
List<RawMigrationScript> actual = reader.read();
assertThat(actual)
.containsExactlyInAnyOrder(
Expand All @@ -109,7 +109,7 @@ void exclude_locations_with_suffix() {
singletonList("classpath:scriptreader/issue36/location"),
StandardCharsets.UTF_8,
"c",
singletonList(".http"));
singletonList(".http"), "\n");

List<RawMigrationScript> actual = reader.read();

Expand All @@ -126,7 +126,7 @@ void handle_locations_with_suffix() {
"classpath:scriptreader/issue36/location_with_suffix"),
StandardCharsets.UTF_8,
"c",
singletonList(".http"));
singletonList(".http"), "\n");

List<RawMigrationScript> actual = reader.read();

Expand All @@ -142,7 +142,7 @@ void withWrongProtocol() {
Arrays.asList("classpath:scriptreader", "http:scriptreader"),
StandardCharsets.UTF_8,
"c",
Arrays.asList(".http", ".other"));
Arrays.asList(".http", ".other"), "\n");
assertThatThrownBy(reader::read)
.isInstanceOf(MigrationException.class)
.hasMessage("could not read location path http:scriptreader, should look like this: " +
Expand All @@ -160,7 +160,7 @@ void normalPath() throws URISyntaxException {
singletonList("file:" + absolutePathToScriptreader),
StandardCharsets.UTF_8,
"c",
singletonList(".http"));
singletonList(".http"), "\n");
List<RawMigrationScript> actual = reader.read();
assertThat(actual)
.containsExactlyInAnyOrder(
Expand All @@ -176,7 +176,7 @@ void exclude_locations_with_suffix() throws URISyntaxException {
singletonList("file:"+absolutePathToScriptreader+"/issue36/location"),
StandardCharsets.UTF_8,
"c",
singletonList(".http"));
singletonList(".http"), "\n");

List<RawMigrationScript> actual = reader.read();

Expand All @@ -195,7 +195,7 @@ void handle_locations_with_suffix() throws URISyntaxException {
"file:"+absolutePathToScriptreader+"/issue36/location_with_suffix"),
StandardCharsets.UTF_8,
"c",
singletonList(".http"));
singletonList(".http"), "\n");

List<RawMigrationScript> actual = reader.read();

Expand All @@ -212,7 +212,7 @@ void invalidPath() {
singletonList("file:X:/snc/scripts"),
StandardCharsets.UTF_8,
"c",
singletonList(".http")).read())
singletonList(".http"), "\n").read())
.isInstanceOf(MigrationException.class)
.hasMessage("couldn't read scripts from file:X:/snc/scripts");
}
Expand All @@ -227,7 +227,7 @@ void validAndInvalidPath() throws URISyntaxException {
Arrays.asList("file:X:/snc/scripts", "file:" + absolutePathToScriptreader),
StandardCharsets.UTF_8,
"c",
singletonList(".http")).read())
singletonList(".http"), "\n").read())
.isInstanceOf(MigrationException.class)
.hasMessage("couldn't read scripts from file:X:/snc/scripts");
}
Expand All @@ -240,7 +240,7 @@ void validPathButNoFiles() throws URISyntaxException {
singletonList("file:" + absolutePathToScriptreader),
StandardCharsets.UTF_8,
"d",
singletonList(".http"));
singletonList(".http"), "\n");
List<RawMigrationScript> actual = reader.read();
assertThat(actual).isEmpty();
}
Expand Down