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

Refactored TransportSingleCustomOperationAction, subclasses and requests #7214

Closed
wants to merge 2 commits into from

Conversation

Projects
None yet
3 participants
@javanna
Copy link
Member

commented Aug 9, 2014

TransportSingleCustomOperationAction is subclassed by two similar, yet different transport actions: TransportAnalyzeAction and TransportGetFieldMappingsIndexAction. Made their difference and similarities more explicit by sharing common code and moving specific code to subclasses:

  • moved index field to the parent SingleCustomOperationAction class
  • moved the common check blocks code to the parent transport action class
  • moved the main transport handler to the TransportAnalyzeAction subclass as it is only used to receive external requests through clients. In the case of the TransportGetFieldMappingsIndexAction instead, the action is internal and executed only locally as part of the user facing TransportGetFieldMappingsAction. The corresponding request gets sent over the transport though as part of the related shard request
  • removed the get field mappings index action from the action names mapping as it is not a transport handler anymore. It was before although never used.
Internal: refactored TransportSingleCustomOperationAction, subclasses…
… and requests

TransportSingleCustomOperationAction is subclassed by two similar, yet different transport action: TransportAnalyzeAction and TransportGetFieldMappingsAction. Made their difference and similarities more explicit by sharing common code and moving specific code to subclasses:
- moved index field to the parent SingleCustomOperationAction class
- moved the common check blocks code to the parent transport action class
- moved the main transport handler to the TransportAnalyzeAction subclass as it is only used to receive external requests through clients. In the case of the TransportGetFieldMappingsIndexAction instead, the action is internal and executed only locally as part of the user facing TransportGetFieldMappingsAction. The corresponding request gets sent over the transport though as part of the related shard request
- removed the get field mappings index action from the action names mapping as it is not a transport handler anymore. It was before although never used.

@javanna javanna added review labels Aug 9, 2014

@javanna javanna self-assigned this Aug 9, 2014

@@ -83,12 +111,18 @@ public void beforeLocalFork() {
public void readFrom(StreamInput in) throws IOException {
super.readFrom(in);
preferLocal = in.readBoolean();
if (in.getVersion().onOrAfter(Version.V_1_4_0)) {
index = in.readOptionalString();
}

This comment has been minimized.

Copy link
@jpountz

jpountz Aug 11, 2014

Contributor

Since the index on not optional for the get field mappings action, maybe there should rather be 2 overridable protected methods readIndex and writeIndex? This way we wouldn't even need the version checks?

This comment has been minimized.

Copy link
@javanna

javanna Aug 11, 2014

Author Member

makes sense, the reason why I didn't do it is that on the long run bw comp checks will go away and everything would be simpler if the index is serialized by the class that holds it. On the other hand the additional methods you suggest don't complicate things that much.

@jpountz jpountz removed the review label Aug 11, 2014

@javanna

This comment has been minimized.

Copy link
Member Author

commented Aug 11, 2014

Updated according to review's comment

@jpountz

This comment has been minimized.

Copy link
Contributor

commented Aug 11, 2014

LGTM

@jpountz jpountz removed the review label Aug 11, 2014

javanna added a commit that referenced this pull request Aug 11, 2014

Internal: refactored TransportSingleCustomOperationAction, subclasses…
… and requests

TransportSingleCustomOperationAction is subclassed by two similar, yet different transport action: TransportAnalyzeAction and TransportGetFieldMappingsAction. Made their difference and similarities more explicit by sharing common code and moving specific code to subclasses:
- moved index field to the parent SingleCustomOperationAction class
- moved the common check blocks code to the parent transport action class
- moved the main transport handler to the TransportAnalyzeAction subclass as it is only used to receive external requests through clients. In the case of the TransportGetFieldMappingsIndexAction instead, the action is internal and executed only locally as part of the user facing TransportGetFieldMappingsAction. The corresponding request gets sent over the transport though as part of the related shard request
- removed the get field mappings index action from the action names mapping as it is not a transport handler anymore. It was before although never used.

Closes #7214

@javanna javanna closed this in a038609 Aug 11, 2014

javanna added a commit that referenced this pull request Sep 8, 2014

Internal: refactored TransportSingleCustomOperationAction, subclasses…
… and requests

TransportSingleCustomOperationAction is subclassed by two similar, yet different transport action: TransportAnalyzeAction and TransportGetFieldMappingsAction. Made their difference and similarities more explicit by sharing common code and moving specific code to subclasses:
- moved index field to the parent SingleCustomOperationAction class
- moved the common check blocks code to the parent transport action class
- moved the main transport handler to the TransportAnalyzeAction subclass as it is only used to receive external requests through clients. In the case of the TransportGetFieldMappingsIndexAction instead, the action is internal and executed only locally as part of the user facing TransportGetFieldMappingsAction. The corresponding request gets sent over the transport though as part of the related shard request
- removed the get field mappings index action from the action names mapping as it is not a transport handler anymore. It was before although never used.

Closes #7214

@clintongormley clintongormley changed the title Internal: refactored TransportSingleCustomOperationAction, subclasses and requests Refactored TransportSingleCustomOperationAction, subclasses and requests Jun 7, 2015

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
You can’t perform that action at this time.