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

Issue 2843 - store both versioned and unversioned canonicals in ParametersMap #2929

Merged
merged 5 commits into from
Nov 3, 2021

Conversation

lmsurpre
Copy link
Member

@lmsurpre lmsurpre commented Nov 1, 2021

To make this simpler, I also updated the way we apply the filter rules.

Note: this changeset introduces the fhir-ig-us-core as a test dependency for
fhir-search. This was strictly out of convenience...the alternative
would be to create a new test-only PackagedRegistryResourceProvider that
provides multiple versions of the same search parameter(s).

ParametersMap

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
Comment on lines 139 to 144
@Deprecated
public Set<Entry<String, SearchParameter>> urlEntries() {
return Collections.unmodifiableSet(urlMap.entrySet());
return Collections.unmodifiableSet(canonicalMap.entrySet().stream()
.filter(e -> !e.getKey().contains("|"))
.collect(Collectors.toSet()));
}
Copy link
Contributor

Choose a reason for hiding this comment

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

Maybe just remove these deprecated methods? No other team should be depending on this behavior. While observable, it shouldn't be a problem.

Copy link
Member Author

Choose a reason for hiding this comment

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

i was trying to preserve backwards compatibility in the java where possible, but i'm ok removing them now if we don't want to wait

Copy link
Contributor

Choose a reason for hiding this comment

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

I vote for removing

Copy link
Member Author

Choose a reason for hiding this comment

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

done

This changeset introduces the fhir-ig-us-core as a test dependency for
fhir-search. This was strictly out of convenience...the alternative
would be to create a new test-only PackagedRegistryResourceProvider that
provides multiple versions of the same search parameter(s).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre force-pushed the issue-2843 branch 2 times, most recently from 0983d7f to 434ad4a Compare November 2, 2021 17:18
otherwise we run into the following error during validation if us-core
is on the path:
```
SEVERE: validateResource(json/profiles/fhir-ig-us-core/Observation-some-day-smoker.json)
unexpected failure: Input resource failed validation:
generated-us-core-smokingstatus-5: Constraint violation:
effective.where(is(dateTime)).exists() (Observation)
```

I also added back in the fhir-examples.version variable because I think
our release automation still depends on this one (I had maybe
accidentally removed it while removing the other variables for
dependabot).

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
Copy link
Contributor

@prb112 prb112 left a comment

Choose a reason for hiding this comment

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

LGTM (left one comment unresolved for the deprecated code)

per review feedback

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre added the breaking-change-java Identifies a change in a method signature that an implementer may use. label Nov 3, 2021
per review feedback

Signed-off-by: Lee Surprenant <lmsurpre@us.ibm.com>
@lmsurpre lmsurpre merged commit 9492fbe into main Nov 3, 2021
@lmsurpre lmsurpre deleted the issue-2843 branch November 3, 2021 11:42
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
breaking-change-java Identifies a change in a method signature that an implementer may use.
Projects
None yet
Development

Successfully merging this pull request may close these issues.

3 participants