Skip to content

Commit 6dc3f39

Browse files
TeddyCrpmbrull
authored andcommitted
Minor migration fix (#26592)
* fix: process migration not executed * fix: tests for process migration not executed * fix: O(n) -> O(1)
1 parent 79015d4 commit 6dc3f39

2 files changed

Lines changed: 201 additions & 19 deletions

File tree

openmetadata-service/src/main/java/org/openmetadata/service/migration/api/MigrationWorkflow.java

Lines changed: 11 additions & 19 deletions
Original file line numberDiff line numberDiff line change
@@ -8,10 +8,12 @@
88
import java.nio.charset.StandardCharsets;
99
import java.util.ArrayList;
1010
import java.util.Arrays;
11+
import java.util.HashSet;
1112
import java.util.List;
1213
import java.util.Map;
1314
import java.util.Objects;
1415
import java.util.Optional;
16+
import java.util.Set;
1517
import java.util.UUID;
1618
import java.util.regex.Matcher;
1719
import java.util.regex.Pattern;
@@ -246,33 +248,23 @@ private static int[] parseVersion(String version) {
246248
*/
247249
public List<MigrationFile> getMigrationsToApply(
248250
List<String> executedMigrations, List<MigrationFile> availableMigrations) {
251+
Set<String> executedSet = new HashSet<>(executedMigrations);
249252
List<MigrationFile> migrationsToApply = new ArrayList<>();
250-
List<MigrationFile> nativeMigrationsToApply =
251-
processNativeMigrations(executedMigrations, availableMigrations);
252-
List<MigrationFile> extensionMigrationsToApply =
253-
processExtensionMigrations(executedMigrations, availableMigrations);
254-
255-
migrationsToApply.addAll(nativeMigrationsToApply);
256-
migrationsToApply.addAll(extensionMigrationsToApply);
253+
migrationsToApply.addAll(processNativeMigrations(executedSet, availableMigrations));
254+
migrationsToApply.addAll(processExtensionMigrations(executedSet, availableMigrations));
257255
return migrationsToApply;
258256
}
259257

260258
private List<MigrationFile> processNativeMigrations(
261-
List<String> executedMigrations, List<MigrationFile> availableMigrations) {
262-
Stream<MigrationFile> availableNativeMigrations =
263-
availableMigrations.stream().filter(migration -> !migration.isExtension);
264-
Optional<String> maxMigration =
265-
executedMigrations.stream().max(MigrationWorkflow::compareVersions);
266-
if (maxMigration.isPresent()) {
267-
return availableNativeMigrations
268-
.filter(migration -> migration.biggerThan(maxMigration.get()))
269-
.toList();
270-
}
271-
return availableNativeMigrations.toList();
259+
Set<String> executedMigrations, List<MigrationFile> availableMigrations) {
260+
return availableMigrations.stream()
261+
.filter(migration -> !migration.isExtension)
262+
.filter(migration -> !executedMigrations.contains(migration.version))
263+
.toList();
272264
}
273265

274266
private List<MigrationFile> processExtensionMigrations(
275-
List<String> executedMigrations, List<MigrationFile> availableMigrations) {
267+
Set<String> executedMigrations, List<MigrationFile> availableMigrations) {
276268
return availableMigrations.stream()
277269
.filter(migration -> migration.isExtension)
278270
.filter(migration -> !executedMigrations.contains(migration.version))
Lines changed: 190 additions & 0 deletions
Original file line numberDiff line numberDiff line change
@@ -0,0 +1,190 @@
1+
package org.openmetadata.service.migration.api;
2+
3+
import static org.junit.jupiter.api.Assertions.assertEquals;
4+
import static org.junit.jupiter.api.Assertions.assertTrue;
5+
import static org.mockito.ArgumentMatchers.eq;
6+
import static org.mockito.Mockito.mock;
7+
import static org.mockito.Mockito.when;
8+
9+
import java.io.File;
10+
import java.io.IOException;
11+
import java.nio.file.Files;
12+
import java.nio.file.Path;
13+
import java.util.ArrayList;
14+
import java.util.Comparator;
15+
import java.util.List;
16+
import java.util.stream.Stream;
17+
import org.jdbi.v3.core.Jdbi;
18+
import org.junit.jupiter.api.AfterEach;
19+
import org.junit.jupiter.api.BeforeEach;
20+
import org.junit.jupiter.api.Test;
21+
import org.openmetadata.service.OpenMetadataApplicationConfig;
22+
import org.openmetadata.service.jdbi3.MigrationDAO;
23+
import org.openmetadata.service.jdbi3.locator.ConnectionType;
24+
import org.openmetadata.service.migration.utils.MigrationFile;
25+
26+
class MigrationWorkflowTest {
27+
28+
private Path tempDir;
29+
private MigrationDAO migrationDAO;
30+
private OpenMetadataApplicationConfig config;
31+
private MigrationWorkflow workflow;
32+
33+
@BeforeEach
34+
void setUp() throws IOException {
35+
tempDir = Files.createTempDirectory("migration-test");
36+
migrationDAO = mock(MigrationDAO.class);
37+
config = mock(OpenMetadataApplicationConfig.class);
38+
Jdbi jdbi = mock(Jdbi.class);
39+
when(jdbi.onDemand(eq(MigrationDAO.class))).thenReturn(migrationDAO);
40+
workflow =
41+
new MigrationWorkflow(
42+
jdbi, tempDir.toString(), ConnectionType.MYSQL, null, null, config, false);
43+
}
44+
45+
@AfterEach
46+
void tearDown() throws IOException {
47+
try (Stream<Path> paths = Files.walk(tempDir)) {
48+
paths.sorted(Comparator.reverseOrder()).map(Path::toFile).forEach(File::delete);
49+
}
50+
}
51+
52+
private MigrationFile createMigrationFile(String version, boolean isExtension)
53+
throws IOException {
54+
Path parentDir = isExtension ? tempDir.resolve("extensions") : tempDir;
55+
Files.createDirectories(parentDir);
56+
Path versionDir = Files.createDirectory(parentDir.resolve(version));
57+
return new MigrationFile(
58+
versionDir.toFile(), migrationDAO, ConnectionType.MYSQL, config, isExtension);
59+
}
60+
61+
@Test
62+
void getMigrationsToApply_backportedPatchVersionsAreIncluded() throws IOException {
63+
List<String> executedMigrations = List.of("1.11.10", "1.12.0", "1.12.1");
64+
List<MigrationFile> availableMigrations =
65+
List.of(
66+
createMigrationFile("1.11.10", false),
67+
createMigrationFile("1.11.11", false),
68+
createMigrationFile("1.11.12", false),
69+
createMigrationFile("1.12.0", false),
70+
createMigrationFile("1.12.1", false),
71+
createMigrationFile("1.12.2", false));
72+
73+
List<MigrationFile> result =
74+
workflow.getMigrationsToApply(executedMigrations, availableMigrations);
75+
76+
List<String> versions = result.stream().map(m -> m.version).toList();
77+
assertEquals(List.of("1.11.11", "1.11.12", "1.12.2"), versions);
78+
}
79+
80+
@Test
81+
void getMigrationsToApply_collateVersionsAreIncluded() throws IOException {
82+
List<String> executedMigrations = List.of("1.11.10", "1.12.0", "1.12.1");
83+
List<MigrationFile> availableMigrations =
84+
List.of(
85+
createMigrationFile("1.11.10", false),
86+
createMigrationFile("1.11.11", false),
87+
createMigrationFile("1.12.0", false),
88+
createMigrationFile("1.12.1", false),
89+
createMigrationFile("1.12.1-collate", false),
90+
createMigrationFile("1.12.2", false));
91+
92+
List<MigrationFile> result =
93+
workflow.getMigrationsToApply(executedMigrations, availableMigrations);
94+
95+
List<String> versions = result.stream().map(m -> m.version).toList();
96+
assertEquals(List.of("1.11.11", "1.12.1-collate", "1.12.2"), versions);
97+
}
98+
99+
@Test
100+
void getMigrationsToApply_collateVersionAlreadyExecuted() throws IOException {
101+
List<String> executedMigrations = List.of("1.11.10", "1.12.0", "1.12.1", "1.12.1-collate");
102+
List<MigrationFile> availableMigrations =
103+
List.of(
104+
createMigrationFile("1.11.10", false),
105+
createMigrationFile("1.12.0", false),
106+
createMigrationFile("1.12.1", false),
107+
createMigrationFile("1.12.1-collate", false),
108+
createMigrationFile("1.12.2", false));
109+
110+
List<MigrationFile> result =
111+
workflow.getMigrationsToApply(executedMigrations, availableMigrations);
112+
113+
List<String> versions = result.stream().map(m -> m.version).toList();
114+
assertEquals(List.of("1.12.2"), versions);
115+
}
116+
117+
@Test
118+
void getMigrationsToApply_extensionMigrationsProcessedSeparately() throws IOException {
119+
List<String> executedMigrations = List.of("1.12.0", "1.12.1");
120+
List<MigrationFile> availableMigrations =
121+
List.of(
122+
createMigrationFile("1.12.0", false),
123+
createMigrationFile("1.12.1", false),
124+
createMigrationFile("1.12.2", false),
125+
createMigrationFile("1.12.1", true),
126+
createMigrationFile("1.12.2", true));
127+
128+
List<MigrationFile> result =
129+
workflow.getMigrationsToApply(executedMigrations, availableMigrations);
130+
131+
List<String> nativeVersions =
132+
result.stream().filter(m -> !m.isExtension).map(m -> m.version).toList();
133+
List<String> extensionVersions =
134+
result.stream().filter(m -> m.isExtension).map(m -> m.version).toList();
135+
136+
assertEquals(List.of("1.12.2"), nativeVersions);
137+
assertEquals(List.of("1.12.2"), extensionVersions);
138+
}
139+
140+
@Test
141+
void getMigrationsToApply_noExecutedMigrationsReturnsAll() throws IOException {
142+
List<String> executedMigrations = new ArrayList<>();
143+
List<MigrationFile> availableMigrations =
144+
List.of(
145+
createMigrationFile("1.11.10", false),
146+
createMigrationFile("1.12.0", false),
147+
createMigrationFile("1.12.0-collate", false));
148+
149+
List<MigrationFile> result =
150+
workflow.getMigrationsToApply(executedMigrations, availableMigrations);
151+
152+
assertEquals(3, result.size());
153+
}
154+
155+
@Test
156+
void getMigrationsToApply_allMigrationsAlreadyExecuted() throws IOException {
157+
List<String> executedMigrations = List.of("1.12.0", "1.12.1", "1.12.1-collate");
158+
List<MigrationFile> availableMigrations =
159+
List.of(
160+
createMigrationFile("1.12.0", false),
161+
createMigrationFile("1.12.1", false),
162+
createMigrationFile("1.12.1-collate", false));
163+
164+
List<MigrationFile> result =
165+
workflow.getMigrationsToApply(executedMigrations, availableMigrations);
166+
167+
assertTrue(result.isEmpty());
168+
}
169+
170+
@Test
171+
void getMigrationsToApply_multipleBackportedMinorVersions() throws IOException {
172+
List<String> executedMigrations = List.of("1.10.5", "1.11.0", "1.12.0", "1.12.1");
173+
List<MigrationFile> availableMigrations =
174+
List.of(
175+
createMigrationFile("1.10.5", false),
176+
createMigrationFile("1.10.6", false),
177+
createMigrationFile("1.11.0", false),
178+
createMigrationFile("1.11.1", false),
179+
createMigrationFile("1.11.1-collate", false),
180+
createMigrationFile("1.12.0", false),
181+
createMigrationFile("1.12.1", false),
182+
createMigrationFile("1.12.2", false));
183+
184+
List<MigrationFile> result =
185+
workflow.getMigrationsToApply(executedMigrations, availableMigrations);
186+
187+
List<String> versions = result.stream().map(m -> m.version).toList();
188+
assertEquals(List.of("1.10.6", "1.11.1", "1.11.1-collate", "1.12.2"), versions);
189+
}
190+
}

0 commit comments

Comments
 (0)