-
Notifications
You must be signed in to change notification settings - Fork 8.8k
fix: smaller scope of an initial refactoring to bridge scim and admin v2 #51456
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
Open
shawkins
wants to merge
1
commit into
keycloak:main
Choose a base branch
from
shawkins:iss51326-b
base: main
Could not load branches
Branch not found: {{ refName }}
Loading
Could not load tags
Nothing to show
Loading
Are you sure you want to change the base?
Some commits from the old base branch may be removed from the timeline,
and old review comments may become outdated.
Open
Changes from all commits
Commits
File filter
Filter by extension
Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
There are no files selected for viewing
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
165 changes: 165 additions & 0 deletions
165
...in-v2/services/src/main/java/org/keycloak/services/client/scim/BaseClientModelSchema.java
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,165 @@ | ||
| package org.keycloak.services.client.scim; | ||
|
|
||
| import java.util.LinkedHashMap; | ||
| import java.util.List; | ||
| import java.util.Map; | ||
| import java.util.Set; | ||
| import java.util.function.BiConsumer; | ||
|
|
||
| import org.keycloak.common.util.TriConsumer; | ||
| import org.keycloak.models.ClientModel; | ||
| import org.keycloak.models.ModelValidationException; | ||
| import org.keycloak.representations.admin.v2.BaseClientRepresentation; | ||
| import org.keycloak.scim.resource.schema.ModelSchema; | ||
| import org.keycloak.scim.resource.schema.attribute.Attribute; | ||
|
|
||
| /** | ||
| * Abstract schema for client models. Defines the 8 shared JPA-queryable fields as | ||
| * {@link Attribute} objects and provides attribute-filtered population from a {@link ClientModel}. | ||
| * | ||
| * @param <R> the representation type, must extend {@link BaseClientRepresentation} | ||
| */ | ||
| public abstract class BaseClientModelSchema<R extends BaseClientRepresentation> | ||
| implements ModelSchema<ClientModel, R> { | ||
|
|
||
| public static final Set<String> JPA_FIELDS = Set.of( | ||
| "clientId", "enabled", "description", "displayName", | ||
| "protocol", "appUrl", "createdTimestamp", "updatedTimestamp"); | ||
|
|
||
| private final Map<String, Attribute<ClientModel, R>> attributes; | ||
|
|
||
| protected BaseClientModelSchema() { | ||
| Map<String, Attribute<ClientModel, R>> map = new LinkedHashMap<>(); | ||
| map.put("clientId", stringAttr("clientId", "clientId", BaseClientRepresentation::setClientId, ClientModel::setClientId)); | ||
| map.put("enabled", boolAttr ("enabled", "enabled", BaseClientRepresentation::setEnabled, (model, v) -> model.setEnabled(Boolean.TRUE.equals(v)))); | ||
| map.put("description", stringAttr("description", "description", BaseClientRepresentation::setDescription, ClientModel::setDescription)); | ||
| map.put("displayName", stringAttr("displayName", "name", BaseClientRepresentation::setDisplayName, ClientModel::setName)); | ||
| map.put("protocol", stringAttr("protocol", "protocol", BaseClientRepresentation::setProtocol, ClientModel::setProtocol)); | ||
| map.put("appUrl", stringAttr("appUrl", "baseUrl", BaseClientRepresentation::setAppUrl, ClientModel::setBaseUrl)); | ||
| map.put("createdTimestamp", longAttr ("createdTimestamp", "createdTimestamp", BaseClientRepresentation::setCreatedTimestamp, null)); // read-only | ||
| map.put("updatedTimestamp", longAttr ("updatedTimestamp", "lastModifiedTimestamp", BaseClientRepresentation::setUpdatedTimestamp, null)); // read-only | ||
| this.attributes = Map.copyOf(map); | ||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private Attribute<ClientModel, R> stringAttr(String name, String entityField, | ||
| BiConsumer<BaseClientRepresentation, String> repSetter, | ||
| BiConsumer<ClientModel, String> modelSetter) { | ||
| return Attribute.<ClientModel, R>simple(name) | ||
| .modelAttributeResolver(a -> entityField) | ||
| .withModelSetter( | ||
| modelSetter != null ? (TriConsumer<ClientModel, String, String>) (model, n, v) -> modelSetter.accept(model, v) : null, | ||
| (BiConsumer<R, String>) (rep, v) -> repSetter.accept(rep, v)) | ||
| .build() | ||
| .get(0); | ||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private Attribute<ClientModel, R> boolAttr(String name, String entityField, | ||
| BiConsumer<BaseClientRepresentation, Boolean> repSetter, | ||
| BiConsumer<ClientModel, Boolean> modelSetter) { | ||
| return Attribute.<ClientModel, R>simple(name) | ||
| .modelAttributeResolver(a -> entityField) | ||
| .bool() | ||
| .withModelSetter( | ||
| modelSetter != null ? (TriConsumer<ClientModel, String, Boolean>) (model, n, v) -> modelSetter.accept(model, v) : null, | ||
| (BiConsumer<R, Boolean>) (rep, v) -> repSetter.accept(rep, v)) | ||
| .build() | ||
| .get(0); | ||
| } | ||
|
|
||
| @SuppressWarnings("unchecked") | ||
| private Attribute<ClientModel, R> longAttr(String name, String entityField, | ||
| BiConsumer<BaseClientRepresentation, Long> repSetter, | ||
| BiConsumer<ClientModel, Long> modelSetter) { | ||
| return Attribute.<ClientModel, R>simple(name) | ||
| .modelAttributeResolver(a -> entityField) | ||
| .timestamp() | ||
| .withModelSetter( | ||
| modelSetter != null ? (TriConsumer<ClientModel, String, Long>) (model, n, v) -> modelSetter.accept(model, v) : null, | ||
| (BiConsumer<R, Long>) (rep, v) -> repSetter.accept(rep, v)) | ||
| .build() | ||
| .get(0); | ||
| } | ||
|
|
||
| @Override | ||
| public Map<String, Attribute<ClientModel, R>> getAttributes() { | ||
| return attributes; | ||
| } | ||
|
|
||
| @Override | ||
| public Attribute<ClientModel, R> getAttributeByPath(String path) { | ||
| return attributes.get(path); | ||
| } | ||
|
|
||
| /** | ||
| * Populates {@code representation} with fields from {@code model}, honouring inclusion/exclusion filters. | ||
| * Mirrors {@code AbstractModelSchema.populateResourceType} but without {@code setId}/{@code addSchema} calls. | ||
| */ | ||
| @Override | ||
| public void populate(R representation, ClientModel model, List<String> attributes, List<String> excludedAttributes) { | ||
| for (Attribute<ClientModel, R> attribute : this.attributes.values()) { | ||
| if (attribute.isExcluded(this, attributes, excludedAttributes)) { | ||
| continue; | ||
| } | ||
| Object value = getAttributeValue(model, attribute.getModelAttributeName()); | ||
| attribute.set(representation, value); | ||
| } | ||
| } | ||
|
|
||
| /** | ||
| * Returns the value of the named model attribute (using the <em>entity-column</em> name, not the schema name). | ||
| */ | ||
| protected Object getAttributeValue(ClientModel model, String name) { | ||
| return switch (name) { | ||
| case "clientId" -> model.getClientId(); | ||
| case "enabled" -> model.isEnabled(); | ||
| case "description" -> model.getDescription(); | ||
| case "name" -> model.getName(); | ||
| case "protocol" -> model.getProtocol(); | ||
| case "baseUrl" -> model.getBaseUrl(); | ||
| case "createdTimestamp" -> model.getCreatedTimestamp(); | ||
| case "lastModifiedTimestamp" -> model.getLastModifiedTimestamp(); | ||
| default -> null; | ||
| }; | ||
| } | ||
|
|
||
| /** Factory method — subclasses return a fresh, empty representation instance. */ | ||
| public abstract R createRepresentation(); | ||
|
|
||
| // ---- Methods not needed for query/projection use ---- | ||
|
|
||
| /** | ||
| * Populates {@code model} from {@code representation} by calling the model-setter side of each attribute. | ||
| * Read-only attributes (createdTimestamp, updatedTimestamp) are silently skipped. | ||
| */ | ||
| @Override | ||
| public void populate(ClientModel model, R representation) { | ||
| throw new UnsupportedOperationException("populate(ClientModel, R) not yet implemented"); | ||
| } | ||
|
shawkins marked this conversation as resolved.
|
||
|
|
||
| @Override | ||
| public void populate(R representation, ClientModel model) { | ||
| throw new UnsupportedOperationException("populate(R, ClientModel) is not supported — use populate(R, ClientModel, List, List) instead"); | ||
| } | ||
|
|
||
| @Override | ||
| public void validate(R representation) throws ModelValidationException { | ||
| throw new UnsupportedOperationException("validate is not supported"); | ||
| } | ||
|
|
||
| @Override | ||
| public String getId() { | ||
| return ""; // anonymous | ||
| } | ||
|
|
||
| @Override | ||
| public String getName() { | ||
| throw new UnsupportedOperationException("not needed for v2"); | ||
| } | ||
|
|
||
| @Override | ||
| public String getDescription() { | ||
| throw new UnsupportedOperationException("not needed for v2"); | ||
| } | ||
| } | ||
This file contains hidden or bidirectional Unicode text that may be interpreted or compiled differently than what appears below. To review, open the file in an editor that reveals hidden Unicode characters.
Learn more about bidirectional Unicode characters
65 changes: 0 additions & 65 deletions
65
...n-v2/services/src/main/java/org/keycloak/services/client/scim/ClientJpaQueryProvider.java
This file was deleted.
Oops, something went wrong.
Oops, something went wrong.
Oops, something went wrong.
Add this suggestion to a batch that can be applied as a single commit.
This suggestion is invalid because no changes were made to the code.
Suggestions cannot be applied while the pull request is closed.
Suggestions cannot be applied while viewing a subset of changes.
Only one suggestion per line can be applied in a batch.
Add this suggestion to a batch that can be applied as a single commit.
Applying suggestions on deleted lines is not supported.
You must change the existing code in this line in order to create a valid suggestion.
Outdated suggestions cannot be applied.
This suggestion has been applied or marked resolved.
Suggestions cannot be applied from pending reviews.
Suggestions cannot be applied on multi-line comments.
Suggestions cannot be applied while the pull request is queued to merge.
Suggestion cannot be applied right now. Please check back later.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
This is the expected initial state.
Uh oh!
There was an error while loading. Please reload this page.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
IIUC it is not an issue as there is a fallback when field is not
JPA_FIELDS?There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Actually, never mind, I need to re-read it properly.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Right, the issue is that if it is a JPA_FIELDS, we only return one of these JPA fields and not things like uuid, direct uris etc. Ok, got it.
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
In a subsequent PR or commit all (or nearly all) the fields will be added to the schema. The JPA_FIELDS tracking will instead become "searchable fields"