ci: Added pre-commit hook to check for Spotless formatting (#25228)

This commit is contained in:
Nidhi 2023-07-10 11:18:52 +05:30 committed by GitHub
parent 0cc716c2a5
commit fca545a115
No known key found for this signature in database
GPG Key ID: 4AEE18F83AFDEB23
12 changed files with 104 additions and 96 deletions

View File

@ -2,3 +2,7 @@
. "$(dirname -- "$0")/_/husky.sh" . "$(dirname -- "$0")/_/husky.sh"
npx lint-staged --cwd app/client && git-secrets --scan --untracked && git-secrets --scan -r npx lint-staged --cwd app/client && git-secrets --scan --untracked && git-secrets --scan -r
echo "Running Spotless check ..."
pushd app/server > /dev/null
(mvn spotless:check 1> /dev/null && popd > /dev/null) || (echo "Spotless check failed, please run mvn spotless:apply" && exit 1)

View File

@ -12,5 +12,4 @@ public class S3PluginConstants {
public static final String AWS_S3_SERVICE_PROVIDER = "amazon-s3"; public static final String AWS_S3_SERVICE_PROVIDER = "amazon-s3";
public static String DEFAULT_FILE_NAME = "MyFile.txt"; public static String DEFAULT_FILE_NAME = "MyFile.txt";
public static final String ACCESS_DENIED_ERROR_CODE = "AccessDenied"; public static final String ACCESS_DENIED_ERROR_CODE = "AccessDenied";
} }

View File

@ -99,9 +99,8 @@ public class AuthenticationSuccessHandlerCE implements ServerAuthenticationSucce
// verification this can be eliminated safely // verification this can be eliminated safely
if (user.getPassword() != null) { if (user.getPassword() != null) {
user.setPassword(null); user.setPassword(null);
user.setSource( user.setSource(LoginSource.fromString(
LoginSource.fromString(((OAuth2AuthenticationToken) authentication).getAuthorizedClientRegistrationId()) ((OAuth2AuthenticationToken) authentication).getAuthorizedClientRegistrationId()));
);
// Update the user in separate thread // Update the user in separate thread
userRepository userRepository
.save(user) .save(user)

View File

@ -8,10 +8,14 @@ import org.springframework.stereotype.Component;
@Component @Component
@Slf4j @Slf4j
public class CacheableFeatureFlagHelperImpl extends CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHelper { public class CacheableFeatureFlagHelperImpl extends CacheableFeatureFlagHelperCEImpl
public CacheableFeatureFlagHelperImpl(TenantService tenantService, ConfigService configService, implements CacheableFeatureFlagHelper {
CloudServicesConfig cloudServicesConfig, CommonConfig commonConfig, public CacheableFeatureFlagHelperImpl(
UserIdentifierService userIdentifierService) { TenantService tenantService,
ConfigService configService,
CloudServicesConfig cloudServicesConfig,
CommonConfig commonConfig,
UserIdentifierService userIdentifierService) {
super(tenantService, configService, cloudServicesConfig, commonConfig, userIdentifierService); super(tenantService, configService, cloudServicesConfig, commonConfig, userIdentifierService);
} }
} }

View File

@ -36,9 +36,12 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel
private final UserIdentifierService userIdentifierService; private final UserIdentifierService userIdentifierService;
public CacheableFeatureFlagHelperCEImpl(TenantService tenantService, ConfigService configService, public CacheableFeatureFlagHelperCEImpl(
CloudServicesConfig cloudServicesConfig, CommonConfig commonConfig, TenantService tenantService,
UserIdentifierService userIdentifierService) { ConfigService configService,
CloudServicesConfig cloudServicesConfig,
CommonConfig commonConfig,
UserIdentifierService userIdentifierService) {
this.tenantService = tenantService; this.tenantService = tenantService;
this.configService = configService; this.configService = configService;
this.cloudServicesConfig = cloudServicesConfig; this.cloudServicesConfig = cloudServicesConfig;
@ -58,24 +61,23 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel
} }
private Mono<Map<String, Object>> getUserDefaultTraits(User user) { private Mono<Map<String, Object>> getUserDefaultTraits(User user) {
return configService.getInstanceId() return configService.getInstanceId().map(instanceId -> {
.map(instanceId -> { Map<String, Object> userTraits = new HashMap<>();
Map<String, Object> userTraits = new HashMap<>(); String emailTrait;
String emailTrait; if (!commonConfig.isCloudHosting()) {
if (!commonConfig.isCloudHosting()) { emailTrait = userIdentifierService.hash(user.getEmail());
emailTrait = userIdentifierService.hash(user.getEmail()); } else {
} else { emailTrait = user.getEmail();
emailTrait = user.getEmail(); }
} userTraits.put("email", emailTrait);
userTraits.put("email", emailTrait); userTraits.put("instanceId", instanceId);
userTraits.put("instanceId", instanceId); userTraits.put("tenantId", user.getTenantId());
userTraits.put("tenantId", user.getTenantId()); userTraits.put("isTelemetryOn", !commonConfig.isTelemetryDisabled());
userTraits.put("isTelemetryOn", !commonConfig.isTelemetryDisabled()); userTraits.put("createdAt", user.getCreatedAt());
userTraits.put("createdAt", user.getCreatedAt()); userTraits.put("defaultTraitsUpdatedAt", Instant.now().getEpochSecond());
userTraits.put("defaultTraitsUpdatedAt", Instant.now().getEpochSecond()); userTraits.put("type", "user");
userTraits.put("type", "user"); return userTraits;
return userTraits; });
});
} }
@CacheEvict(cacheName = "featureFlag", key = "{#userIdentifier}") @CacheEvict(cacheName = "featureFlag", key = "{#userIdentifier}")
@ -90,13 +92,8 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel
Mono<String> defaultTenantIdMono = tenantService.getDefaultTenantId(); Mono<String> defaultTenantIdMono = tenantService.getDefaultTenantId();
return Mono.zip(instanceIdMono, defaultTenantIdMono, getUserDefaultTraits(user)) return Mono.zip(instanceIdMono, defaultTenantIdMono, getUserDefaultTraits(user))
.flatMap(objects -> { .flatMap(objects -> {
return this.getRemoteFeatureFlagsByIdentity( return this.getRemoteFeatureFlagsByIdentity(new FeatureFlagIdentityTraits(
new FeatureFlagIdentityTraits( objects.getT1(), objects.getT2(), Set.of(userIdentifier), objects.getT3()));
objects.getT1(),
objects.getT2(),
Set.of(userIdentifier),
objects.getT3())
);
}) })
.map(newValue -> newValue.get(userIdentifier)); .map(newValue -> newValue.get(userIdentifier));
} }
@ -108,16 +105,16 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel
* @param featureFlagIdentityTraits * @param featureFlagIdentityTraits
* @return * @return
*/ */
private Mono<Map<String, Map<String, Boolean>>> getRemoteFeatureFlagsByIdentity(FeatureFlagIdentityTraits featureFlagIdentityTraits) { private Mono<Map<String, Map<String, Boolean>>> getRemoteFeatureFlagsByIdentity(
FeatureFlagIdentityTraits featureFlagIdentityTraits) {
return WebClientUtils.create(cloudServicesConfig.getBaseUrl()) return WebClientUtils.create(cloudServicesConfig.getBaseUrl())
.post() .post()
.uri("/api/v1/feature-flags") .uri("/api/v1/feature-flags")
.body(BodyInserters.fromValue(featureFlagIdentityTraits)) .body(BodyInserters.fromValue(featureFlagIdentityTraits))
.exchangeToMono(clientResponse -> { .exchangeToMono(clientResponse -> {
if (clientResponse.statusCode().is2xxSuccessful()) { if (clientResponse.statusCode().is2xxSuccessful()) {
return clientResponse.bodyToMono(new ParameterizedTypeReference<ResponseDTO<Map<String, return clientResponse.bodyToMono(
Map<String, Boolean>>>>() { new ParameterizedTypeReference<ResponseDTO<Map<String, Map<String, Boolean>>>>() {});
});
} else { } else {
return clientResponse.createError(); return clientResponse.createError();
} }
@ -126,8 +123,7 @@ public class CacheableFeatureFlagHelperCEImpl implements CacheableFeatureFlagHel
.onErrorMap( .onErrorMap(
// Only map errors if we haven't already wrapped them into an AppsmithException // Only map errors if we haven't already wrapped them into an AppsmithException
e -> !(e instanceof AppsmithException), e -> !(e instanceof AppsmithException),
e -> new AppsmithException(AppsmithError.CLOUD_SERVICES_ERROR, e.getMessage()) e -> new AppsmithException(AppsmithError.CLOUD_SERVICES_ERROR, e.getMessage()))
)
.onErrorResume(error -> { .onErrorResume(error -> {
// We're gobbling up errors here so that all feature flags are turned off by default // We're gobbling up errors here so that all feature flags are turned off by default
// This will be problematic if we do not maintain code to reflect validity of flags // This will be problematic if we do not maintain code to reflect validity of flags

View File

@ -4,7 +4,6 @@ import com.appsmith.server.domains.User;
import com.appsmith.server.featureflags.FeatureFlagEnum; import com.appsmith.server.featureflags.FeatureFlagEnum;
import reactor.core.publisher.Mono; import reactor.core.publisher.Mono;
import java.util.List;
import java.util.Map; import java.util.Map;
public interface FeatureFlagServiceCE { public interface FeatureFlagServiceCE {
@ -36,5 +35,4 @@ public interface FeatureFlagServiceCE {
* @return Mono<Map < String, Boolean>> * @return Mono<Map < String, Boolean>>
*/ */
Mono<Map<String, Boolean>> getAllFeatureFlagsForUser(); Mono<Map<String, Boolean>> getAllFeatureFlagsForUser();
} }

View File

@ -21,7 +21,6 @@ import java.time.Instant;
import java.time.temporal.ChronoUnit; import java.time.temporal.ChronoUnit;
import java.util.Map; import java.util.Map;
@Slf4j @Slf4j
public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE { public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
@ -42,13 +41,14 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
private final CacheableFeatureFlagHelper cacheableFeatureFlagHelper; private final CacheableFeatureFlagHelper cacheableFeatureFlagHelper;
@Autowired @Autowired
public FeatureFlagServiceCEImpl(SessionUserService sessionUserService, public FeatureFlagServiceCEImpl(
FF4j ff4j, SessionUserService sessionUserService,
TenantService tenantService, FF4j ff4j,
ConfigService configService, TenantService tenantService,
CloudServicesConfig cloudServicesConfig, ConfigService configService,
UserIdentifierService userIdentifierService, CloudServicesConfig cloudServicesConfig,
CacheableFeatureFlagHelper cacheableFeatureFlagHelper) { UserIdentifierService userIdentifierService,
CacheableFeatureFlagHelper cacheableFeatureFlagHelper) {
this.sessionUserService = sessionUserService; this.sessionUserService = sessionUserService;
this.ff4j = ff4j; this.ff4j = ff4j;
this.tenantService = tenantService; this.tenantService = tenantService;
@ -58,7 +58,6 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
this.cacheableFeatureFlagHelper = cacheableFeatureFlagHelper; this.cacheableFeatureFlagHelper = cacheableFeatureFlagHelper;
} }
private Mono<Boolean> checkAll(String featureName, User user) { private Mono<Boolean> checkAll(String featureName, User user) {
Boolean check = check(featureName, user); Boolean check = check(featureName, user);
@ -81,8 +80,7 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
@Override @Override
public Mono<Boolean> check(FeatureFlagEnum featureEnum) { public Mono<Boolean> check(FeatureFlagEnum featureEnum) {
return sessionUserService.getCurrentUser() return sessionUserService.getCurrentUser().flatMap(user -> check(featureEnum, user));
.flatMap(user -> check(featureEnum, user));
} }
@Override @Override
@ -93,15 +91,13 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
@Override @Override
public Mono<Map<String, Boolean>> getAllFeatureFlagsForUser() { public Mono<Map<String, Boolean>> getAllFeatureFlagsForUser() {
Mono<User> currentUser = sessionUserService.getCurrentUser().cache(); Mono<User> currentUser = sessionUserService.getCurrentUser().cache();
Flux<Tuple2<String, User>> featureUserTuple = Flux.fromIterable(ff4j.getFeatures().keySet()) Flux<Tuple2<String, User>> featureUserTuple = Flux.fromIterable(
ff4j.getFeatures().keySet())
.flatMap(featureName -> Mono.just(featureName).zipWith(currentUser)); .flatMap(featureName -> Mono.just(featureName).zipWith(currentUser));
Mono<Map<String, Boolean>> localFlagsForUser = featureUserTuple Mono<Map<String, Boolean>> localFlagsForUser = featureUserTuple
.filter(objects -> !objects.getT2().isAnonymous()) .filter(objects -> !objects.getT2().isAnonymous())
.collectMap( .collectMap(Tuple2::getT1, tuple -> check(tuple.getT1(), tuple.getT2()));
Tuple2::getT1,
tuple -> check(tuple.getT1(), tuple.getT2())
);
return Mono.zip(localFlagsForUser, this.getAllRemoteFeatureFlagsForUser()) return Mono.zip(localFlagsForUser, this.getAllRemoteFeatureFlagsForUser())
.map(tuple -> { .map(tuple -> {
@ -117,22 +113,23 @@ public class FeatureFlagServiceCEImpl implements FeatureFlagServiceCE {
*/ */
private Mono<Map<String, Boolean>> getAllRemoteFeatureFlagsForUser() { private Mono<Map<String, Boolean>> getAllRemoteFeatureFlagsForUser() {
Mono<User> userMono = sessionUserService.getCurrentUser().cache(); Mono<User> userMono = sessionUserService.getCurrentUser().cache();
return userMono return userMono.flatMap(user -> {
.flatMap(user -> { String userIdentifier = userIdentifierService.getUserIdentifier(user);
String userIdentifier = userIdentifierService.getUserIdentifier(user); // Checks for flags present in cache and if the cache is not expired
// Checks for flags present in cache and if the cache is not expired return cacheableFeatureFlagHelper
return cacheableFeatureFlagHelper .fetchUserCachedFlags(userIdentifier, user)
.fetchUserCachedFlags(userIdentifier, user) .flatMap(cachedFlags -> {
.flatMap(cachedFlags -> { if (cachedFlags.getRefreshedAt().until(Instant.now(), ChronoUnit.MINUTES)
if (cachedFlags.getRefreshedAt().until(Instant.now(), ChronoUnit.MINUTES) < this.featureFlagCacheTimeMin) { < this.featureFlagCacheTimeMin) {
return Mono.just(cachedFlags.getFlags()); return Mono.just(cachedFlags.getFlags());
} else { } else {
// empty the cache for the userIdentifier as expired // empty the cache for the userIdentifier as expired
return cacheableFeatureFlagHelper.evictUserCachedFlags(userIdentifier) return cacheableFeatureFlagHelper
.then(cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier, user)) .evictUserCachedFlags(userIdentifier)
.flatMap(cachedFlagsUpdated -> Mono.just(cachedFlagsUpdated.getFlags())); .then(cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier, user))
} .flatMap(cachedFlagsUpdated -> Mono.just(cachedFlagsUpdated.getFlags()));
}); }
}); });
});
} }
} }

View File

@ -516,13 +516,16 @@ public class ThemeServiceCEImpl extends BaseService<ThemeRepositoryCE, Theme, St
destinationApp.setPublishedModeThemeId(publishedModeThemeId); destinationApp.setPublishedModeThemeId(publishedModeThemeId);
// this will update the theme id in DB // this will update the theme id in DB
// also returning the updated application object so that theme id are available to the next pipeline // also returning the updated application object so that theme id are available to the next pipeline
return applicationService.setAppTheme( return applicationService
destinationApp.getId(), editModeThemeId, publishedModeThemeId, applicationPermission.getEditPermission()) .setAppTheme(
.thenReturn(destinationApp); destinationApp.getId(),
editModeThemeId,
publishedModeThemeId,
applicationPermission.getEditPermission())
.thenReturn(destinationApp);
}) })
.switchIfEmpty( .switchIfEmpty(
Mono.error(new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, "Failed to import theme")) Mono.error(new AppsmithException(AppsmithError.GENERIC_BAD_REQUEST, "Failed to import theme")));
);
} }
private Mono<Theme> updateExistingAppThemeFromJSON( private Mono<Theme> updateExistingAppThemeFromJSON(
@ -531,7 +534,8 @@ public class ThemeServiceCEImpl extends BaseService<ThemeRepositoryCE, Theme, St
return getOrSaveTheme(themeFromJson, destinationApp); return getOrSaveTheme(themeFromJson, destinationApp);
} }
return repository.findById(existingThemeId) return repository
.findById(existingThemeId)
.defaultIfEmpty(new Theme()) // fallback when application theme is deleted .defaultIfEmpty(new Theme()) // fallback when application theme is deleted
.flatMap(existingTheme -> { .flatMap(existingTheme -> {
if (!StringUtils.hasLength(existingTheme.getId()) || existingTheme.isSystemTheme()) { if (!StringUtils.hasLength(existingTheme.getId()) || existingTheme.isSystemTheme()) {

View File

@ -1419,7 +1419,14 @@ public class ImportExportApplicationServiceCEImpl implements ImportExportApplica
importedApplication.setPublishedPages(applicationPageMap.get(VIEW)); importedApplication.setPublishedPages(applicationPageMap.get(VIEW));
return applicationPageMap; return applicationPageMap;
}) })
.flatMap(unused -> newActionService.importActions(importedNewActionList, importedApplication, branchName, pageNameMap, pluginMap, datasourceMap, permissionProvider)) .flatMap(unused -> newActionService.importActions(
importedNewActionList,
importedApplication,
branchName,
pageNameMap,
pluginMap,
datasourceMap,
permissionProvider))
.flatMap(importActionResultDTO -> { .flatMap(importActionResultDTO -> {
log.info( log.info(
"Actions imported. applicationId {}, result: {}", "Actions imported. applicationId {}, result: {}",

View File

@ -23,7 +23,6 @@ import static org.junit.jupiter.api.Assertions.assertFalse;
import static org.junit.jupiter.api.Assertions.assertNotNull; import static org.junit.jupiter.api.Assertions.assertNotNull;
import static org.junit.jupiter.api.Assertions.assertTrue; import static org.junit.jupiter.api.Assertions.assertTrue;
@ExtendWith(SpringExtension.class) @ExtendWith(SpringExtension.class)
@SpringBootTest @SpringBootTest
@Slf4j @Slf4j
@ -92,7 +91,7 @@ public class FeatureFlagServiceTest {
} }
@Test @Test
public void getFeatureFlags_withUserIdentifier_redisKeyExists(){ public void getFeatureFlags_withUserIdentifier_redisKeyExists() {
String userIdentifier = "testIdentifier"; String userIdentifier = "testIdentifier";
User dummyUser = new User(); User dummyUser = new User();
Mono<CachedFlags> cachedFlagsMono = cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier, dummyUser); Mono<CachedFlags> cachedFlagsMono = cacheableFeatureFlagHelper.fetchUserCachedFlags(userIdentifier, dummyUser);
@ -127,5 +126,4 @@ public class FeatureFlagServiceTest {
return ff4j; return ff4j;
} }
} }
} }

View File

@ -852,7 +852,8 @@ public class ThemeServiceTest {
@WithUserDetails("api_user") @WithUserDetails("api_user")
@Test @Test
public void importThemesToApplication_ApplicationThemeNotFound_DefaultThemeImported() { public void importThemesToApplication_ApplicationThemeNotFound_DefaultThemeImported() {
Theme defaultTheme = themeRepository.getSystemThemeByName(Theme.DEFAULT_THEME_NAME).block(); Theme defaultTheme =
themeRepository.getSystemThemeByName(Theme.DEFAULT_THEME_NAME).block();
// create the theme information present in the application JSON // create the theme information present in the application JSON
Theme themeInJson = new Theme(); Theme themeInJson = new Theme();
@ -874,18 +875,18 @@ public class ThemeServiceTest {
.flatMap(applicationRepository::save) .flatMap(applicationRepository::save)
.flatMap(savedApplication -> { .flatMap(savedApplication -> {
assert savedApplication.getId() != null; assert savedApplication.getId() != null;
return themeService.importThemesToApplication(savedApplication, applicationJson) return themeService
.importThemesToApplication(savedApplication, applicationJson)
.thenReturn(savedApplication.getId()); .thenReturn(savedApplication.getId());
}) })
.flatMap(applicationId -> .flatMap(applicationId -> applicationRepository.findById(applicationId, MANAGE_APPLICATIONS));
applicationRepository.findById(applicationId, MANAGE_APPLICATIONS)
);
StepVerifier.create(applicationMono) StepVerifier.create(applicationMono)
.assertNext(app -> { .assertNext(app -> {
// both edit mode and published mode should have default theme set // both edit mode and published mode should have default theme set
assertThat(app.getEditModeThemeId()).isEqualTo(app.getPublishedModeThemeId()); assertThat(app.getEditModeThemeId()).isEqualTo(app.getPublishedModeThemeId());
assertThat(app.getEditModeThemeId()).isEqualTo(defaultTheme.getId()); assertThat(app.getEditModeThemeId()).isEqualTo(defaultTheme.getId());
}).verifyComplete(); })
.verifyComplete();
} }
} }

View File

@ -174,6 +174,7 @@
<goals> <goals>
<goal>apply</goal> <goal>apply</goal>
</goals> </goals>
<!-- This gets triggered automatically when we run the build script -->
<phase>compile</phase> <phase>compile</phase>
</execution> </execution>
</executions> </executions>