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

Remove amended by and amended at filter from loadArticles #1005

Merged
merged 2 commits into from
Feb 14, 2025
Merged
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -63,14 +63,8 @@ public ResponseEntity<List<ArticleResponseSchema>> getArticles(
@RequestParam final Optional<DokumentExpressionEli> amendedBy,
@RequestParam final Optional<String> amendedAt
) {
final var query = new LoadArticlesFromDokumentUseCase.Query(
eli,
amendedBy.orElse(null),
amendedAt.orElse(null)
);

final var articlesWithZf0 = loadArticlesFromDokumentUseCase
.loadArticlesFromDokument(query)
.loadArticlesFromDokument(new LoadArticlesFromDokumentUseCase.Query(eli))
.stream()
.map(ArticleResponseMapper::fromNormArticle)
.toList();
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -94,9 +94,7 @@ public ResponseEntity<List<ElementResponseSchema>> getElementList(
@RequestParam final Optional<DokumentExpressionEli> amendedBy
) {
List<ElementResponseSchema> elements = loadElementsByTypeUseCase
.loadElementsByType(
new LoadElementsByTypeUseCase.Query(eli, Arrays.asList(type), amendedBy.orElse(null))
)
.loadElementsByType(new LoadElementsByTypeUseCase.Query(eli, Arrays.asList(type)))
.stream()
.map(ElementResponseMapper::fromElementNode)
.toList();
Expand Down
Original file line number Diff line number Diff line change
@@ -1,16 +1,14 @@
package de.bund.digitalservice.ris.norms.application.port.input;

import de.bund.digitalservice.ris.norms.domain.entity.Article;
import de.bund.digitalservice.ris.norms.domain.entity.Dokument;
import de.bund.digitalservice.ris.norms.domain.entity.eli.DokumentExpressionEli;
import java.util.List;
import javax.annotation.Nullable;

/** Use case for getting a list of {@link Article}s from a Dokument (eg. {@link de.bund.digitalservice.ris.norms.domain.entity.Regelungstext}). */
/** Use case for getting a list of {@link Article}s from a {@link Dokument}. */
public interface LoadArticlesFromDokumentUseCase {
/**
* Load the list of articles from a dokument. Articles can be filtered based on whether they have
* pending (passive) modifications that originate from a specific law, or that will be applied at
* a specific date.
* Load the list of articles from a dokument.
*
* @param query Query used for identifying the articles
* @return List of articles (can be empty)
Expand All @@ -21,19 +19,6 @@ public interface LoadArticlesFromDokumentUseCase {
* Contains the parameters needed for loading articles from a dokument.
*
* @param eli The ELI used to identify the dokument
* @param amendedBy ELI of an amending law. When specified, only articles with passive
* modifications from that amending law are included in the result.
* @param amendedAt eId of a lifecycle event. When specified, only articles with passive
* modifications that will be applied at the date of this lifecycle event will be included in
* the result.
*/
record Query(
DokumentExpressionEli eli,
@Nullable DokumentExpressionEli amendedBy,
@Nullable String amendedAt
) {
public Query(DokumentExpressionEli eli) {
this(eli, null, null);
}
}
record Query(DokumentExpressionEli eli) {}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,14 +4,12 @@
import de.bund.digitalservice.ris.norms.domain.entity.eli.DokumentExpressionEli;
import de.bund.digitalservice.ris.norms.utils.exceptions.NormsAppException;
import java.util.List;
import javax.annotation.Nullable;
import org.w3c.dom.Node;

/** Use case for getting a list of elements of certain types as {@link Node} from a dokument. */
public interface LoadElementsByTypeUseCase {
/**
* Load the list of elements of certain types from the dokument. Elements can additionally be filtered
* to include only those touched by a specific amending command.
* Load the list of elements of certain types from the dokument.
*
* @return List of elements from the dokument
* @param query Query used for identifying the elements
Expand All @@ -24,18 +22,8 @@ public interface LoadElementsByTypeUseCase {
* @param eli The ELI used to identify the dokument
* @param elementType The types of the elements. While this is a list of strings, only certain
* values are allowed. Check {@link ElementService.ElementType} for the supported types.
* @param amendedBy EId of an amending command. If provided, filters the list to include only
* elements touched by that amending command.
*/
record Query(
DokumentExpressionEli eli,
List<String> elementType,
@Nullable DokumentExpressionEli amendedBy
) {
public Query(DokumentExpressionEli eli, List<String> elementType) {
this(eli, elementType, null);
}
}
record Query(DokumentExpressionEli eli, List<String> elementType) {}

/** Indicates that at least one of the requested types is not supported. */
class UnsupportedElementTypeException
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -6,13 +6,9 @@
import de.bund.digitalservice.ris.norms.application.port.input.*;
import de.bund.digitalservice.ris.norms.application.port.output.LoadRegelungstextPort;
import de.bund.digitalservice.ris.norms.domain.entity.*;
import de.bund.digitalservice.ris.norms.domain.entity.eli.DokumentExpressionEli;
import de.bund.digitalservice.ris.norms.utils.XmlMapper;
import java.util.Collections;
import java.util.List;
import java.util.Objects;
import java.util.Optional;
import java.util.function.Predicate;
import org.springframework.stereotype.Service;

/** Service for loading a norm's articles */
Expand Down Expand Up @@ -56,57 +52,11 @@ public String loadArticleHtml(final LoadArticleHtmlUseCase.Query query) {

@Override
public List<Article> loadArticlesFromDokument(final LoadArticlesFromDokumentUseCase.Query query) {
final var amendedAt = query.amendedAt();
final var amendedBy = query.amendedBy();

final var regelungstext = loadRegelungstextPort
.loadRegelungstext(new LoadRegelungstextPort.Command(query.eli()))
.orElseThrow(() -> new RegelungstextNotFoundException(query.eli().toString()));

List<Article> articles = regelungstext.getArticles();

// Filter list of articles by passive mods if at least one filter is specified
if (amendedBy != null || amendedAt != null) {
var filterPassiveMods = getPassiveModsAmendingByOrAt(regelungstext, amendedBy, amendedAt);
var passiveModFilter = createPassiveModFilter(filterPassiveMods);
articles = articles.stream().filter(passiveModFilter).toList();
}

return articles;
}

private List<TextualMod> getPassiveModsAmendingByOrAt(
final Regelungstext fromRegelungstext,
final DokumentExpressionEli amendingBy,
final String amendingAt
) {
if (amendingBy == null && amendingAt == null) return List.of();

return fromRegelungstext
.getMeta()
.getAnalysis()
.map(Analysis::getPassiveModifications)
.orElse(Collections.emptyList())
.stream()
.filter(passiveModification -> {
if (amendingBy == null) return true;

return passiveModification
.getSourceHref()
.flatMap(Href::getExpressionEli)
.map(sourceEli -> sourceEli.equals(amendingBy))
.orElse(false);
})
.filter(passiveModification -> {
if (amendingAt == null) return true;

return passiveModification
.getForcePeriodEid()
.flatMap(fromRegelungstext::getStartEventRefForTemporalGroup)
.map(startEventRef -> startEventRef.equals(amendingAt))
.orElse(false);
})
.toList();
return regelungstext.getArticles();
}

@Override
Expand All @@ -132,24 +82,4 @@ public List<String> loadSpecificArticlesXmlFromDokument(

return articles.stream().map(a -> XmlMapper.toString(a.getElement())).toList();
}

private Predicate<Article> createPassiveModFilter(final List<TextualMod> mods) {
return article ->
// If we filter by amendedAt or amendedBy: Those properties are found
// in the passive modifications we already collected above. What's left
// now is to only return the articles that are going to be modified by
// those passive modifications.
mods
.stream()
.map(TextualMod::getDestinationHref)
.flatMap(Optional::stream)
.map(Href::getEId)
.flatMap(Optional::stream)
.anyMatch(destinationEid ->
// Modifications can be either on the article itself or anywhere
// inside the article, hence the "contains" rather than exact
// matching.
destinationEid.contains(article.getEid())
);
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -4,18 +4,10 @@
import de.bund.digitalservice.ris.norms.application.exception.RegelungstextNotFoundException;
import de.bund.digitalservice.ris.norms.application.port.input.*;
import de.bund.digitalservice.ris.norms.application.port.output.LoadRegelungstextPort;
import de.bund.digitalservice.ris.norms.domain.entity.Analysis;
import de.bund.digitalservice.ris.norms.domain.entity.EId;
import de.bund.digitalservice.ris.norms.domain.entity.Href;
import de.bund.digitalservice.ris.norms.domain.entity.Norm;
import de.bund.digitalservice.ris.norms.domain.entity.TextualMod;
import de.bund.digitalservice.ris.norms.domain.entity.eli.DokumentExpressionEli;
import de.bund.digitalservice.ris.norms.utils.NodeParser;
import de.bund.digitalservice.ris.norms.utils.XmlMapper;
import java.util.Collections;
import java.util.List;
import java.util.Optional;
import javax.annotation.Nullable;
import org.springframework.stereotype.Service;
import org.w3c.dom.Node;

Expand Down Expand Up @@ -120,52 +112,13 @@ public List<Node> loadElementsByType(LoadElementsByTypeUseCase.Query query) {
.loadRegelungstext(new LoadRegelungstextPort.Command(query.eli()))
.orElseThrow(() -> new RegelungstextNotFoundException(query.eli().toString()));

// Source EIDs from passive mods
final var passiveModsDestinationEids = getDestinationEidsFromPassiveMods(
regelungstext
.getMeta()
.getAnalysis()
.map(Analysis::getPassiveModifications)
.orElse(Collections.emptyList()),
query.amendedBy()
);

return NodeParser
.getNodesFromExpression(combinedXPaths, regelungstext.getDocument())
.stream()
.filter(element -> { // filter by "amendedBy")
// no amending law -> all elements are fine
if (query.amendedBy() == null) return true;

var eId = EId.fromMandatoryNode(element).value();
return passiveModsDestinationEids.stream().anyMatch(modEid -> modEid.contains(eId));
})
.toList();
}

private String getXPathForEid(String eid) {
return String.format("//*[@eId='%s']", eid);
}

private List<String> getDestinationEidsFromPassiveMods(
List<TextualMod> mods,
@Nullable DokumentExpressionEli amendedBy
) {
return mods
.stream()
.filter(passiveMod -> {
if (amendedBy == null) return true;

return passiveMod
.getSourceHref()
.flatMap(Href::getExpressionEli)
.orElseThrow()
.equals(amendedBy);
})
.map(TextualMod::getDestinationHref)
.flatMap(Optional::stream)
.map(Href::getEId)
.flatMap(Optional::stream)
.toList();
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -99,96 +99,6 @@ void itReturnsArticles() throws Exception {
);
}

@Test
void itReturnsArticlesFilteredByAmendedAt() throws Exception {
// Given
var regelungstext = Fixtures.loadRegelungstextFromDisk(
"NormWithMultiplePassiveModifications.xml"
);

when(loadRegelungstextUseCase.loadRegelungstext(any())).thenReturn(regelungstext);

when(loadArticlesFromDokumentUseCase.loadArticlesFromDokument(any()))
.thenReturn(
regelungstext
.getArticles()
.stream()
.filter(article -> article.getEid().equals("hauptteil-1_art-1"))
.toList()
);

// When
mockMvc
.perform(
get(
"/api/v1/norms/eli/bund/bgbl-1/2023/413/2023-12-29/1/deu/regelungstext-1/articles?amendedAt=meta-1_lebzykl-1_ereignis-4"
)
.accept(MediaType.APPLICATION_JSON)
)
// Then
.andExpect(status().isOk())
.andExpect(jsonPath("$[0]").exists())
.andExpect(jsonPath("$[0].eid").value("hauptteil-1_art-1"))
.andExpect(jsonPath("$[1]").doesNotExist());

verify(loadArticlesFromDokumentUseCase, times(1))
.loadArticlesFromDokument(
new LoadArticlesFromDokumentUseCase.Query(
DokumentExpressionEli.fromString(
"eli/bund/bgbl-1/2023/413/2023-12-29/1/deu/regelungstext-1"
),
null,
"meta-1_lebzykl-1_ereignis-4"
)
);
}

@Test
void itReturnsArticlesFilteredByAmendedBy() throws Exception {
// Given
var regelungstext = Fixtures.loadRegelungstextFromDisk(
"NormWithPassiveModificationsInDifferentArticles.xml"
);

when(loadRegelungstextUseCase.loadRegelungstext(any())).thenReturn(regelungstext);

when(loadArticlesFromDokumentUseCase.loadArticlesFromDokument(any()))
.thenReturn(
regelungstext
.getArticles()
.stream()
.filter(article -> article.getEid().equals("hauptteil-1_art-1"))
.toList()
);

// When
mockMvc
.perform(
get(
"/api/v1/norms/eli/bund/bgbl-1/2023/413/2023-12-29/1/deu/regelungstext-1/articles?amendedBy=eli/bund/bgbl-1/2017/s815/1995-03-15/1/deu/regelungstext-1"
)
.accept(MediaType.APPLICATION_JSON)
)
// Then
.andExpect(status().isOk())
.andExpect(jsonPath("$[0]").exists())
.andExpect(jsonPath("$[0].eid").value("hauptteil-1_art-1"))
.andExpect(jsonPath("$[1]").doesNotExist());

verify(loadArticlesFromDokumentUseCase, times(1))
.loadArticlesFromDokument(
new LoadArticlesFromDokumentUseCase.Query(
DokumentExpressionEli.fromString(
"eli/bund/bgbl-1/2023/413/2023-12-29/1/deu/regelungstext-1"
),
DokumentExpressionEli.fromString(
"eli/bund/bgbl-1/2017/s815/1995-03-15/1/deu/regelungstext-1"
),
null
)
);
}

@Test
void itReturnsUnprocessableEntityWhenMandatoryNodeIsMissing() throws Exception {
// Given
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -190,10 +190,7 @@ void itReturnsOnlyTheElementsMatchingTheGivenAmendingLaw() throws Exception {
DokumentExpressionEli.fromString(
"eli/bund/bgbl-1/1990/s2954/2022-12-19/1/deu/regelungstext-1"
),
eq(List.of("preface", "preamble", "article", "conclusions")),
DokumentExpressionEli.fromString(
"eli/bund/bgbl-1/2017/s815/1995-03-15/1/deu/regelungstext-1"
)
eq(List.of("preface", "preamble", "article", "conclusions"))
)
)
)
Expand Down
Loading