Introduce a dedicated ProjectClient interface
#132237
There are no files selected for viewing
| Original file line number | Diff line number | Diff line change |
|---|---|---|
| @@ -0,0 +1,46 @@ | ||
| /* | ||
| * Copyright Elasticsearch B.V. and/or licensed to Elasticsearch B.V. under one | ||
| * or more contributor license agreements. Licensed under the "Elastic License | ||
| * 2.0", the "GNU Affero General Public License v3.0 only", and the "Server Side | ||
| * Public License v 1"; you may not use this file except in compliance with, at | ||
| * your election, the "Elastic License 2.0", the "GNU Affero General Public | ||
| * License v3.0 only", or the "Server Side Public License, v 1". | ||
| */ | ||
|
|
||
| package org.elasticsearch.client.internal; | ||
|
|
||
| import org.elasticsearch.action.ActionListener; | ||
| import org.elasticsearch.action.ActionRequest; | ||
| import org.elasticsearch.action.ActionResponse; | ||
| import org.elasticsearch.action.ActionType; | ||
| import org.elasticsearch.cluster.metadata.ProjectId; | ||
|
|
||
| /** | ||
| * A dedicated {@link Client} that is scoped to a specific project. It will set the project ID in the thread context before executing any | ||
| * requests. | ||
| */ | ||
| public class ProjectClient extends FilterClient { | ||
|
|
||
| private final ProjectId projectId; | ||
|
|
||
| public ProjectClient(Client in, ProjectId projectId) { | ||
| super(in); | ||
| this.projectId = projectId; | ||
| } | ||
|
|
||
| @Override | ||
| protected <Request extends ActionRequest, Response extends ActionResponse> void doExecute( | ||
| ActionType<Response> action, | ||
| Request request, | ||
| ActionListener<Response> listener | ||
| ) { | ||
| projectResolver().executeOnProject(projectId, () -> super.doExecute(action, request, listener)); | ||
| } | ||
|
|
||
| @Override | ||
| public ProjectClient projectClient(ProjectId projectId) { | ||
| throw new IllegalStateException( | ||
| "Unable to create a project client for project [" + projectId + "], nested project client creation is not supported" | ||
| ); | ||
| } | ||
| } | ||
| Original file line number | Diff line number | Diff line change |
|---|---|---|
|
|
@@ -76,6 +76,7 @@ | |
| import org.elasticsearch.client.internal.AdminClient; | ||
| import org.elasticsearch.client.internal.Client; | ||
| import org.elasticsearch.client.internal.FilterClient; | ||
| import org.elasticsearch.client.internal.ProjectClient; | ||
| import org.elasticsearch.cluster.metadata.ProjectId; | ||
| import org.elasticsearch.cluster.project.ProjectResolver; | ||
| import org.elasticsearch.common.settings.Settings; | ||
|
|
@@ -96,6 +97,7 @@ public abstract class AbstractClient implements Client { | |
| private final ThreadPool threadPool; | ||
| private final ProjectResolver projectResolver; | ||
| private final AdminClient admin; | ||
| private ProjectClient defaultProjectClient; | ||
|
|
||
| @SuppressWarnings("this-escape") | ||
| public AbstractClient(Settings settings, ThreadPool threadPool, ProjectResolver projectResolver) { | ||
|
|
@@ -122,7 +124,7 @@ public ProjectResolver projectResolver() { | |
| } | ||
|
|
||
| @Override | ||
| public AdminClient admin() { | ||
nielsbauman marked this conversation as resolved.
Outdated
Show resolved
Hide resolved
|
||
| return admin; | ||
| } | ||
|
|
||
|
|
@@ -417,29 +419,27 @@ protected <Request extends ActionRequest, Response extends ActionResponse> void | |
| } | ||
|
|
||
| @Override | ||
| public ProjectClient projectClient(ProjectId projectId) { | ||
| // We only take the shortcut when the given project ID matches the "current" project ID. If it doesn't, we'll let #executeOnProject | ||
| // take care of error handling. | ||
| if (projectResolver.supportsMultipleProjects() == false && projectId.equals(projectResolver.getProjectId())) { | ||
nielsbauman marked this conversation as resolved.
Show resolved
Hide resolved
|
||
| // We construct a dedicated project client for the default project if we're in single project mode. This dedicated project | ||
| // client is an optimization in that it does not use the project resolver and instead executes the request directly. | ||
| if (defaultProjectClient == null) { | ||
| defaultProjectClient = new ProjectClient(this, projectId) { | ||
|
||
| @Override | ||
| protected <Request extends ActionRequest, Response extends ActionResponse> void doExecute( | ||
| ActionType<Response> action, | ||
| Request request, | ||
| ActionListener<Response> listener | ||
| ) { | ||
| in().execute(action, request, listener); | ||
| } | ||
| }; | ||
| } | ||
| return defaultProjectClient; | ||
| } | ||
| return new ProjectClient(this, projectId); | ||
| } | ||
|
|
||
| /** | ||
|
|
||
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.
I had thought the
ProjectClientwould be an interface rather than a concrete class, e.g. something likeHaving a concrete
ProjectClientclass that extendsFilterClientfeels a bit odd to me since they are conceptually different things. Having theFilterClientas the implementation and returned from theprojectClient()method makes more sense to me.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.
Why are they conceptually different things to you? All that a
ProjectClientdoes is add the project ID in the thread context, which is exactly what aFilterClientis for.ProjectClientdoesn't need to extendFilterClient. It can extendAbstractClientas well, but extendingFilterClientreduced a little bit of boilerplate code. Can you explain why you think they are conceptually different but you do think using aFilterClientat runtime does make sense?IMO, a
ProjectClientinterface like that doesn't really dictate anything about the client. The only method it provides is the getter. Any usages of the interface will just have to "trust" that an implementing class does the project scoping in the requests. In other words, the interface doesn't really provide any guarantee. What do you think?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.
I'd think this is an implementation detail and not what the interface should care. To me,
ProjectClientis a business concept which really just means this client is supposed to work with a single project (i.e. it's scope, denoted by its interface facegetProjectId()), whileFilterClientis an implementation choice. Like you said, it does not even have to be implemented withFilterClient.I am not sure whether this really is a problem. We can argue that
Client#filterWithHeadermethod does not provide any guarantee either. The contract can be in the javadoc (JDK's own interfaces do that). If we do feel some more hints are needed, we can augument the interface to be something like the followings: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.
Ok, you convinced me. Thanks for explaining. I converted
ProjectClientto an interface in 63e371b.