Skip to content

Commit

Permalink
Fix push bug
Browse files Browse the repository at this point in the history
  • Loading branch information
samdgupi committed May 2, 2024
1 parent 964d7a4 commit 64c3619
Show file tree
Hide file tree
Showing 14 changed files with 54 additions and 32 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -303,7 +303,7 @@ public void getAllApps(HttpRequest request, HttpResponder responder,
* @param pageToken A token for paginating through results.
* @param pageSize The size of each page for pagination.
* @param sortOrder The sort order for the metadata.
* @param sortOn The field on which to sort the metadata.
* @param sortOn The field on which to sort the metadata.
* @param filter A filter string for filtering the metadata.
* filter query format - "name=<name-filter> AND syncStatus=<SYNCED/UNSYNCED>".
* @throws Exception If an error occurs during the metadata retrieval process.
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -41,6 +41,7 @@
import io.cdap.cdap.proto.id.NamespaceId;
import io.cdap.cdap.proto.operation.OperationMeta;
import io.cdap.cdap.proto.operation.OperationRun;
import io.cdap.cdap.proto.sourcecontrol.Provider;
import io.cdap.cdap.proto.sourcecontrol.PullMultipleAppsRequest;
import io.cdap.cdap.proto.sourcecontrol.PushAppRequest;
import io.cdap.cdap.proto.sourcecontrol.PushMultipleAppsRequest;
Expand Down Expand Up @@ -350,6 +351,14 @@ private RepositoryConfigRequest validateAndGetRepoConfig(FullHttpRequest request
if (repoRequest == null || repoRequest.getConfig() == null) {
throw new RepositoryConfigValidationException("Repository configuration must be specified.");
}
// Only allow gitlab and bitbucket if feature flag is enabled
if (!Feature.SOURCE_CONTROL_MANAGEMENT_GITLAB_BITBUCKET.isEnabled(featureFlagsProvider)) {
if (repoRequest.getConfig().getProvider() != Provider.GITHUB) {
throw new BadRequestException(
String.format("Provider %s is not supported", repoRequest.getConfig().getProvider())
);
}
}
repoRequest.getConfig().validate();
return repoRequest;
} catch (JsonSyntaxException e) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -376,7 +376,7 @@ private PullAppResponse<?> pullAndValidateApplication(ApplicationReference appRe
PullAppResponse<?> pullResponse = sourceControlOperationRunner.pull(
new PullAppOperationRequest(appRef, repoConfig));

if (latestMeta != null
if (latestMeta != null && latestMeta.getFileHash() != null
&& latestMeta.getFileHash().equals(pullResponse.getApplicationFileHash())) {
throw new NoChangesToPullException(
String.format("Pipeline deployment was not successful because there is "
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -100,8 +100,9 @@ public ListenableFuture<Set<OperationResource>> run(LongRunningOperationContext

try {
ApplicationDetail currentDetail = applicationManager.get(appRef);
if (currentDetail.getSourceControlMeta() != null && currentDetail.getSourceControlMeta()
.getFileHash().equals(response.getApplicationFileHash())) {
if (currentDetail.getSourceControlMeta() != null
&& response.getApplicationFileHash()
.equals(currentDetail.getSourceControlMeta().getFileHash())) {
LOG.trace("Application {} already have same commit, skipping",
response.getApplicationName());
return;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -259,7 +259,7 @@ private CloseableIterator<StructuredRow> getScanIterator(
Range range,
SortOrder sortOrder,
SortBy sortBy) throws IOException {
if (sortBy == SortBy.LAST_SYNCED_DATE) {
if (sortBy == SortBy.LAST_SYNCED_AT) {
return table.scan(range, Integer.MAX_VALUE,
StoreDefinition.NamespaceSourceControlMetadataStore.LAST_MODIFIED_FIELD, sortOrder);
}
Expand All @@ -275,7 +275,7 @@ private List<Field<?>> getRangeFields(
request.getNamespace()));
fields.add(
Fields.stringField(StoreDefinition.NamespaceSourceControlMetadataStore.TYPE_FIELD, type));
if (request.getSortOn() == SortBy.LAST_SYNCED_DATE) {
if (request.getSortOn() == SortBy.LAST_SYNCED_AT) {
SourceControlMeta meta = get(
new ApplicationReference(request.getNamespace(), request.getScanAfter()));
fields.add(Fields.longField(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -200,7 +200,7 @@ private CloseableIterator<StructuredRow> getScanIterator(
Range range,
SortOrder sortOrder,
SortBy sortBy) throws IOException {
if (sortBy == SortBy.LAST_SYNCED_DATE) {
if (sortBy == SortBy.LAST_SYNCED_AT) {
return table.scan(range, Integer.MAX_VALUE,
StoreDefinition.RepositorySourceControlMetadataStore.LAST_MODIFIED_FIELD, sortOrder);
}
Expand Down Expand Up @@ -245,7 +245,7 @@ private List<Field<?>> getRangeFields(ScanSourceControlMetadataRequest request,
request.getNamespace()));
fields.add(
Fields.stringField(StoreDefinition.RepositorySourceControlMetadataStore.TYPE_FIELD, type));
if (request.getSortOn() == SortBy.LAST_SYNCED_DATE) {
if (request.getSortOn() == SortBy.LAST_SYNCED_AT) {
ImmutablePair<Long, Boolean> lastModifiedAndStatusPair = get(
new ApplicationReference(request.getNamespace(), request.getScanAfter()));
fields.add(Fields.longField(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -599,7 +599,7 @@ public void testScanRepoMetadata() throws Exception {
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(Namespace.DEFAULT.getId())
.setSortOrder(
SortOrder.DESC).setSortOn(SortBy.LAST_SYNCED_DATE).build();
SortOrder.DESC).setSortOn(SortBy.LAST_SYNCED_AT).build();
sourceControlService.scanRepoMetadata(request, batchSize, gotRecords::add);
expectedRecords = insertedRecords.stream()
.sorted(Comparator.nullsFirst(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -140,7 +140,7 @@ public void testScanWithDifferentLimit() {
// when last modified is in desc order
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedDesc;
Assert.assertArrayEquals(expectedRecords.toArray(), gotRecords.toArray());
Expand All @@ -149,15 +149,15 @@ public void testScanWithDifferentLimit() {
// when last modified is in asc order
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setSortOrder(SortOrder.ASC).setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOrder(SortOrder.ASC).setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedAsc;
Assert.assertArrayEquals(expectedRecords.toArray(), gotRecords.toArray());

// verify limit for testNamespaceRecordsSortedOnNameDesc
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE).setLimit(3)
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.PIPELINE_NAME).build();
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.NAME).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnNameDesc.stream().limit(3)
.collect(Collectors.toList());
Expand Down Expand Up @@ -210,7 +210,7 @@ public void testScanWithPagination() {
// verify SYNCED and name filter with pageToken and limit for testNamespaceRecordsSortedOnLastModifiedDesc
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE).setLimit(1)
.setScanAfter("dependent100").setSortOrder(SortOrder.DESC).setSortOn(SortBy.PIPELINE_NAME)
.setScanAfter("dependent100").setSortOrder(SortOrder.DESC).setSortOn(SortBy.NAME)
.setFilter(new SourceControlMetadataFilter("t", true)).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedDesc.stream()
Expand All @@ -221,7 +221,7 @@ public void testScanWithPagination() {
// verify UNSYNCED and name filter with pageToken and limit for testNamespaceRecordsSortedOnLastModifiedDesc
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE).setLimit(1)
.setScanAfter("dependent100").setSortOn(SortBy.LAST_SYNCED_DATE)
.setScanAfter("dependent100").setSortOn(SortBy.LAST_SYNCED_AT)
.setSortOrder(SortOrder.DESC)
.setFilter(new SourceControlMetadataFilter("t", false)).build();
store.scan(request, TYPE, gotRecords::add);
Expand Down Expand Up @@ -254,7 +254,7 @@ record -> record.getName().equals("scm-test") || record.getName().equals("zapapp
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setScanAfter("zapapp").setLimit(5).setSortOrder(SortOrder.DESC)
.setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedDesc.stream().filter(
record -> record.getName().equals("newapp") || record.getName().equals("scm-test")
Expand All @@ -265,7 +265,7 @@ record -> record.getName().equals("newapp") || record.getName().equals("scm-test
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setScanAfter("test").setLimit(3).setSortOrder(SortOrder.DESC)
.setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedDesc.stream().filter(
record -> record.getName().equals("dependent100") || record.getName()
Expand All @@ -276,7 +276,7 @@ record -> record.getName().equals("dependent100") || record.getName()
// verify page token for testNamespaceRecordsSortedOnNameDesc
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.PIPELINE_NAME).setScanAfter("test")
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.NAME).setScanAfter("test")
.setLimit(3).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnNameDesc.stream().filter(
Expand All @@ -288,7 +288,7 @@ record -> record.getName().equals("scm-test") || record.getName().equals("depend
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setScanAfter("scm-test").setLimit(4).setSortOrder(SortOrder.ASC)
.setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedAsc.stream().filter(
record -> record.getName().equals("zapapp") || record.getName().equals("newapp")
Expand All @@ -300,7 +300,7 @@ record -> record.getName().equals("zapapp") || record.getName().equals("newapp")
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setScanAfter("datafusionquickstart").setLimit(2).setSortOrder(SortOrder.ASC)
.setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedAsc.stream().filter(
record -> record.getName().equals("newapp") || record.getName().equals("scm-test"))
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -129,7 +129,7 @@ public void testScanWithDifferentLimit() {
// when last modified is in desc order
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedDesc;
Assert.assertArrayEquals(expectedRecords.toArray(), gotRecords.toArray());
Expand All @@ -138,15 +138,15 @@ public void testScanWithDifferentLimit() {
// when last modified is in asc order
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setSortOrder(SortOrder.ASC).setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOrder(SortOrder.ASC).setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedAsc;
Assert.assertArrayEquals(expectedRecords.toArray(), gotRecords.toArray());

// verify limit for testNamespaceRecordsSortedOnNameDesc
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE).setLimit(3)
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.PIPELINE_NAME).build();
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.NAME).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnNameDesc.stream().limit(3)
.collect(Collectors.toList());
Expand Down Expand Up @@ -199,7 +199,7 @@ public void testScanWithPagination() {
// verify SYNCED and name filter with pageToken and limit for testNamespaceRecordsSortedOnLastModifiedDesc
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE).setLimit(1)
.setScanAfter("dependent100").setSortOrder(SortOrder.DESC).setSortOn(SortBy.PIPELINE_NAME)
.setScanAfter("dependent100").setSortOrder(SortOrder.DESC).setSortOn(SortBy.NAME)
.setFilter(new SourceControlMetadataFilter("t", true)).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedDesc.stream()
Expand All @@ -210,7 +210,7 @@ public void testScanWithPagination() {
// verify UNSYNCED and name filter with pageToken and limit for testNamespaceRecordsSortedOnLastModifiedDesc
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE).setLimit(1)
.setScanAfter("dependent100").setSortOn(SortBy.LAST_SYNCED_DATE)
.setScanAfter("dependent100").setSortOn(SortBy.LAST_SYNCED_AT)
.setSortOrder(SortOrder.DESC)
.setFilter(new SourceControlMetadataFilter("t", false)).build();
store.scan(request, TYPE, gotRecords::add);
Expand Down Expand Up @@ -243,7 +243,7 @@ record -> record.getName().equals("scm-test") || record.getName().equals("zapapp
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setScanAfter("zapapp").setLimit(5).setSortOrder(SortOrder.DESC)
.setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedDesc.stream().filter(
record -> record.getName().equals("newapp") || record.getName().equals("scm-test")
Expand All @@ -254,7 +254,7 @@ record -> record.getName().equals("newapp") || record.getName().equals("scm-test
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setScanAfter("test").setLimit(3).setSortOrder(SortOrder.DESC)
.setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedDesc.stream().filter(
record -> record.getName().equals("dependent100") || record.getName()
Expand All @@ -265,7 +265,7 @@ record -> record.getName().equals("dependent100") || record.getName()
// verify page token for testNamespaceRecordsSortedOnNameDesc
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.PIPELINE_NAME).setScanAfter("test")
.setSortOrder(SortOrder.DESC).setSortOn(SortBy.NAME).setScanAfter("test")
.setLimit(3).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnNameDesc.stream().filter(
Expand All @@ -277,7 +277,7 @@ record -> record.getName().equals("scm-test") || record.getName().equals("depend
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setScanAfter("scm-test").setLimit(4).setSortOrder(SortOrder.ASC)
.setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedAsc.stream().filter(
record -> record.getName().equals("zapapp") || record.getName().equals("newapp")
Expand All @@ -289,7 +289,7 @@ record -> record.getName().equals("zapapp") || record.getName().equals("newapp")
gotRecords.clear();
request = ScanSourceControlMetadataRequest.builder().setNamespace(NAMESPACE)
.setScanAfter("datafusionquickstart").setLimit(2).setSortOrder(SortOrder.ASC)
.setSortOn(SortBy.LAST_SYNCED_DATE).build();
.setSortOn(SortBy.LAST_SYNCED_AT).build();
store.scan(request, TYPE, gotRecords::add);
expectedRecords = testNamespaceRecordsSortedOnLastModifiedAsc.stream().filter(
record -> record.getName().equals("newapp") || record.getName().equals("scm-test"))
Expand Down
9 changes: 8 additions & 1 deletion cdap-common/src/main/resources/cdap-default.xml
Original file line number Diff line number Diff line change
Expand Up @@ -6009,7 +6009,7 @@

<property>
<name>feature.wrangler.kryo.serialization.enabled</name>
<value>false</value>
<value>true</value>
<description>
If true, wrangler will serialize rows using kryo.
</description>
Expand Down Expand Up @@ -6089,6 +6089,13 @@
pipelines will be enabled.
</description>
</property>
<property>
<name>feature.source.control.management.gitlab.bitbucket.enabled</name>
<value>true</value>
<description>
Enable gitlab and bitbucket integration
</description>
</property>
<property>
<name>source.control.git.command.timeout.seconds</name>
<value>30</value>
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -43,7 +43,8 @@ public enum Feature {
WRANGLER_EXECUTION_SQL("6.10.0"),
WRANGLER_SCHEMA_MANAGEMENT("6.10.0"),
NAMESPACED_SERVICE_ACCOUNTS("6.10.0"),
WRANGLER_KRYO_SERIALIZATION("6.10.1");
WRANGLER_KRYO_SERIALIZATION("6.10.1"),
SOURCE_CONTROL_MANAGEMENT_GITLAB_BITBUCKET("6.10.1");

private final PlatformInfo.Version versionIntroduced;
private final boolean defaultAfterIntroduction;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,6 +16,7 @@

package io.cdap.cdap.proto;

import com.google.gson.annotations.SerializedName;
import java.util.Objects;
import javax.annotation.Nullable;

Expand All @@ -32,6 +33,7 @@ public class SourceControlMetadataRecord {
private final String specificationHash;
@Nullable
private final String commitId;
@SerializedName("lastSyncedAt")
@Nullable
private final Long lastModified;
private final Boolean isSynced;
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,5 +20,6 @@
* Sort order options for namespace and repository pipelines.
*/
public enum SortBy {
PIPELINE_NAME, LAST_SYNCED_DATE
NAME,
LAST_SYNCED_AT
}
Original file line number Diff line number Diff line change
Expand Up @@ -63,6 +63,7 @@ public SourceControlMeta(String fileHash, @Nullable String commitId,
this.syncStatus = syncStatus;
}

@Nullable
public String getFileHash() {
return fileHash;
}
Expand Down

0 comments on commit 64c3619

Please sign in to comment.