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
thomaslhostis committed May 31, 2021
1 parent 441b5df commit 116fe55
Show file tree
Hide file tree
Showing 10 changed files with 100 additions and 62 deletions.
12 changes: 2 additions & 10 deletions ...org/hesperides/core/infrastructure/mongo/platforms/MongoPlatformProjectionRepository.java
View file
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
30 changes: 10 additions & 20 deletions ...main/java/org/hesperides/core/infrastructure/mongo/platforms/MongoPlatformRepository.java
View file
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);
}
7 changes: 2 additions & 5 deletions ...n/java/org/hesperides/core/infrastructure/mongo/platforms/documents/PlatformDocument.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -36,7 +36,6 @@

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;


Expand Down Expand Up @@ -235,10 +234,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
2 changes: 1 addition & 1 deletion ...ation/src/main/java/org/hesperides/core/presentation/controllers/PlatformsController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -88,7 +88,7 @@ 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()")
@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) {

Expand Down
18 changes: 14 additions & 4 deletions ...tion/src/main/java/org/hesperides/core/presentation/controllers/PropertiesController.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -53,7 +53,7 @@ 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()")
@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);
Expand Down Expand Up @@ -136,9 +136,9 @@ 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()")
@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()")
@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
20 changes: 14 additions & 6 deletions documentation/lightweight-architecture-decision-records/search_properties.md
View file
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,27 @@ 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`.

### 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
15 changes: 8 additions & 7 deletions tests/bdd/src/main/java/org/hesperides/test/bdd/platforms/PlatformClient.java
View file
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
7 changes: 4 additions & 3 deletions tests/bdd/src/main/java/org/hesperides/test/bdd/platforms/scenarios/SearchProperties.java
View file
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
47 changes: 43 additions & 4 deletions tests/bdd/src/test/resources/platforms/properties/search-properties.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -7,18 +7,18 @@ Feature: Search properties
When I try to search for properties without a name or a value
Then the request is rejected with a bad request error

Scenario: search a property by name and/or value
Scenario: search a property by exact name and/or value
Given an existing module
And an existing platform with this module
And the platform has these valued properties
| name | value |
| simple-property | simple-value |
| another-property | simple-value |
| another-simple-property | other-value |
When I search for properties by name "simple-property"
When I search for properties by name "another-property"
Then the list of properties found is
| propertyName | propertyValue |
| simple-property | simple-value |
| propertyName | propertyValue |
| another-property | simple-value |
When I search for properties by value "simple-value"
Then the list of properties found is
| propertyName | propertyValue |
Expand Down Expand Up @@ -142,3 +142,42 @@ Feature: Search properties
| propertyName | propertyValue | applicationName |
| property | value | A |
| property | value | B |

#issue-872
Scenario: trying to search properties with less than 3 characters should fail
When I try to search for properties by name "ab"
Then the request is rejected with a bad request error
When I try to search for properties by value "ab"
Then the request is rejected with a bad request error
When I try to search for properties by name "ab" and value "de"
Then the request is rejected with a bad request error
When I try to search for properties by name "abc" and value "de"
Then the request is successful
When I try to search for properties by name "ab" and value "cde"
Then the request is successful

#issue-872
Scenario: search a property by partial name and/or value
Given an existing module
And an existing platform with this module
And the platform has these valued properties
| name | value |
| simple-property | simple-value |
| another-property | simple-value |
| another-simple-property | other-value |
When I search for properties by name "simple-"
Then the list of properties found is
| propertyName | propertyValue |
| simple-property | simple-value |
| another-simple-property | other-value |
When I search for properties by value "simple-"
Then the list of properties found is
| propertyName | propertyValue |
| simple-property | simple-value |
| another-property | simple-value |
When I search for properties by name "simple-" and value "simple-"
Then the list of properties found is
| propertyName | propertyValue |
| simple-property | simple-value |

#todo essayer de trouver un test de cas marginal + mettre à jour le LADR
4 changes: 2 additions & 2 deletions tests/bdd/src/test/resources/technos/search-technos.feature
View file
Original file line number Diff line number Diff line change
Expand Up @@ -23,13 +23,13 @@ Feature: Search technos
When I search for some of these technos, limiting the number of results to 100
Then the list of techno results is limited to 12 items

# Issue 863
#issue-863
Scenario: search for an existing techno using the wrong case
Given an existing techno named "aTechno"
When I search for the techno named "ATECHNO"
Then the techno is found

# Issue 863
#issue-863
Scenario: search for an existing techno using the wrong case again
Given an existing techno named "aTechno"
When I search for the techno named "atechno"
Expand Down

0 comments on commit 116fe55

Please sign in to comment.