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

When @EnableHypermediaSupport has no types, only enable user-defined media types. #1060

Open
gregturn opened this issue Aug 31, 2019 · 6 comments · May be fixed by #1265
Open

When @EnableHypermediaSupport has no types, only enable user-defined media types. #1060

gregturn opened this issue Aug 31, 2019 · 6 comments · May be fixed by #1265
Assignees
Labels
in: configuration Configuration and setup

Comments

@gregturn
Copy link
Contributor

If an empty list of hypermedia types is provided, activate all of them.

Also consider making an empty list the default value for this annotation.

@gregturn gregturn added the in: configuration Configuration and setup label Aug 31, 2019
@gregturn gregturn added this to the 1.0.0.RC2 milestone Aug 31, 2019
@gregturn gregturn self-assigned this Aug 31, 2019
gregturn referenced this issue Aug 31, 2019
Clean up code containing various patterns:

* Remove redundant `final`, `public`, and `static` declarations in interfaces and enums.
* Replace redundant `? extends Object` generic parameters with `?`.
* Convert collections to arrays using empty array instead of pre-sized one (modern JVM recommendation).
* Initialize collections using constructor call over `addAll`.
* Favor `addAll` over iterating and adding one-by-one.
* Take advantage of `Map.forEach` that was introduced with Java 8.
@gregturn
Copy link
Contributor Author

gregturn commented Aug 31, 2019

When using an empty list, the old code had this:

if (types.isEmpty()) {
	return configurationProviders.toArray(new String[0]);
}

This was incorrect, and would produce an ArrayStoreException.

Instead, the purpose is to enable ALL the MediaTypeConfigurationProvider found in the application context, like this:

if (types.isEmpty()) {
	return configurationProviders.stream() //
			.map(MediaTypeConfigurationProvider::getConfiguration) //
			.map(Class::getName) //
			.toArray(String[]::new);
}

@gregturn
Copy link
Contributor Author

Since a Stream must be created in either situation, this may be preferable:

return configurationProviders.stream() //
		.filter(it -> {
			// If there are no types, then let them all through
			if (types.isEmpty()) {
				return true;
			}
			// Filter the ones supporting the given media types
			return it.supportsAny(types);
		}) //
		.map(MediaTypeConfigurationProvider::getConfiguration) //
		.map(Class::getName) //
		.toArray(String[]::new);

gregturn added a commit that referenced this issue Aug 31, 2019
gregturn added a commit that referenced this issue Sep 4, 2019
@gregturn gregturn modified the milestones: 1.0.0.RC2, 1.0.0.RELEASE Sep 5, 2019
@gregturn
Copy link
Contributor Author

gregturn commented Sep 5, 2019

Duplicates #1015 .

@gregturn gregturn modified the milestones: 1.0.0.RELEASE, 1.0.0.RC3 Sep 16, 2019
@gregturn gregturn modified the milestones: 1.0.0.RELEASE, 1.1.0.M1 Sep 26, 2019
@odrotbohm odrotbohm modified the milestones: 1.1.0.M1, 1.1.0.M2 Jan 14, 2020
@odrotbohm odrotbohm modified the milestones: 1.1.0.M2, 1.1.0 M3 Feb 11, 2020
@gregturn gregturn modified the milestones: 1.1.0.M3, 1.1.0.M4 Mar 31, 2020
gregturn added a commit that referenced this issue Apr 9, 2020
gregturn added a commit that referenced this issue Apr 9, 2020
Default it to being empty. Existing code, `@EnableHypermediaSupport(types = HAL)` will still operate as expected. But simply using the annotation with no arguments will activate ALL types.
gregturn added a commit that referenced this issue Apr 9, 2020
Default it to being empty. Existing code, `@EnableHypermediaSupport(types = HAL)` will still operate as expected. But simply using the annotation with no arguments will activate ALL types.
gregturn added a commit that referenced this issue Apr 9, 2020
Default it to being empty. Existing code, `@EnableHypermediaSupport(types = HAL)` will still operate as expected. But simply using the annotation with no arguments will activate ALL types.

Related pull request: #1065.
gregturn added a commit that referenced this issue Apr 9, 2020
Default it to being empty. Existing code, `@EnableHypermediaSupport(types = HAL)` will still operate as expected. But simply using the annotation with no arguments will activate ALL types.

Related issues: #1015.
Related pull request: #1065, #1265.
gregturn added a commit that referenced this issue Apr 9, 2020
Default it to being empty. Existing code, `@EnableHypermediaSupport(types = HAL)` will still operate as expected. But simply using the annotation with no arguments will activate ALL types.

Related issues: #1015.
Related pull request: #1065, #1265.
@gregturn gregturn modified the milestones: 1.1.0.RC1, 1.1.0.RELEASE Apr 27, 2020
@gregturn
Copy link
Contributor Author

gregturn commented May 5, 2020

UPDATE: If enabled but empty, then don't register the pre-built ones. BUT activate custom ones.

Issue a warning if NO hypermedia types are found.

@gregturn gregturn changed the title Handle @EnableHypermediaSupport(types={}) When @EnableHypermediaSupport has no types, only enable user-defined media types. May 6, 2020
@gregturn
Copy link
Contributor Author

gregturn commented May 6, 2020

As discussed in a video call, we are switching it up so that enabling hypermedia support with no specified types, Spring HATEOAS will attempt to ONLY enable user-defined types.

If NO media types are found, issue a warning that you've enabled hypermedia support but not provided a media type.

This will avoid enabling newly added types from the team.

May want to include a custom annotation where the user can define a mediatype, but shield it from being picked up.

@gregturn
Copy link
Contributor Author

gregturn commented Aug 4, 2020

Because @EnableHypermediaSupport kicks in early in the lifecycle, we can't look for HypermediaMappingInformation types at that stage.

Only alternative to ANY sort of check sounds like some lifecycle event handler that validates right before the app goes active.

@odrotbohm odrotbohm modified the milestones: 1.2 M1, 1.2 RC1 Aug 11, 2020
@gregturn gregturn modified the milestones: 1.2 RC1, 1.2 GA Sep 15, 2020
@odrotbohm odrotbohm modified the milestones: 1.2 GA, 1.3 M1 Oct 27, 2020
@odrotbohm odrotbohm modified the milestones: 1.3 M1, 1.3 M2 Jan 13, 2021
@odrotbohm odrotbohm modified the milestones: 1.3 M2, 1.3 RC1 Feb 16, 2021
@odrotbohm odrotbohm modified the milestones: 1.3 M3, 1.4 M1 Mar 15, 2021
@odrotbohm odrotbohm modified the milestones: 1.4 M1, 1.4 M2 Jul 14, 2021
@odrotbohm odrotbohm modified the milestones: 1.4 M2, 1.4 M3 Aug 12, 2021
@odrotbohm odrotbohm modified the milestones: 1.4 M3, 1.4 candidates Sep 16, 2021
@odrotbohm odrotbohm removed this from the 1.4 candidates milestone Nov 13, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment