Skip to content

Commit

Permalink
Fix #872 : Recherche de propriétés par nom et/ou valeur partielle
Browse files Browse the repository at this point in the history
  • Loading branch information
thomaslhostis committed May 9, 2021
1 parent 876141e commit acab59c
Show file tree
Hide file tree
Showing 10 changed files with 135 additions and 66 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -48,7 +48,6 @@
import java.util.stream.Stream;

import static java.util.stream.Collectors.groupingBy;
import static org.apache.commons.lang3.StringUtils.isEmpty;
import static org.hesperides.commons.SpringProfiles.FAKE_MONGO;
import static org.hesperides.commons.SpringProfiles.MONGO;
import static org.hesperides.core.infrastructure.mongo.Collections.PLATFORM;
Expand Down Expand Up @@ -452,15 +451,8 @@ public List<PlatformPropertiesView> onFindAllApplicationsPropertiesQuery(FindAll
@Override
@Timed
public List<PropertySearchResultView> onSearchPropertiesQuery(SearchPropertiesQuery query) {
List<PlatformDocument> platformDocuments;

if (isEmpty(query.getPropertyValue())) {
platformDocuments = platformRepository.findPlatformsByPropertiesName(query.getPropertyName());
} else if (isEmpty(query.getPropertyName())) {
platformDocuments = platformRepository.findPlatformsByPropertiesValue(query.getPropertyValue());
} else {
platformDocuments = platformRepository.findPlatformsByPropertiesNameAndValue(query.getPropertyName(), query.getPropertyValue());
}
List<PlatformDocument> platformDocuments = platformRepository
.findPlatformsByPropertiesNameAndValue(query.getPropertyName(), query.getPropertyValue());

return platformDocuments.stream()
.map(platformDocument -> platformDocument.filterToPropertySearchResultViews(
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -62,32 +62,22 @@ public interface MongoPlatformRepository extends MongoRepository<PlatformDocumen
@Query(value = "{ }", fields = "{ 'key': 1, 'isProductionPlatform': 1, 'deployedModules.id': 1, 'deployedModules.propertiesPath': 1, 'deployedModules.valuedProperties': 1 }")
List<PlatformDocument> findAllApplicationsPropertiesQuery();

// La raison pour laquelle il y a 3 requêtes pour la recherche de propriétés est que
// je n'ai pas réussi à rendre le nom et la valeur optionnels dans la clause `value`
String SEARCHED_PROPERTIES_FIELDS = "{" +
" 'key': 1," +
" 'isProductionPlatform': 1," +
" 'deployedModules.id': 1," +
" 'deployedModules.propertiesPath': 1," +
" 'deployedModules.valuedProperties': 1, " +
"}";

@Query(value = "{ 'deployedModules.valuedProperties.name': ?0 }", fields = SEARCHED_PROPERTIES_FIELDS)
List<PlatformDocument> findPlatformsByPropertiesName(String propertyName);

@Query(value = "{ 'deployedModules.valuedProperties.value': ?0 }", fields = SEARCHED_PROPERTIES_FIELDS)
List<PlatformDocument> findPlatformsByPropertiesValue(String propertyValue);

@Query(
value = "{" +
value = "{ " +
" 'deployedModules.valuedProperties': {" +
" $elemMatch: {" +
" 'name': ?0," +
" 'value': ?1" +
" 'name': { '$regex': ?0, '$options': 'i' }," +
" 'value': { '$regex': ?1, '$options': 'i' }" +
" }" +
" } " +
"}",
fields = SEARCHED_PROPERTIES_FIELDS
fields = "{" +
" 'key': 1," +
" 'isProductionPlatform': 1," +
" 'deployedModules.id': 1," +
" 'deployedModules.propertiesPath': 1," +
" 'deployedModules.valuedProperties': 1, " +
"}"
)
List<PlatformDocument> findPlatformsByPropertiesNameAndValue(String propertyName, String propertyValue);
}
Original file line number Diff line number Diff line change
Expand Up @@ -28,6 +28,8 @@
import org.hesperides.core.domain.platforms.queries.views.properties.PropertySearchResultView;
import org.hesperides.core.infrastructure.MinimalPlatformRepository;
import org.springframework.data.annotation.Id;
import org.springframework.data.mongodb.core.index.CompoundIndex;
import org.springframework.data.mongodb.core.index.CompoundIndexes;
import org.springframework.data.mongodb.core.mapping.Document;
import org.springframework.util.CollectionUtils;

Expand All @@ -36,13 +38,39 @@

import static java.util.stream.Collectors.groupingBy;
import static java.util.stream.Collectors.toList;
import static org.apache.commons.lang3.StringUtils.isEmpty;
import static org.hesperides.core.infrastructure.mongo.Collections.PLATFORM;


@Data
@Document(collection = PLATFORM)
@NoArgsConstructor
// Les index sont créés ici pour 3 raisons :
// 1. Si on utilisait l'annotation @Indexed directement sur les champs `name` et `value`, cela
// créerait un index pour chaque utilisation de `ValuedPropertyDocument`, c'est-à-dire les
// propriétés globales et les propriétés valorisées. Or, actuellement, nous n'avons besoin
// d'index que pour les propriétés valorisées.
// 2. Par défaut, le nom de l'index tient compte de l'imbrication des propriétés et il semblerait
// que cela ait posé problème sur la plateforme d'intégration parce que le nom des index était
// trop long (par exemple "deployedModules.valuedProperties.name").
// 3. Cela permet de les regrouper.
@CompoundIndexes({
@CompoundIndex(
name = "valued_property_name",
def = "{ 'deployedModules.valuedProperties.name' : 1 }",
background = true
),
@CompoundIndex(
name = "valued_property_value",
def = "{ 'deployedModules.valuedProperties.value' : 1 }",
background = true
),
@CompoundIndex(
name = "valued_property_name_value",
def = "{ 'deployedModules.valuedProperties.name' : 1, " +
"'deployedModules.valuedProperties.value' : 1 }",
background = true
)
})
public class PlatformDocument {

private static int DEFAULT_NUMBER_OF_ARCHIVED_MODULE_VERSIONS = 2;
Expand Down Expand Up @@ -235,10 +263,8 @@ public List<PropertySearchResultView> filterToPropertySearchResultViews(String p
.stream()
.filter(ValuedPropertyDocument.class::isInstance)
.map(ValuedPropertyDocument.class::cast)
.filter(property ->
(isEmpty(propertyName) || property.getName().equals(propertyName)) &&
(isEmpty(propertyValue) || property.getValue().equals(propertyValue))
).map(property -> new PropertySearchResultView(
.filter(property -> property.getName().contains(propertyName) && property.getValue().contains(propertyValue))
.map(property -> new PropertySearchResultView(
property.getName(),
property.getValue(),
getKey().getApplicationName(),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -88,8 +88,8 @@ public ResponseEntity<PlatformIO> createPlatform(Authentication authentication,
@GetMapping("/{application_name}/platforms/{platform_name:.+}")
public ResponseEntity<PlatformIO> getPlatform(@PathVariable("application_name") final String applicationName,
@PathVariable("platform_name") final String platformName,
@ApiParam(value = "En milliseconds depuis l'EPOCH. Pour le générer via Javascript à partir d'une date: new Date('2019-01-01 12:00:00').getTime()")
@RequestParam(value = "timestamp", required = false) final Long timestamp,
@ApiParam(value = "En milliseconds depuis l'EPOCH. Pour le générer via Javascript à partir d'une date : new Date('2019-01-01 12:00:00').getTime()")
@RequestParam(value = "timestamp", required = false) final Long timestamp,
@RequestParam(value = "with_password_info", required = false) final Boolean withPasswordFlag) {

Platform.Key platformKey = new Platform.Key(applicationName, platformName);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -53,8 +53,8 @@ public ResponseEntity<PropertiesIO> getValuedProperties(Authentication authentic
@PathVariable("application_name") final String applicationName,
@PathVariable("platform_name") final String platformName,
@RequestParam("path") final String propertiesPath,
@ApiParam(value = "En milliseconds depuis l'EPOCH. Pour le générer via Javascript à partir d'une date: new Date('2019-01-01 12:00:00').getTime()")
@RequestParam(value = "timestamp", required = false) final Long timestamp) {
@ApiParam(value = "En milliseconds depuis l'EPOCH. Pour le générer via Javascript à partir d'une date : new Date('2019-01-01 12:00:00').getTime()")
@RequestParam(value = "timestamp", required = false) final Long timestamp) {
Platform.Key platformKey = new Platform.Key(applicationName, platformName);
User authenticatedUser = new User(authentication);
Long propertiesVersionId = propertiesUseCases.getPropertiesVersionId(platformKey, propertiesPath, timestamp);
Expand Down Expand Up @@ -136,10 +136,10 @@ public ResponseEntity<PropertiesDiffOutput> getPropertiesDiff(Authentication aut
@RequestParam("to_path") String toPropertiesPath,
@RequestParam(value = "to_instance_name", required = false, defaultValue = "") String toInstanceName,
@RequestParam(value = "compare_stored_values", required = false) boolean compareStoredValues,
@ApiParam(value = "En milliseconds depuis l'EPOCH. Correspond au champ \"left\" de la sortie JSON. Pour le générer via Javascript à partir d'une date: new Date('2019-01-01 12:00:00').getTime()")
@RequestParam(value = "timestamp", required = false) Long timestamp,
@ApiParam(value = "En milliseconds depuis l'EPOCH. Correspond au champ \"right\" de la sortie JSON. Pour le générer via Javascript à partir d'une date: new Date('2019-01-01 12:00:00').getTime()")
@RequestParam(value = "origin_timestamp", required = false) Long originTimestamp) {
@ApiParam(value = "En milliseconds depuis l'EPOCH. Correspond au champ \"left\" de la sortie JSON. Pour le générer via Javascript à partir d'une date : new Date('2019-01-01 12:00:00').getTime()")
@RequestParam(value = "timestamp", required = false) Long timestamp,
@ApiParam(value = "En milliseconds depuis l'EPOCH. Correspond au champ \"right\" de la sortie JSON. Pour le générer via Javascript à partir d'une date : new Date('2019-01-01 12:00:00').getTime()")
@RequestParam(value = "origin_timestamp", required = false) Long originTimestamp) {
Platform.Key fromPlatformKey = new Platform.Key(fromApplicationName, fromPlatformName);
Platform.Key toPlatformKey = new Platform.Key(toApplicationName, toPlatformName);

Expand Down Expand Up @@ -239,8 +239,18 @@ public ResponseEntity<List<PropertySearchResultOutput>> searchProperties(
@RequestParam(value = "property_name", required = false) String propertyName,
@RequestParam(value = "property_value", required = false) String propertyValue) {

propertyName = StringUtils.defaultString(propertyName, "");
propertyValue = StringUtils.defaultString(propertyValue, "");

if (isBlank(propertyName) && isBlank(propertyValue)) {
throw new IllegalArgumentException("You have to search for a property's name and/or value");
throw new IllegalArgumentException("Please type in a name and/or a value");
}

int searchInputMinimumCharactersCount = 3;
// La règle est d'avoir au moins un des deux champs avec au moins trois caractères
if (propertyName.length() < searchInputMinimumCharactersCount && propertyValue.length() < searchInputMinimumCharactersCount) {
throw new IllegalArgumentException("You have to search for a property's name and/or value with at least " +
searchInputMinimumCharactersCount + " characters");
}

User user = new User(authentication);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -20,8 +20,6 @@ LADR frontend: [Recherches de propriétés](https://github.com/voyages-sncf-tech
* La possibilité de filtrer les résultats par application
* Indiquer dans le retour de l'API, par propriété : son type (globale / de module / d'instance), ses annotations et s'il
s'agit d'une propriété supprimée
* Permettre d'effectuer la recherche en mode partial match ou regex, dans ce cas il faudra penser à demander à
l'utilisateur de renseigner au moins 3 caractères pour limiter l'impact sur les performances
* Lorsqu'un utilisateur n'a pas les droits de prod globaux mais seulement sur certaines applications, afficher les mots
de passe de prod en clair pour ces applications

Expand Down Expand Up @@ -125,7 +123,6 @@ Les inconvénients de cette solution :

Si cette solution est choisie, il reste plusieurs choses à faire :
* Reproduire cette requête en Spring Data Mongo => https://stackoverflow.com/a/55117298/2430043
* Mettre en place un index sur `name` et `value` => `@Indexed(background = true)`

Si par la suite il faut pouvoir filtrer par expression régulière, il faudra alors passer à la version 4.2 de MongoDB : https://docs.mongodb.com/manual/reference/operator/aggregation/regexMatch/

Expand All @@ -136,16 +133,29 @@ L'idée est de ne récupérer que les documents contenant des propriétés reche
Cette solution est envisagée pour sa facilité de mise en œuvre et donc sa maintenabilité.

Les inconvénients sont :

* Des performances inférieures à la solution précédente
* Le filtre sur les propriétés recherchées (par nom et/ou valeur) est appliqué sur la requête qui nous retourne les plateformes contenant ces propriétés, il faut ensuite filtrer en Java les propriétés pour ne retourner que celles qui sont concernées par la recherche
* Le filtre sur les propriétés recherchées (par nom et/ou valeur) est appliqué sur la requête qui nous retourne les
plateformes contenant ces propriétés, il faut ensuite filtrer en Java les propriétés pour ne retourner que celles qui
sont concernées par la recherche

En termes de performances et à titre d'exemple, une recherche de propriétés ayant le nom "platform" (environ 400 résultats sans autre filtre) prend environ une seconde à s'exécuter de bout en bout.
En termes de performances et à titre d'exemple, une recherche de propriétés ayant le nom "platform" (environ 400
résultats sans autre filtre) prend environ une seconde à s'exécuter de bout en bout.

C'est cette méthode qui, pour l'instant, a été implémentée.

#### Modification du 03/02/2021

Mise en place de la recherche par nom et/ou valeur partielle. La requête MongoDB utilise désormais la méthode `$regex`
et le filtre Java la méthode `.contains`.

+ Mise en place d'index sur le nom et la valeur des propriétés.

### Créer une vue à l'aide des évènements de l'application

L'EventSourcing permet typiquement de créer une collection ne contenant que les données dont on a besoin pour effectuer une recherche de propriétés :
L'EventSourcing permet typiquement de créer une collection ne contenant que les données dont on a besoin pour effectuer
une recherche de propriétés :

* Application
* Plateforme
* Module déployé (properties path)
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -335,14 +335,15 @@ public void getAllApplicationsPasswords() {
restTemplate.getForEntity("/applications/all_passwords", String.class);
}

public void searchProperties(String propertyName, String propertyValue) {
searchProperties(propertyName, propertyValue, null);
}

public void searchProperties(String propertyName, String propertyValue, String tryTo) {
String url = "/applications/search_properties?" +
"property_name={property_name}&" +
"property_value={property_value}";
String url = "/applications/search_properties?";

if (propertyName != null) {
url += "property_name=" + propertyName + "&";
}
if (propertyValue != null) {
url += "property_value=" + propertyValue;
}

restTemplate.getForEntity(url,
getResponseType(tryTo, PropertySearchResultOutput[].class),
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -45,11 +45,12 @@ public class SearchProperties extends HesperidesScenario implements En {

public SearchProperties() {

When("^I search for properties" +
When("^I( try to)? search for properties" +
"(?: by name \"([^\"]+)\")?" +
"(?: (?:by|and)? value \"([^\"]+)\")?$", (String propertyName,
"(?: (?:by|and)? value \"([^\"]+)\")?$", (String tryTo,
String propertyName,
String propertyValue) -> {
platformClient.searchProperties(propertyName, propertyValue);
platformClient.searchProperties(propertyName, propertyValue, tryTo);
});

When("I try to search for properties without a name or a value", () -> {
Expand Down
Loading

0 comments on commit acab59c

Please sign in to comment.