Skip to content
New issue

Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.

By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.

Already on GitHub? Sign in to your account

disable auto case indexation #39

Merged
merged 23 commits into from
Aug 21, 2024
Merged

Conversation

jamal-khey
Copy link
Contributor

@jamal-khey jamal-khey commented Aug 1, 2024

Please check if the PR fulfills these requirements

  • The commit message follows our guidelines
  • Tests for the changes have been added (for bug fixes / features)
  • Docs have been added / updated (for bug fixes / features)

What kind of change does this PR introduce?
this PR disable auto indexation of the cases, and intreduce a new param "indexed" to indicate wich cases should be indexed by /v1/supervision/case/reindex
This also PR create a supervision service to delete elk documents and reindex cases.

What is the current behavior?
when a case is created or duplication , this service index it automatically

What is the new behavior (if this is a feature change)?
cases are indexed on demand

Does this PR introduce a breaking change or deprecate an API?

  • Yes
  • No

If yes, please check if the following requirements are fulfilled

  • The Breaking Change or Deprecated label has been added
  • The migration steps are described in the following section

What changes might users need to make in their application due to this PR? (migration steps)
The endpoint for reindexing all cases has been renamed from /v1/cases/index-all to /v1/supervision/cases/reindex.
Any client applications or scripts that previously called the /v1/cases/index-all endpoint will need to be updated to use the new /v1/supervision/cases/reindex endpoint.

The database schema has also been changed to add a new column "indexed" , the values of this collumn will be set to false by default.

Other information:

To test this PR :

  • delete elastic search index "cases
  • restart the service
  • use the endpoint /v1/supervision/cases/reindex to recreate the index ( you should have no cases indexed , all of them are marked indexed=false by default)
  • choose some cases and update "indexed" value using PUT /cases/{caseUuid}/indexation
  • re-do a delete index - re-create index , now the choosen cases should appear in elastic search index

@jamal-khey jamal-khey changed the title Jamalkhey/disable auto case indexation disable auto case indexation Aug 1, 2024
@jamal-khey jamal-khey self-assigned this Aug 1, 2024
@jamal-khey jamal-khey requested a review from AAJELLAL August 1, 2024 08:01
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
@jamal-khey jamal-khey force-pushed the jamalkhey/disable-auto-case-indexation branch from decb125 to 4fb0ead Compare August 1, 2024 08:04
Signed-off-by: jamal-khey <myjamal89@gmail.com>
@jamal-khey jamal-khey force-pushed the jamalkhey/disable-auto-case-indexation branch from fde5d71 to dff6051 Compare August 1, 2024 13:27
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
@jamal-khey jamal-khey force-pushed the jamalkhey/disable-auto-case-indexation branch from 00c954d to 571a851 Compare August 5, 2024 14:29
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
@jamal-khey jamal-khey requested a review from SlimaneAmar August 7, 2024 08:27
LOGGER.debug("importCase request received with file = {}", file.getName());
UUID caseUuid = caseService.importCase(file, withExpiration);
UUID caseUuid = caseService.importCase(file, withExpiration, indexed);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename by withIndexation
But in database leave indexed

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -17,4 +18,8 @@
@Repository
public interface CaseMetadataRepository extends JpaRepository<CaseMetadataEntity, UUID> {

@Override
List<CaseMetadataEntity> findAllById(Iterable<UUID> uuids);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

return nbIndexesToDelete;
}

public long getIndexedCaseElementsCount() {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove elements -> getIndexedCasesCount

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

src/main/java/com/powsybl/caseserver/CaseController.java Outdated Show resolved Hide resolved
@@ -165,9 +166,10 @@ public ResponseEntity<UUID> importCase(@RequestParam("file") MultipartFile file,
@ApiResponse(responseCode = "500", description = "An error occurred during the case file duplication")})
public ResponseEntity<UUID> duplicateCase(
@RequestParam("duplicateFrom") UUID caseId,
@RequestParam(value = "withExpiration", required = false, defaultValue = "false") boolean withExpiration) {
@RequestParam(value = "withExpiration", required = false, defaultValue = "false") boolean withExpiration,
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove paramter
Do indexation if the origin case is indexed only

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

@@ -273,6 +284,12 @@ public void disableCaseExpiration(UUID caseUuid) {
caseMetadataEntity.setExpirationDate(null);
}

@Transactional
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Remove it

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Comment on lines 42 to 45
@RunWith(SpringRunner.class)
@AutoConfigureMockMvc
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.MOCK, properties = {"case-store-directory=/cases"})
@ContextConfigurationWithTestChannel
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Suggested change
@RunWith(SpringRunner.class)
@AutoConfigureMockMvc
@SpringBootTest(webEnvironment = SpringBootTest.WebEnvironment.MOCK, properties = {"case-store-directory=/cases"})
@ContextConfigurationWithTestChannel
@RunWith(SpringRunner.class)
@AutoConfigureMockMvc
@SpringBootTest
@ContextConfiguration(classes = {CaseApplication.class})

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done partially , the tests does not pass without properties = {"case-store-directory=/cases"}

private FileSystem fileSystem;

@Test
public void testGetElementInfosCount() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Ranma element by case anywhere

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

}

@Test
public void testDeleteElementInfos() throws Exception {
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Rename by testReindexAll ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Assert.assertEquals(2, supervisionService.getIndexedCaseElementsCount());

}

Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Need a test for index name ?

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

fileSystem = Jimfs.newFileSystem(Configuration.unix());
caseService.setFileSystem(fileSystem);
caseService.setComputationManager(Mockito.mock(ComputationManager.class));
cleanDB();
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Move cleanDB in the tearDown method

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

done

Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
@jamal-khey jamal-khey requested a review from SlimaneAmar August 18, 2024 19:15
@@ -238,7 +236,10 @@ UUID duplicateCase(UUID sourceCaseUuid, boolean withExpiration) {
CaseInfos existingCaseInfos = caseInfosService.getCaseInfosByUuid(sourceCaseUuid.toString()).orElseThrow();
CaseInfos caseInfos = createInfos(existingCaseInfos.getName(), newCaseUuid, existingCaseInfos.getFormat());
caseInfosService.addCaseInfos(caseInfos);
createCaseMetadataEntity(newCaseUuid, withExpiration);

Optional<CaseMetadataEntity> existingCase = caseMetadataRepository.findById(sourceCaseUuid);
Copy link
Contributor

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Throws NO_FOUND exception like disableCaseExpiration

@SlimaneAmar
Copy link
Contributor

Remove any reference of the word element

Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: jamal-khey <myjamal89@gmail.com>
@jamal-khey jamal-khey requested a review from SlimaneAmar August 19, 2024 12:45
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Signed-off-by: Slimane AMAR <amarsli@gm0winl104.bureau.si.interne>
Copy link

sonarcloud bot commented Aug 21, 2024

@jamal-khey jamal-khey merged commit b9d7b67 into main Aug 21, 2024
4 checks passed
@jamal-khey jamal-khey deleted the jamalkhey/disable-auto-case-indexation branch August 21, 2024 12:51
jonenst added a commit that referenced this pull request Sep 16, 2024
Signed-off-by: jamal-khey <myjamal89@gmail.com>
jonenst pushed a commit that referenced this pull request Sep 16, 2024
Signed-off-by: jamal-khey <myjamal89@gmail.com>
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
Breaking change API is broken
Projects
None yet
Development

Successfully merging this pull request may close these issues.

2 participants