teragrep / cfe_31

0 stars 0 forks source link

Refactor JSON file read location #36

Open MoonBow-1 opened 6 months ago

MoonBow-1 commented 6 months ago

Description

CFE-31 should read JSON files "near" their usage place, instead of reading it once and passing the JsonObject around to the required class. Use case or motivation behind the feature request

Related issues

32 Issue came up from this, but requires more refactoring than a single class

Additional context

Example of correct implementation was pointed to: https://github.com/teragrep/snw_01/blob/main/src/main/java/com/teragrep/snw_01/tableapi/APIResponse.java#71

MoonBow-1 commented 6 months ago

JsonGenerateFile changed from:

public CFE01GenerateFile getCFE01GenerateFile(final CaptureMetaMap captureMetaMap)
    throws JsonParseException {

    final ArrayList<CaptureDefinitionEntry> files = getCFE01Files();

    final JsonOptions jsonOptions = new JsonOptions(this.generateFileAsJson);
    final Options options = jsonOptions.toOptions();

    final JsonTemplates jsonTemplates = new JsonTemplates(this.generateFileAsJson);
    final Templates templates = jsonTemplates.toTemplates();

    final JsonInputTypes jsonInputTypes = new JsonInputTypes(generateFileAsJson);
    final InputTypes inputTypes = jsonInputTypes.toInputTypes();

    final JsonRulebases jsonRulebases = new JsonRulebases(generateFileAsJson);
    final Rulebases rulebases = jsonRulebases.toRulebases();

     // Metrics aren't used anywhere at the moment
    final Metrics metrics = new Metrics();

    return new CFE01GenerateFile(
        files,
        rulebases,
        inputTypes,
        templates,
        options,
        metrics,
        this.pathToFile,
        captureMetaMap
    );
}

to:

public CFE01GenerateFile getCFE01GenerateFile(final CaptureMetaMap captureMetaMap)
    throws JsonParseException {
    try (Reader reader = Files.newBufferedReader(this.pathToFile)) {
        final JsonObject generateFileAsJson = JsonParser.parseReader(reader).getAsJsonObject();

        final ArrayList<CaptureDefinitionEntry> files = getCFE01Files(generateFileAsJson);

        final JsonOptions jsonOptions = new JsonOptions(generateFileAsJson);
        final Options options = jsonOptions.toOptions();

        final JsonTemplates jsonTemplates = new JsonTemplates(generateFileAsJson);
        final Templates templates = jsonTemplates.toTemplates();

        final JsonInputTypes jsonInputTypes = new JsonInputTypes(generateFileAsJson);
        final InputTypes inputTypes = jsonInputTypes.toInputTypes();

        final JsonRulebases jsonRulebases = new JsonRulebases(generateFileAsJson);
        final Rulebases rulebases = jsonRulebases.toRulebases();

      // Metrics aren't used anywhere at the moment
        final Metrics metrics = new Metrics();

        return new CFE01GenerateFile(
            files,
            rulebases,
            inputTypes,
            templates,
            options,
            metrics,
            this.pathToFile,
            captureMetaMap
        );
    } catch (IOException e) {
        throw new RuntimeException(e);
    }
}

Currently contemplating if the reading should still be moved to the individual Json classes, instead of the JsonGenerate

MoonBow-1 commented 6 months ago

Moving the file reading to the "highest" level methods (toOptions, toTemplates etc.) increases memory allocations by about 9%, which is not a concern at this moment.

Compared to the last comment, the getCFE01GenerateFile method looks like:

public CFE01GenerateFile getCFE01GenerateFile(final CaptureMetaMap captureMetaMap)
    throws JsonParseException, IOException {

    final ArrayList<CaptureDefinitionEntry> files = getCFE01Files();

    final JsonOptions jsonOptions = new JsonOptions(this.generateFilePath);
    final Options options = jsonOptions.getOptions();

    final JsonTemplates jsonTemplates = new JsonTemplates(this.generateFilePath);
    final Templates templates = jsonTemplates.getTemplates();

    final JsonInputTypes jsonInputTypes = new JsonInputTypes(this.generateFilePath);
    final InputTypes inputTypes = jsonInputTypes.getInputTypes();

    final JsonRulebases jsonRulebases = new JsonRulebases(this.generateFilePath);
    final Rulebases rulebases = jsonRulebases.getRulebases();

    return new CFE01GenerateFile(
        files,
        rulebases,
        inputTypes,
        templates,
        options,
        new Metrics(),// Metrics aren't used anywhere at the moment
        this.generateFilePath.path,
        captureMetaMap
    );
}

With for example getOptions() looking like:

public Options getOptions() throws IOException {
    try (Reader reader = Files.newBufferedReader(this.generateFilePath.path)) {
        final JsonObject generateFileAsJson = JsonParser.parseReader(reader).getAsJsonObject();

      ...
    }
}
MoonBow-1 commented 6 months ago

Refactored try blocks to be as small as possible, only including the code that throws an exception

MoonBow-1 commented 5 months ago

Cleaned the getCFE01GenerateFile method up a bit:

final Options options = new JsonOptions(this.generateFilePath).getOptions();
final Templates templates = new JsonTemplates(this.generateFilePath).getTemplates();
final InputTypes inputTypes = new JsonInputTypes(this.generateFilePath).getInputTypes();
final Rulebases rulebases = new JsonRulebases(this.generateFilePath).getRulebases();