Skip to content

Commit

Permalink
code review remarqs
Browse files Browse the repository at this point in the history 
Signed-off-by: Rehili Ghazwa <ghazwarhili@gmail.com>
  • Loading branch information
@ghazwarhili
ghazwarhili committed Oct 11, 2024
1 parent 2db73b4 commit dba9843
Show file tree
Hide file tree
Showing 8 changed files with 167 additions and 112 deletions.
33 changes: 30 additions & 3 deletions src/main/java/com/powsybl/caseserver/CaseException.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -20,14 +20,17 @@ public final class CaseException extends RuntimeException {

public enum Type {
FILE_NOT_IMPORTABLE,
FILE_NOT_FOUND,
STORAGE_DIR_NOT_CREATED,
ILLEGAL_FILE_NAME,
DIRECTORY_ALREADY_EXISTS,
DIRECTORY_EMPTY,
DIRECTORY_NOT_FOUND,
ORIGINAL_FILE_NOT_FOUND,
TEMP_FILE_INIT,
TEMP_FILE_PROCESS, TEMP_DIRECTORY_CREATION,
TEMP_FILE_PROCESS,
TEMP_DIRECTORY_CREATION,
ZIP_FILE_PROCESS,
UNSUPPORTED_FORMAT
}

Expand All @@ -43,6 +46,11 @@ public CaseException(Type type, String message, Exception e) {
this.type = type;
}

public CaseException(Type type, String message, Throwable e) {
super(message, e);
this.type = type;
}

public Type getType() {
return type;
}
Expand Down Expand Up @@ -72,9 +80,14 @@ public static CaseException createFileNotImportable(Path file) {
return new CaseException(Type.FILE_NOT_IMPORTABLE, "This file cannot be imported: " + file);
}

public static CaseException createFileNotImportable(String file) {
public static CaseException createFileNotImportable(String file, Exception e) {
Objects.requireNonNull(file);
return new CaseException(Type.FILE_NOT_IMPORTABLE, "This file cannot be imported: " + file);
return new CaseException(Type.FILE_NOT_IMPORTABLE, "This file cannot be imported: " + file, e);
}

public static CaseException getFileNameNotFound(UUID uuid) {
Objects.requireNonNull(uuid);
return new CaseException(Type.FILE_NOT_FOUND, "The file name with the following uuid doesn't exist: " + uuid);
}

public static CaseException createStorageNotInitialized(Path storageRootDir) {
Expand All @@ -97,6 +110,11 @@ public static CaseException initTempFile(UUID uuid, Exception e) {
return new CaseException(Type.TEMP_FILE_INIT, "Error initializing temporary case file: " + uuid, e);
}

public static CaseException initTempFile(UUID uuid, Throwable e) {
Objects.requireNonNull(uuid);
return new CaseException(Type.TEMP_FILE_INIT, "Error initializing temporary case file: " + uuid, e);
}

public static CaseException initTempFile(UUID uuid) {
return CaseException.initTempFile(uuid, null);
}
Expand All @@ -106,10 +124,19 @@ public static CaseException processTempFile(UUID uuid, Exception e) {
return new CaseException(Type.TEMP_FILE_PROCESS, "Error processing temporary case file: " + uuid, e);
}

public static CaseException processTempFile(UUID uuid, Throwable e) {
Objects.requireNonNull(uuid);
return new CaseException(Type.TEMP_FILE_PROCESS, "Error processing temporary case file: " + uuid, e);
}

public static CaseException processTempFile(UUID uuid) {
return CaseException.processTempFile(uuid, null);
}

public static CaseException importZipContent(UUID uuid, Exception e) {
return new CaseException(Type.ZIP_FILE_PROCESS, "Error processing zip content file: " + uuid, e);
}

public static CaseException createUnsupportedFormat(String format) {
return new CaseException(Type.UNSUPPORTED_FORMAT, "The format: " + format + " is unsupported");
}
Expand Down
22 changes: 20 additions & 2 deletions src/main/java/com/powsybl/caseserver/service/CaseService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -29,6 +29,7 @@
import java.time.Instant;
import java.time.temporal.ChronoUnit;
import java.util.*;
import java.util.stream.Collectors;
import java.util.zip.ZipEntry;
import java.util.zip.ZipOutputStream;

Expand Down Expand Up @@ -59,6 +60,17 @@ default void createCaseMetadataEntity(UUID newCaseUuid, boolean withExpiration,
caseMetadataRepository.save(new CaseMetadataEntity(newCaseUuid, expirationTime, withIndexation));
}

default List<CaseInfos> getMetadata(List<UUID> ids) {
List<CaseInfos> cases = new ArrayList<>();
ids.forEach(caseUuid -> {
CaseInfos caseInfos = getCaseInfos(caseUuid);
if (Objects.nonNull(caseInfos)) {
cases.add(caseInfos);
}
});
return cases;
}

default Importer getImporterOrThrowsException(Path caseFile, ComputationManager computationManager) {
DataSource dataSource = DataSource.fromPath(caseFile);
Importer importer = Importer.find(dataSource, computationManager);
Expand Down Expand Up @@ -114,6 +126,14 @@ default Optional<ExportCaseInfos> exportCase(UUID caseUuid, String format, Strin
}
}

default List<CaseInfos> getCasesToReindex(CaseMetadataRepository caseMetadataRepository) {
Set<UUID> casesToReindex = caseMetadataRepository.findAllByIndexedTrue()
.stream()
.map(CaseMetadataEntity::getId)
.collect(Collectors.toSet());
return getCases().stream().filter(c -> casesToReindex.contains(c.getUuid())).toList();
}

List<CaseInfos> getCasesToReindex();

List<CaseInfos> getCases();
Expand Down Expand Up @@ -142,7 +162,5 @@ default Optional<ExportCaseInfos> exportCase(UUID caseUuid, String format, Strin

List<CaseInfos> searchCases(String query);

List<CaseInfos> getMetadata(List<UUID> ids);

void setComputationManager(ComputationManager mock);
}
62 changes: 10 additions & 52 deletions src/main/java/com/powsybl/caseserver/service/FsCaseService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -9,9 +9,6 @@
import com.powsybl.caseserver.CaseException;
import com.powsybl.caseserver.dto.CaseInfos;
import com.powsybl.caseserver.elasticsearch.CaseInfosService;
import com.powsybl.caseserver.parsers.FileNameInfos;
import com.powsybl.caseserver.parsers.FileNameParser;
import com.powsybl.caseserver.parsers.FileNameParsers;
import com.powsybl.caseserver.repository.CaseMetadataEntity;
import com.powsybl.caseserver.repository.CaseMetadataRepository;
import com.powsybl.computation.ComputationManager;
Expand All @@ -31,14 +28,11 @@

import java.io.IOException;
import java.io.UncheckedIOException;
import java.nio.file.DirectoryStream;
import java.nio.file.FileSystem;
import java.nio.file.FileSystems;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.StandardCopyOption;
import java.util.*;
import java.util.stream.Collectors;
import java.nio.file.*;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.UUID;
import java.util.stream.Stream;

import static com.powsybl.caseserver.CaseException.createDirectoryNotFound;
Expand Down Expand Up @@ -99,6 +93,9 @@ public String getCaseName(UUID caseUuid) {
throw createDirectoryNotFound(caseUuid);
}
CaseInfos caseInfos = getCaseInfos(file);
if (caseInfos == null) {
throw CaseException.getFileNameNotFound(caseUuid);
}
return caseInfos.getName();
}

Expand Down Expand Up @@ -249,25 +246,8 @@ private CaseMetadataEntity getCaseMetaDataEntity(UUID caseUuid) {
return caseMetadataRepository.findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "case " + caseUuid + " not found"));
}

// TODO should not be duplicated with S3CaseService
public List<CaseInfos> getCasesToReindex() {
Set<UUID> casesToReindex = caseMetadataRepository.findAllByIndexedTrue()
.stream()
.map(CaseMetadataEntity::getId)
.collect(Collectors.toSet());
return getCases(getStorageRootDir()).stream().filter(c -> casesToReindex.contains(c.getUuid())).toList();
}

@Override
public CaseInfos createInfos(String fileBaseName, UUID caseUuid, String format) {
FileNameParser parser = FileNameParsers.findParser(fileBaseName);
if (parser != null) {
Optional<? extends FileNameInfos> fileNameInfos = parser.parse(fileBaseName);
if (fileNameInfos.isPresent()) {
return CaseInfos.create(fileBaseName, caseUuid, format, fileNameInfos.get());
}
}
return CaseInfos.builder().name(fileBaseName).uuid(caseUuid).format(format).build();
return getCasesToReindex(caseMetadataRepository);
}

@Transactional
Expand Down Expand Up @@ -364,7 +344,7 @@ public List<CaseInfos> getCases() {
return walk.filter(Files::isRegularFile)
.map(this::getCaseInfos)
.filter(Objects::nonNull)
.collect(Collectors.toList());
.toList();
} catch (IOException e) {
throw new UncheckedIOException(e);
}
Expand All @@ -377,26 +357,4 @@ public List<CaseInfos> searchCases(String query) {
return caseInfosService.searchCaseInfos(query);
}

public List<CaseInfos> getCases(Path directory) {
try (Stream<Path> walk = Files.walk(directory)) {
return walk.filter(Files::isRegularFile)
.map(file -> createInfos(file.getFileName().toString(), UUID.fromString(file.getParent().getFileName().toString()), getFormat(file)))
.collect(Collectors.toList());
} catch (IOException e) {
throw new UncheckedIOException(e);
}
}

public List<CaseInfos> getMetadata(List<UUID> ids) {
List<CaseInfos> cases = new ArrayList<>();
ids.forEach(caseUuid -> {
Path file = getCaseFile(caseUuid);
if (file != null) {
CaseInfos caseInfos = getCaseInfos(file);
cases.add(caseInfos);
}
});
return cases;
}

}
35 changes: 11 additions & 24 deletions src/main/java/com/powsybl/caseserver/service/S3CaseService.java
View file
Original file line number Diff line number Diff line change
Expand Up @@ -60,6 +60,7 @@
public class S3CaseService implements CaseService {

private static final Logger LOGGER = LoggerFactory.getLogger(S3CaseService.class);
public static final int MAX_SIZE = 500000000;

private ComputationManager computationManager = LocalComputationManager.getDefault();

Expand Down Expand Up @@ -116,7 +117,7 @@ private <R, T1 extends Throwable, T2 extends Throwable> R withTempCopy(UUID case
} catch (CaseException e) {
throw CaseException.initTempFile(caseUuid, e);
} catch (Throwable ex) {
throw CaseException.initTempFile(caseUuid);
throw CaseException.initTempFile(caseUuid, ex);
}
// after this line, need to cleanup the file
try {
Expand All @@ -125,7 +126,7 @@ private <R, T1 extends Throwable, T2 extends Throwable> R withTempCopy(UUID case
} catch (CaseException e) {
throw CaseException.createFileNotImportable(tempdirPath);
} catch (Throwable t) {
throw CaseException.processTempFile(caseUuid);
throw CaseException.processTempFile(caseUuid, t);
}
} finally {
try {
Expand Down Expand Up @@ -353,7 +354,7 @@ public UUID importCase(MultipartFile mpf, boolean withExpiration, boolean withIn
importZipContent(mpf.getInputStream(), caseUuid);
}
} catch (IOException e) {
throw CaseException.createFileNotImportable(caseName);
throw CaseException.createFileNotImportable(caseName, e);
}

createCaseMetadataEntity(caseUuid, withExpiration, withIndexation, caseMetadataRepository);
Expand All @@ -367,11 +368,14 @@ public UUID importCase(MultipartFile mpf, boolean withExpiration, boolean withIn
}

private void importZipContent(InputStream inputStream, UUID caseUuid) throws IOException {
try (ZipInputStream zipInputStream = new SecuredZipInputStream(inputStream, 1000, 500000000)) {
try (ZipInputStream zipInputStream = new SecuredZipInputStream(inputStream, 1000, MAX_SIZE)) {
ZipEntry entry;

while ((entry = zipInputStream.getNextEntry()) != null) {
if (!entry.isDirectory()) {
if (entry.getSize() > MAX_SIZE) {
throw new IOException("File is too large: " + entry.getName());
}
processEntry(caseUuid, zipInputStream, entry);
}
zipInputStream.closeEntry();
Expand Down Expand Up @@ -425,8 +429,8 @@ public UUID duplicateCase(UUID sourceCaseUuid, boolean withExpiration) {
if (caseBytes.isPresent()) {
try {
importZipContent(new ByteArrayInputStream(caseBytes.get()), newCaseUuid);
} catch (IOException ioException) {
throw new UncheckedIOException(ioException);
} catch (Exception e) {
throw CaseException.importZipContent(sourceCaseUuid, e);
}

}
Expand All @@ -441,13 +445,8 @@ public UUID duplicateCase(UUID sourceCaseUuid, boolean withExpiration) {
return newCaseUuid;
}

// TODO should not be duplicated with FSCaseService
public List<CaseInfos> getCasesToReindex() {
Set<UUID> casesToReindex = caseMetadataRepository.findAllByIndexedTrue()
.stream()
.map(CaseMetadataEntity::getId)
.collect(Collectors.toSet());
return getCases().stream().filter(c -> casesToReindex.contains(c.getUuid())).toList();
return getCasesToReindex(caseMetadataRepository);
}

@Transactional
Expand Down Expand Up @@ -528,18 +527,6 @@ public List<CaseInfos> searchCases(String query) {
return caseInfosService.searchCaseInfos(query);
}

@Override
public List<CaseInfos> getMetadata(List<UUID> ids) {
List<CaseInfos> cases = new ArrayList<>();
ids.forEach(caseUuid -> {
CaseInfos caseInfos = getCaseInfos(caseUuid);
if (Objects.nonNull(caseInfos)) {
cases.add(caseInfos);
}
});
return cases;
}

private CaseMetadataEntity getCaseMetaDataEntity(UUID caseUuid) {
return caseMetadataRepository.findById(caseUuid).orElseThrow(() -> new ResponseStatusException(HttpStatus.NOT_FOUND, "case " + caseUuid + NOT_FOUND));
}
Expand Down
Loading

0 comments on commit dba9843

Please sign in to comment.