Skip to content
New issue

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

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

Already on GitHub? # to your account

Closes #908 - Added functionality to retrieve the first line of a match. #911

Merged
merged 5 commits into from
Aug 13, 2020

Conversation

MariusBrill
Copy link
Member

@MariusBrill MariusBrill commented Aug 13, 2020

This change is Reviewable

Copy link
Member

@mariusoe mariusoe left a comment

Choose a reason for hiding this comment

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

Reviewed 4 of 4 files at r1.
Reviewable status: all files reviewed, 4 unresolved discussions (waiting on @mbrill-nt)


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/file/FileController.java, line 102 at r1 (raw file):

retrieveFirstLine

Imo, includeFirstLine would be better.

Also ensure that the request accepts it in the following format include-first-line. In the request we don't want to have parameters in camel case .


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/search/FileContentSearchEngine.java, line 127 at r1 (raw file):

Optional.of(content.substring(startLine.getStartIndex(), startLine.getEndIndex())

Please split this up in multiple statements.


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/search/FileContentSearchEngine.java, line 130 at r1 (raw file):

SearchResult

We have a lot of stuff here, now, so let's add the lombok builder the search result class and use a builder.
Don't forget to add the @NoArgsConstructor as well, otherwise it cannot be serialized by spring, I think..

Then we don't have to store all these variables.

while (matcher.find() && limitCounter.decrementAndGet() >= 0) {
    int start = matcher.start();
    SearchResult.SearchResultBuilder resultBuilder = SearchResult.builder();

    while (start >= currentLine.getEndIndex()) {
        currentLine = listIterator.next();
    }
    resultBuilder.startLine(currentLine.getLineNumber());
    resultBuilder.startColumn(start - currentLine.getStartIndex());

    ...

    SearchResult result = resultBuilder.build();
    results.add(result);
}

components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/search/SearchResult.java, line 25 at r1 (raw file):

     * The first line of the match.
     */
    private Optional<String> firstLine;

Don't use an Optional here. Makes it more complicated for serialization.

Also add the @JsonInclude(Include.NON_NULL) annotation to it, then it will be ignored in case it is null.

Copy link
Member Author

@MariusBrill MariusBrill left a comment

Choose a reason for hiding this comment

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

Reviewable status: 0 of 4 files reviewed, 4 unresolved discussions (waiting on @mariusoe)


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/rest/file/FileController.java, line 102 at r1 (raw file):

Previously, mariusoe (Marius Oehler) wrote…
retrieveFirstLine

Imo, includeFirstLine would be better.

Also ensure that the request accepts it in the following format include-first-line. In the request we don't want to have parameters in camel case .

Done.


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/search/FileContentSearchEngine.java, line 127 at r1 (raw file):

Previously, mariusoe (Marius Oehler) wrote…
Optional.of(content.substring(startLine.getStartIndex(), startLine.getEndIndex())

Please split this up in multiple statements.

Done.


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/search/FileContentSearchEngine.java, line 130 at r1 (raw file):

Previously, mariusoe (Marius Oehler) wrote…
SearchResult

We have a lot of stuff here, now, so let's add the lombok builder the search result class and use a builder.
Don't forget to add the @NoArgsConstructor as well, otherwise it cannot be serialized by spring, I think..

Then we don't have to store all these variables.

while (matcher.find() && limitCounter.decrementAndGet() >= 0) {
    int start = matcher.start();
    SearchResult.SearchResultBuilder resultBuilder = SearchResult.builder();

    while (start >= currentLine.getEndIndex()) {
        currentLine = listIterator.next();
    }
    resultBuilder.startLine(currentLine.getLineNumber());
    resultBuilder.startColumn(start - currentLine.getStartIndex());

    ...

    SearchResult result = resultBuilder.build();
    results.add(result);
}

Done.


components/inspectit-ocelot-configurationserver/src/main/java/rocks/inspectit/ocelot/search/SearchResult.java, line 25 at r1 (raw file):

Previously, mariusoe (Marius Oehler) wrote…

Don't use an Optional here. Makes it more complicated for serialization.

Also add the @JsonInclude(Include.NON_NULL) annotation to it, then it will be ignored in case it is null.

Done.

Copy link
Member

@mariusoe mariusoe left a comment

Choose a reason for hiding this comment

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

:lgtm:

Reviewed 4 of 4 files at r2, 1 of 1 files at r3.
Reviewable status: all files reviewed, 1 unresolved discussion (waiting on @mariusoe)

Copy link
Member

@mariusoe mariusoe left a comment

Choose a reason for hiding this comment

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

Reviewable status: :shipit: complete! all files reviewed, all discussions resolved

@mariusoe mariusoe merged commit 3cfb897 into inspectIT:master Aug 13, 2020
@MariusBrill MariusBrill deleted the Insert_firstline_match branch May 25, 2021 10:48
# for free to join this conversation on GitHub. Already have an account? # to comment
Labels
None yet
Projects
None yet
Development

Successfully merging this pull request may close these issues.

Extend configuration file search by optionally inserting the first line of a match
2 participants