From a4f974dcaacf656b3062c89a9050f9e67873021f Mon Sep 17 00:00:00 2001 From: Adrien Grand Date: Wed, 13 Aug 2014 10:58:59 +0200 Subject: [PATCH] Internal: Add some @Nullable annotations and fix related compilation warnings. Added @Nullable to: - IndicesService.indexService - IndexService.shard - IndexService.shardInjector This change doesn't try to do anything smart but just makes sure that a *MissingException is thrown instead of a NullPointerException when the requested object doesn't exist. Close #7251 --- .../explain/TransportExplainAction.java | 2 +- .../action/update/TransportUpdateAction.java | 5 +- .../metadata/MetaDataMappingService.java | 48 +++++++++---------- .../index/service/IndexService.java | 15 ++++++ .../elasticsearch/indices/IndicesService.java | 2 + .../cluster/IndicesClusterStateService.java | 11 ++--- .../TransportNodesListShardStoreMetaData.java | 4 +- .../indices/store/SimpleDistributorTests.java | 2 +- 8 files changed, 51 insertions(+), 38 deletions(-) diff --git a/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java b/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java index c6c539d5c3b..62d9ae29291 100644 --- a/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java +++ b/src/main/java/org/elasticsearch/action/explain/TransportExplainAction.java @@ -108,7 +108,7 @@ public class TransportExplainAction extends TransportShardSingleOperationAction< @Override protected ExplainResponse shardOperation(ExplainRequest request, ShardId shardId) throws ElasticsearchException { - IndexService indexService = indicesService.indexService(shardId.getIndex()); + IndexService indexService = indicesService.indexServiceSafe(shardId.getIndex()); IndexShard indexShard = indexService.shardSafe(shardId.id()); Term uidTerm = new Term(UidFieldMapper.NAME, Uid.createUidAsBytes(request.type(), request.id())); Engine.GetResult result = indexShard.get(new Engine.Get(false, uidTerm)); diff --git a/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java b/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java index f1395f484ec..9d9d7d2f7b9 100644 --- a/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java +++ b/src/main/java/org/elasticsearch/action/update/TransportUpdateAction.java @@ -273,7 +273,10 @@ public class TransportUpdateAction extends TransportInstanceSingleOperationActio listener.onResponse(update); IndexService indexServiceOrNull = indicesService.indexService(request.concreteIndex()); if (indexServiceOrNull != null) { - indexService.shard(request.request().shardId()).indexingService().noopUpdate(request.request().type()); + IndexShard shard = indexService.shard(request.request().shardId()); + if (shard != null) { + shard.indexingService().noopUpdate(request.request().type()); + } } break; default: diff --git a/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java b/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java index 1a1aac47525..b76e2e7c49e 100644 --- a/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java +++ b/src/main/java/org/elasticsearch/cluster/metadata/MetaDataMappingService.java @@ -25,7 +25,10 @@ import com.google.common.collect.Sets; import org.elasticsearch.action.ActionListener; import org.elasticsearch.action.admin.indices.mapping.delete.DeleteMappingClusterStateUpdateRequest; import org.elasticsearch.action.admin.indices.mapping.put.PutMappingClusterStateUpdateRequest; -import org.elasticsearch.cluster.*; +import org.elasticsearch.cluster.AckedClusterStateUpdateTask; +import org.elasticsearch.cluster.ClusterService; +import org.elasticsearch.cluster.ClusterState; +import org.elasticsearch.cluster.ProcessedClusterStateUpdateTask; import org.elasticsearch.cluster.ack.ClusterStateUpdateResponse; import org.elasticsearch.common.Priority; import org.elasticsearch.common.collect.Tuple; @@ -491,33 +494,28 @@ public class MetaDataMappingService extends AbstractComponent { Map newMappers = newHashMap(); Map existingMappers = newHashMap(); for (String index : request.indices()) { - IndexService indexService = indicesService.indexService(index); - if (indexService != null) { - // try and parse it (no need to add it here) so we can bail early in case of parsing exception - DocumentMapper newMapper; - DocumentMapper existingMapper = indexService.mapperService().documentMapper(request.type()); - if (MapperService.DEFAULT_MAPPING.equals(request.type())) { - // _default_ types do not go through merging, but we do test the new settings. Also don't apply the old default - newMapper = indexService.mapperService().parse(request.type(), new CompressedString(request.source()), false); - } else { - newMapper = indexService.mapperService().parse(request.type(), new CompressedString(request.source())); - if (existingMapper != null) { - // first, simulate - DocumentMapper.MergeResult mergeResult = existingMapper.merge(newMapper, mergeFlags().simulate(true)); - // if we have conflicts, and we are not supposed to ignore them, throw an exception - if (!request.ignoreConflicts() && mergeResult.hasConflicts()) { - throw new MergeMappingException(mergeResult.conflicts()); - } + IndexService indexService = indicesService.indexServiceSafe(index); + // try and parse it (no need to add it here) so we can bail early in case of parsing exception + DocumentMapper newMapper; + DocumentMapper existingMapper = indexService.mapperService().documentMapper(request.type()); + if (MapperService.DEFAULT_MAPPING.equals(request.type())) { + // _default_ types do not go through merging, but we do test the new settings. Also don't apply the old default + newMapper = indexService.mapperService().parse(request.type(), new CompressedString(request.source()), false); + } else { + newMapper = indexService.mapperService().parse(request.type(), new CompressedString(request.source())); + if (existingMapper != null) { + // first, simulate + DocumentMapper.MergeResult mergeResult = existingMapper.merge(newMapper, mergeFlags().simulate(true)); + // if we have conflicts, and we are not supposed to ignore them, throw an exception + if (!request.ignoreConflicts() && mergeResult.hasConflicts()) { + throw new MergeMappingException(mergeResult.conflicts()); } } + } - newMappers.put(index, newMapper); - if (existingMapper != null) { - existingMappers.put(index, existingMapper); - } - - } else { - throw new IndexMissingException(new Index(index)); + newMappers.put(index, newMapper); + if (existingMapper != null) { + existingMappers.put(index, existingMapper); } } diff --git a/src/main/java/org/elasticsearch/index/service/IndexService.java b/src/main/java/org/elasticsearch/index/service/IndexService.java index 1c0830531f5..d43a21f6b0b 100644 --- a/src/main/java/org/elasticsearch/index/service/IndexService.java +++ b/src/main/java/org/elasticsearch/index/service/IndexService.java @@ -21,6 +21,7 @@ package org.elasticsearch.index.service; import com.google.common.collect.ImmutableSet; import org.elasticsearch.ElasticsearchException; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.inject.Injector; import org.elasticsearch.index.IndexComponent; import org.elasticsearch.index.IndexShardMissingException; @@ -79,12 +80,26 @@ public interface IndexService extends IndexComponent, Iterable { boolean hasShard(int shardId); + /** + * Return the shard with the provided id, or null if there is no such shard. + */ + @Nullable IndexShard shard(int shardId); + /** + * Return the shard with the provided id, or throw an exception if it doesn't exist. + */ IndexShard shardSafe(int shardId) throws IndexShardMissingException; + /** + * Return the shard injector for the provided id, or null if there is no such shard. + */ + @Nullable Injector shardInjector(int shardId); + /** + * Return the shard injector for the provided id, or throw an exception if there is no such shard. + */ Injector shardInjectorSafe(int shardId) throws IndexShardMissingException; String indexUUID(); diff --git a/src/main/java/org/elasticsearch/indices/IndicesService.java b/src/main/java/org/elasticsearch/indices/IndicesService.java index f8edce8e294..955519d2351 100644 --- a/src/main/java/org/elasticsearch/indices/IndicesService.java +++ b/src/main/java/org/elasticsearch/indices/IndicesService.java @@ -22,6 +22,7 @@ package org.elasticsearch.indices; import com.google.common.collect.ImmutableMap; import org.elasticsearch.ElasticsearchException; import org.elasticsearch.action.admin.indices.stats.CommonStatsFlags; +import org.elasticsearch.common.Nullable; import org.elasticsearch.common.component.LifecycleComponent; import org.elasticsearch.common.settings.Settings; import org.elasticsearch.index.service.IndexService; @@ -62,6 +63,7 @@ public interface IndicesService extends Iterable, LifecycleCompone * Even if the index name appeared in {@link #indices()} null can still be returned as an * index maybe removed in the meantime, so preferable use the associated {@link IndexService} in order to prevent NPE. */ + @Nullable IndexService indexService(String index); /** diff --git a/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java b/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java index a71290a3241..cb089f624a0 100644 --- a/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java +++ b/src/main/java/org/elasticsearch/indices/cluster/IndicesClusterStateService.java @@ -542,8 +542,8 @@ public class IndicesClusterStateService extends AbstractLifecycleComponent nodes = internalCluster().nodesInclude("test"); assertThat(nodes.isEmpty(), equalTo(false)); IndicesService indicesService = internalCluster().getInstance(IndicesService.class, nodes.iterator().next()); - InternalIndexShard indexShard = (InternalIndexShard) (indicesService.indexService(index).shard(shardId)); + InternalIndexShard indexShard = (InternalIndexShard) (indicesService.indexService(index).shardSafe(shardId)); return indexShard.store().directory(); } }