Shard key not honored in ReferenceLookupDelegate when DocumentReference resolves to a empty collection

[stefanbildl]

Shard key not honored in ReferenceLookupDelegate

I have identified an issue in org.springframework.data.mongodb.core.convert.ReferenceLookupDelegate when using sharded mongodb collections.

The shard key of our "Task" collection includes the fields "datacenter" and "flowId":

@Getter
@Setter
@Document(collection = "task", language = "none")
@AllArgsConstructor
@Sharded(shardKey = {"datacenter", "flowId"}, immutableKey = true)
public class Task {
....
}

• We use a String field named "datacenter" that specifies where the data is stored (can be in one of our datacenters around the world)
• e.g. If data is in Mexico, datacenter is "MEX", if data is in Shanghai, datacenter is "SHA", etc.
• If we omit the datacenter field in any query, the query is run globally on every location (which is very slow)
• If a location is not reachable global queries fail. This leads to extended downtime in our case!

@Getter
@Setter
@Document(collection = "task", language = "none")
@AllArgsConstructor
@Sharded(shardKey = {"datacenter", "flowId"}, immutableKey = true)
public class Task {
...
	@DocumentReference(lookup = "{'flowId': ?#{flowId}, 'datacenter': ?#{datacenter}}")
	@Indexed
	private Set<Task> dependsOn = new LinkedHashSet<>();
}

If "dependsOn" is a empty set, the ReferenceLookupDelegate method computeFilter
returns

ListDocumentReferenceQuery(NO_RESULTS_PREDICATE,...)

which - in our case - causes a global query. If a datacenter is currently unreachable, the query fails!

	DocumentReferenceQuery computeFilter(MongoPersistentProperty property, Object source, SpELContext spELContext) {
...
			if (objects.isEmpty()) {
				return new ListDocumentReferenceQuery(NO_RESULTS_PREDICATE, sort); // here is the Problem NO_RESULTS_PREDICATE is a query without datacenter field
			}
...

I don't understand, why this NO_RESULTS_PREDICATE is needed here. Is this just a way to say: query for a empty list?

Proposition

Instead of handling the empty list case in computeFilter, just add the following lines to readReference:

	@Nullable
	public Object readReference(MongoPersistentProperty property, Object source, LookupFunction lookupFunction,
			MongoEntityReader entityReader) {

		Object value = source instanceof DocumentReferenceSource documentReferenceSource ? documentReferenceSource.getTargetSource()
				: source;
 
                 // THIS IS NEW
		 if (property.isCollectionLike() && value instanceof Collection && ((Collection)value).isEmpty()) {
                      return new ArrayList<>();
                 }
                 // END OF NEW STUFF

                DocumentReferenceQuery filter = computeFilter(property, source, spELContext);

.....

Thank you very much!

[stefanbildl]

Here is a hotfix for the problem:

Just use the following ReferenceResolver

    @Bean
    @Override
    public MappingMongoConverter mappingMongoConverter(MongoDatabaseFactory databaseFactory, MongoCustomConversions customConversions, MongoMappingContext mappingContext) {
        DbRefResolver dbRefResolver = new MyReferenceResolver(databaseFactory);
        MappingMongoConverter converter = new MappingMongoConverter(dbRefResolver, mappingContext);
        converter.setCustomConversions(customConversions);
        converter.setCodecRegistryProvider(databaseFactory);
        return converter;
    }



    private class MyReferenceResolver  extends DefaultDbRefResolver {
        public MyReferenceResolver(MongoDatabaseFactory mongoDbFactory) {
            super(mongoDbFactory);
        }

        @Override
        public Object resolveReference(MongoPersistentProperty property, Object source, ReferenceLookupDelegate referenceLookupDelegate, MongoEntityReader entityReader) {
            if(source instanceof DocumentReferenceSource s && s.getTargetSource() instanceof Collection<?> c && c.isEmpty())  {
                return new ArrayList<>();
            }
            return super.resolveReference(property, source, referenceLookupDelegate, entityReader);
        }
    }

[christophstrobl]

Thanks for reaching out and taking the time to provide a PR. We'll have a look.

[stefanbildl]

Hi, any news on this issue?
Thank you.

[christophstrobl]

@stefanbildl it's on the list - just not on the top of it.

[christophstrobl]

@stefanbildl we've slightly revised the proposed fix. You may want to have a look at the current issue branch.

[stefanbildl]

@christophstrobl thanks, looks good to me. Your refactor makes it way easier to understand!

[stefanbildl]

Oh, one thing I just noticed:

This is immutable: Collections.emptyList();

This could produce another bug where a user would try to add new entries to the empty list.
I intentionally used new ArrayList<>() for that purpose.

[stefanbildl]

@christophstrobl please review this, thanks!

[stefanbildl]

I think I get it now. the iterable is converted to the target type, am I right?

[christophstrobl]

@stefanbildl this is the way! yes.