Skip to content

Commit

Permalink
refactor confusing methods (airbytehq#6641)
Browse files Browse the repository at this point in the history
  • Loading branch information
cgardens authored Oct 15, 2021
1 parent d154f8b commit c129e02
Show file tree
Hide file tree
Showing 7 changed files with 39 additions and 49 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -135,10 +135,7 @@ public List<StandardSourceDefinition> listStandardSourceDefinitions() throws Jso
}

public void writeStandardSourceDefinition(final StandardSourceDefinition sourceDefinition) throws JsonValidationException, IOException {
persistence.writeConfig(
ConfigSchema.STANDARD_SOURCE_DEFINITION,
sourceDefinition.getSourceDefinitionId().toString(),
sourceDefinition);
persistence.writeConfig(ConfigSchema.STANDARD_SOURCE_DEFINITION, sourceDefinition.getSourceDefinitionId().toString(), sourceDefinition);
}

public void deleteStandardSourceDefinition(final UUID sourceDefId) throws IOException {
Expand All @@ -149,14 +146,6 @@ public void deleteStandardSourceDefinition(final UUID sourceDefId) throws IOExce
}
}

public List<StandardSourceDefinition> listStandardSources() throws JsonValidationException, IOException {
return persistence.listConfigs(ConfigSchema.STANDARD_SOURCE_DEFINITION, StandardSourceDefinition.class);
}

public void writeStandardSource(final StandardSourceDefinition source) throws JsonValidationException, IOException {
persistence.writeConfig(ConfigSchema.STANDARD_SOURCE_DEFINITION, source.getSourceDefinitionId().toString(), source);
}

public StandardDestinationDefinition getStandardDestinationDefinition(final UUID destinationDefinitionId)
throws JsonValidationException, IOException, ConfigNotFoundException {
return persistence.getConfig(ConfigSchema.STANDARD_DESTINATION_DEFINITION, destinationDefinitionId.toString(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -116,7 +116,7 @@ public void testMissingObjectsProperException() {

@Test
public void testSource() throws IOException, JsonValidationException {
configRepository.writeStandardSource(SOURCE_DEF);
configRepository.writeStandardSourceDefinition(SOURCE_DEF);
configRepository.writeSourceConnection(SOURCE, emptyConnectorSpec);

final UUID retrievedWorkspace = workspaceHelper.getWorkspaceForSourceIdIgnoreExceptions(SOURCE_ID);
Expand Down Expand Up @@ -144,7 +144,7 @@ public void testDestination() throws IOException, JsonValidationException {

@Test
public void testConnection() throws IOException, JsonValidationException {
configRepository.writeStandardSource(SOURCE_DEF);
configRepository.writeStandardSourceDefinition(SOURCE_DEF);
configRepository.writeSourceConnection(SOURCE, emptyConnectorSpec);
configRepository.writeStandardDestinationDefinition(DEST_DEF);
configRepository.writeDestinationConnection(DEST, emptyConnectorSpec);
Expand Down Expand Up @@ -184,7 +184,7 @@ public void testOperation() throws IOException, JsonValidationException {

@Test
public void testConnectionAndJobs() throws IOException, JsonValidationException {
configRepository.writeStandardSource(SOURCE_DEF);
configRepository.writeStandardSourceDefinition(SOURCE_DEF);
configRepository.writeSourceConnection(SOURCE, emptyConnectorSpec);
configRepository.writeStandardDestinationDefinition(DEST_DEF);
configRepository.writeDestinationConnection(DEST, emptyConnectorSpec);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -56,7 +56,7 @@ public SourceDefinitionsHandler(
}

@VisibleForTesting
static SourceDefinitionRead buildSourceDefinitionRead(StandardSourceDefinition standardSourceDefinition) {
static SourceDefinitionRead buildSourceDefinitionRead(final StandardSourceDefinition standardSourceDefinition) {
try {
return new SourceDefinitionRead()
.sourceDefinitionId(standardSourceDefinition.getSourceDefinitionId())
Expand All @@ -65,16 +65,16 @@ static SourceDefinitionRead buildSourceDefinitionRead(StandardSourceDefinition s
.dockerImageTag(standardSourceDefinition.getDockerImageTag())
.documentationUrl(new URI(standardSourceDefinition.getDocumentationUrl()))
.icon(loadIcon(standardSourceDefinition.getIcon()));
} catch (URISyntaxException | NullPointerException e) {
} catch (final URISyntaxException | NullPointerException e) {
throw new InternalServerKnownException("Unable to process retrieved latest source definitions list", e);
}
}

public SourceDefinitionReadList listSourceDefinitions() throws IOException, JsonValidationException {
return toSourceDefinitionReadList(configRepository.listStandardSources());
return toSourceDefinitionReadList(configRepository.listStandardSourceDefinitions());
}

private static SourceDefinitionReadList toSourceDefinitionReadList(List<StandardSourceDefinition> defs) {
private static SourceDefinitionReadList toSourceDefinitionReadList(final List<StandardSourceDefinition> defs) {
final List<SourceDefinitionRead> reads = defs.stream()
.map(SourceDefinitionsHandler::buildSourceDefinitionRead)
.collect(Collectors.toList());
Expand All @@ -88,17 +88,18 @@ public SourceDefinitionReadList listLatestSourceDefinitions() {
private List<StandardSourceDefinition> getLatestSources() {
try {
return githubStore.getLatestSources();
} catch (InterruptedException e) {
} catch (final InterruptedException e) {
throw new InternalServerKnownException("Request to retrieve latest destination definitions failed", e);
}
}

public SourceDefinitionRead getSourceDefinition(SourceDefinitionIdRequestBody sourceDefinitionIdRequestBody)
public SourceDefinitionRead getSourceDefinition(final SourceDefinitionIdRequestBody sourceDefinitionIdRequestBody)
throws ConfigNotFoundException, IOException, JsonValidationException {
return buildSourceDefinitionRead(configRepository.getStandardSourceDefinition(sourceDefinitionIdRequestBody.getSourceDefinitionId()));
}

public SourceDefinitionRead createSourceDefinition(SourceDefinitionCreate sourceDefinitionCreate) throws JsonValidationException, IOException {
public SourceDefinitionRead createSourceDefinition(final SourceDefinitionCreate sourceDefinitionCreate)
throws JsonValidationException, IOException {
imageValidator.assertValidIntegrationImage(sourceDefinitionCreate.getDockerRepository(), sourceDefinitionCreate.getDockerImageTag());

final UUID id = uuidSupplier.get();
Expand All @@ -110,12 +111,12 @@ public SourceDefinitionRead createSourceDefinition(SourceDefinitionCreate source
.withName(sourceDefinitionCreate.getName())
.withIcon(sourceDefinitionCreate.getIcon());

configRepository.writeStandardSource(sourceDefinition);
configRepository.writeStandardSourceDefinition(sourceDefinition);

return buildSourceDefinitionRead(sourceDefinition);
}

public SourceDefinitionRead updateSourceDefinition(SourceDefinitionUpdate sourceDefinitionUpdate)
public SourceDefinitionRead updateSourceDefinition(final SourceDefinitionUpdate sourceDefinitionUpdate)
throws ConfigNotFoundException, IOException, JsonValidationException {
final StandardSourceDefinition currentSourceDefinition =
configRepository.getStandardSourceDefinition(sourceDefinitionUpdate.getSourceDefinitionId());
Expand All @@ -129,16 +130,16 @@ public SourceDefinitionRead updateSourceDefinition(SourceDefinitionUpdate source
.withName(currentSourceDefinition.getName())
.withIcon(currentSourceDefinition.getIcon());

configRepository.writeStandardSource(newSource);
configRepository.writeStandardSourceDefinition(newSource);
// we want to re-fetch the spec for updated definitions.
schedulerSynchronousClient.resetCache();
return buildSourceDefinitionRead(newSource);
}

public static String loadIcon(String name) {
public static String loadIcon(final String name) {
try {
return name == null ? null : MoreResources.readResource("icons/" + name);
} catch (Exception e) {
} catch (final Exception e) {
return null;
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -93,7 +93,7 @@ public void setup() throws IOException, JsonValidationException, ConfigNotFoundE
.withName("test-source")
.withTombstone(false)
.withWorkspaceId(workspaceId);
when(configRepository.listStandardSources())
when(configRepository.listStandardSourceDefinitions())
.thenReturn(List.of(standardSourceDefinition));
when(configRepository.getStandardSourceDefinition(standardSourceDefinition.getSourceDefinitionId()))
.thenReturn(standardSourceDefinition);
Expand Down Expand Up @@ -230,31 +230,31 @@ public void testImportIntoWorkspaceWithoutConflicts() throws JsonValidationExcep

@Test
public void testReplaceDeploymentMetadata() throws Exception {
UUID oldDeploymentUuid = UUID.randomUUID();
UUID newDeploymentUuid = UUID.randomUUID();
final UUID oldDeploymentUuid = UUID.randomUUID();
final UUID newDeploymentUuid = UUID.randomUUID();

JsonNode airbyteVersion = Jsons.deserialize("{\"key\":\"airbyte_version\",\"value\":\"dev\"}");
JsonNode serverUuid = Jsons.deserialize("{\"key\":\"server_uuid\",\"value\":\"e895a584-7dbf-48ce-ace6-0bc9ea570c34\"}");
JsonNode date = Jsons.deserialize("{\"key\":\"date\",\"value\":\"1956-08-17\"}");
JsonNode oldDeploymentId = Jsons.deserialize(
final JsonNode airbyteVersion = Jsons.deserialize("{\"key\":\"airbyte_version\",\"value\":\"dev\"}");
final JsonNode serverUuid = Jsons.deserialize("{\"key\":\"server_uuid\",\"value\":\"e895a584-7dbf-48ce-ace6-0bc9ea570c34\"}");
final JsonNode date = Jsons.deserialize("{\"key\":\"date\",\"value\":\"1956-08-17\"}");
final JsonNode oldDeploymentId = Jsons.deserialize(
String.format("{\"key\":\"%s\",\"value\":\"%s\"}", DefaultJobPersistence.DEPLOYMENT_ID_KEY, oldDeploymentUuid));
JsonNode newDeploymentId = Jsons.deserialize(
final JsonNode newDeploymentId = Jsons.deserialize(
String.format("{\"key\":\"%s\",\"value\":\"%s\"}", DefaultJobPersistence.DEPLOYMENT_ID_KEY, newDeploymentUuid));

JobPersistence jobPersistence = mock(JobPersistence.class);
final JobPersistence jobPersistence = mock(JobPersistence.class);

// when new deployment id does not exist, the old deployment id is removed
when(jobPersistence.getDeployment()).thenReturn(Optional.empty());
Stream<JsonNode> inputStream1 = Stream.of(airbyteVersion, serverUuid, date, oldDeploymentId);
Stream<JsonNode> outputStream1 = ConfigDumpImporter.replaceDeploymentMetadata(jobPersistence, inputStream1);
Stream<JsonNode> expectedStream1 = Stream.of(airbyteVersion, serverUuid, date);
final Stream<JsonNode> inputStream1 = Stream.of(airbyteVersion, serverUuid, date, oldDeploymentId);
final Stream<JsonNode> outputStream1 = ConfigDumpImporter.replaceDeploymentMetadata(jobPersistence, inputStream1);
final Stream<JsonNode> expectedStream1 = Stream.of(airbyteVersion, serverUuid, date);
assertEquals(expectedStream1.collect(Collectors.toList()), outputStream1.collect(Collectors.toList()));

// when new deployment id exists, the old deployment id is replaced with the new one
when(jobPersistence.getDeployment()).thenReturn(Optional.of(newDeploymentUuid));
Stream<JsonNode> inputStream2 = Stream.of(airbyteVersion, serverUuid, date, oldDeploymentId);
Stream<JsonNode> outputStream2 = ConfigDumpImporter.replaceDeploymentMetadata(jobPersistence, inputStream2);
Stream<JsonNode> expectedStream2 = Stream.of(airbyteVersion, serverUuid, date, newDeploymentId);
final Stream<JsonNode> inputStream2 = Stream.of(airbyteVersion, serverUuid, date, oldDeploymentId);
final Stream<JsonNode> outputStream2 = ConfigDumpImporter.replaceDeploymentMetadata(jobPersistence, inputStream2);
final Stream<JsonNode> expectedStream2 = Stream.of(airbyteVersion, serverUuid, date, newDeploymentId);
assertEquals(expectedStream2.collect(Collectors.toList()), outputStream2.collect(Collectors.toList()));
}

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -290,7 +290,7 @@ private void setupTestData(final UUID workspaceId) throws JsonValidationExceptio
configPersistence.writeConfig(ConfigSchema.SOURCE_CONNECTION, sourceid.toString(), new SourceConnection()
.withSourceId(sourceid)
.withWorkspaceId(workspaceId)
.withSourceDefinitionId(configRepository.listStandardSources().get(0).getSourceDefinitionId())
.withSourceDefinitionId(configRepository.listStandardSourceDefinitions().get(0).getSourceDefinitionId())
.withName("test-source")
.withConfiguration(Jsons.emptyObject())
.withTombstone(false));
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -77,17 +77,17 @@ private StandardSourceDefinition generateSource() {
void testListSourceDefinitions() throws JsonValidationException, IOException, URISyntaxException {
final StandardSourceDefinition source2 = generateSource();

when(configRepository.listStandardSources()).thenReturn(Lists.newArrayList(source, source2));
when(configRepository.listStandardSourceDefinitions()).thenReturn(Lists.newArrayList(source, source2));

SourceDefinitionRead expectedSourceDefinitionRead1 = new SourceDefinitionRead()
final SourceDefinitionRead expectedSourceDefinitionRead1 = new SourceDefinitionRead()
.sourceDefinitionId(source.getSourceDefinitionId())
.name(source.getName())
.dockerRepository(source.getDockerRepository())
.dockerImageTag(source.getDockerImageTag())
.documentationUrl(new URI(source.getDocumentationUrl()))
.icon(SourceDefinitionsHandler.loadIcon(source.getIcon()));

SourceDefinitionRead expectedSourceDefinitionRead2 = new SourceDefinitionRead()
final SourceDefinitionRead expectedSourceDefinitionRead2 = new SourceDefinitionRead()
.sourceDefinitionId(source2.getSourceDefinitionId())
.name(source2.getName())
.dockerRepository(source.getDockerRepository())
Expand All @@ -107,7 +107,7 @@ void testGetSourceDefinition() throws JsonValidationException, ConfigNotFoundExc
when(configRepository.getStandardSourceDefinition(source.getSourceDefinitionId()))
.thenReturn(source);

SourceDefinitionRead expectedSourceDefinitionRead = new SourceDefinitionRead()
final SourceDefinitionRead expectedSourceDefinitionRead = new SourceDefinitionRead()
.sourceDefinitionId(source.getSourceDefinitionId())
.name(source.getName())
.dockerRepository(source.getDockerRepository())
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -113,7 +113,7 @@ private void assertPreMigrationConfigs(final Path configRoot, final JobPersisten
final ConfigRepository configRepository =
new ConfigRepository(FileSystemConfigPersistence.createWithValidation(configRoot), new NoOpSecretsHydrator(), Optional.of(secretPersistence),
Optional.of(secretPersistence));
final Map<String, StandardSourceDefinition> sourceDefinitionsBeforeMigration = configRepository.listStandardSources().stream()
final Map<String, StandardSourceDefinition> sourceDefinitionsBeforeMigration = configRepository.listStandardSourceDefinitions().stream()
.collect(Collectors.toMap(c -> c.getSourceDefinitionId().toString(), c -> c));
assertTrue(sourceDefinitionsBeforeMigration.containsKey(DEPRECATED_SOURCE_DEFINITION_NOT_BEING_USED));
assertTrue(sourceDefinitionsBeforeMigration.containsKey(DEPRECATED_SOURCE_DEFINITION_BEING_USED));
Expand Down Expand Up @@ -144,7 +144,7 @@ private void assertPostMigrationConfigs(final Path importRoot) throws Exception
}

private void assertSourceDefinitions(final ConfigRepository configRepository) throws JsonValidationException, IOException {
final Map<String, StandardSourceDefinition> sourceDefinitions = configRepository.listStandardSources()
final Map<String, StandardSourceDefinition> sourceDefinitions = configRepository.listStandardSourceDefinitions()
.stream()
.collect(Collectors.toMap(c -> c.getSourceDefinitionId().toString(), c -> c));
assertTrue(sourceDefinitions.size() >= 59);
Expand Down

0 comments on commit c129e02

Please sign in to comment.