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

Does not work 'classpath*:' for Resource[] property on configuration properties since 2.x #15835

Closed
kazuki43zoo opened this issue Feb 2, 2019 · 13 comments
Assignees
Labels
type: regression A regression from a previous release
Milestone

Comments

@kazuki43zoo
Copy link
Contributor

kazuki43zoo commented Feb 2, 2019

I encountered a un-expected behavior on configuration properties feature since Spring Boot 2.x.
On 1.5.x, it work fine.

Conditions

  • Define a property of Resource[] on custom configuration properties class
  • Specify a property value using classpath*: prefix such as classpath*:files/*.txt

Expected result

  • Binding multiple Resource instance that matches pattern string

Actual result

  • Binding a single Resource that holds a specify pattern string such as classpath*:files/*.txt

Repro test

demo.zip

package com.example.demoresources;

import org.assertj.core.api.Assertions;
import org.junit.BeforeClass;
import org.junit.Test;
import org.junit.runner.RunWith;
import org.springframework.beans.factory.annotation.Autowired;
import org.springframework.boot.context.properties.ConfigurationProperties;
import org.springframework.boot.context.properties.EnableConfigurationProperties;
import org.springframework.boot.test.context.SpringBootTest;
import org.springframework.core.io.Resource;
import org.springframework.test.context.junit4.SpringRunner;

import java.io.IOException;
import java.nio.file.Files;
import java.nio.file.Path;
import java.nio.file.Paths;
import java.util.Arrays;
import java.util.List;
import java.util.stream.Collectors;

@RunWith(SpringRunner.class)
@SpringBootTest(properties = "my.files=classpath*:files/*.txt")
@EnableConfigurationProperties({DemoResourcesApplicationTests.MyProperties.class})
public class DemoResourcesApplicationTests {

  @Autowired
  MyProperties myProperties;

  @BeforeClass
  public static void setup() throws IOException {
    Path dir = Paths.get("target", "test-classes", "files");
    Files.createDirectories(dir);
    createFileIfNotExist(dir.resolve("a.txt"));
    createFileIfNotExist(dir.resolve("b.txt"));
  }

  private static void createFileIfNotExist(Path path) throws IOException {
    if (!path.toFile().exists()) {
      Files.createFile(path);
    }
  }

  @Test
  public void contextLoads() {
    Resource[] files = myProperties.getFiles();
    List<String> fileNames = Arrays.stream(files).map(Resource::getFilename).collect(Collectors.toList());
    Assertions.assertThat(fileNames)
        .hasSize(2)
        .contains("a.txt", "b.txt"); // Success with Spring Boot 1.5.x but fail with Spring Boot 2.x ...
  }

  @ConfigurationProperties(prefix = "my")
  public static class MyProperties {
    private Resource[] files = {};

    public void setFiles(Resource[] files) {
      this.files = files;
    }

    public Resource[] getFiles() {
      return files;
    }

  }

}
@wilkinsona
Copy link
Member

wilkinsona commented Feb 4, 2019

Thanks for the sample. There's some similarity to the problem described in #12166.

In 1.5, the conversion is handled by Framework's ResourceArrayPropertyEditor. In 2.x, the conversion is handled by Boot's DelimitedStringToArrayConverter. ResourceArrayPropertyEditor is registered with Boot's TypeConverterConversionService but TypeConverterConverter is only registered with a single convertible pair of java.lang.String -> java.lang.Object which does not match the java.lang.String -> [Lorg.springframework.core.io.Resource; convertible pair for the String to Resource[] conversion.

Here's an addition to ArrayBinderTests that reproduces the problem:

@Test
public void bindToResourceArrayShouldUsePropertyEditorAndPatternResolution() {
	MockConfigurationPropertySource source = new MockConfigurationPropertySource();
	source.put("foo", "classpath*:/**/*.class");
	this.sources.add(source);
	assertThat(this.binder.bind("foo", Bindable.of(Resource[].class)).get().length)
			.isGreaterThan(1);
}

Binding to a collection exhibits the same problem.

@wilkinsona wilkinsona added type: regression A regression from a previous release and removed status: waiting-for-triage An issue we've not yet triaged labels Feb 4, 2019
@wilkinsona wilkinsona added this to the 2.1.x milestone Feb 4, 2019
@wilkinsona wilkinsona added the status: pending-design-work Needs design work before any code can be developed label Feb 15, 2019
@mbhave mbhave self-assigned this Apr 2, 2019
@mbhave mbhave added the for: team-attention An issue we'd like other members of the team to review label Apr 2, 2019
@philwebb
Copy link
Member

philwebb commented Apr 3, 2019

I'm not totally sure that we should try and support pattern expansion in the same way as the ResourceEditor. I'm a little uneasy about performing such logic during the binding process. I wonder if binding the pattern as a String then using that later against a ResourcePatternResolver might be a better approach in most situations?

@philwebb philwebb removed the for: team-attention An issue we'd like other members of the team to review label Apr 26, 2019
@wilkinsona
Copy link
Member

We're going to deal with this on a case-by-case basis. We'll add only the resource array and resource collection property editors for now.

@mbhave
Copy link
Contributor

mbhave commented May 1, 2019

The Spring Framework @Value behavior is inconsistent for resource arrays vs collections. We should hold off on a change in Boot until we know what Framework might do.

@mbhave mbhave added the status: blocked An issue that's blocked on an external project change label May 1, 2019
@filiphr
Copy link
Contributor

filiphr commented Jul 1, 2019

Just to add some more info from #12993 (comment). When using Resource[] with @Value it works correctly.

For example doing:

@Value("${dummy.dummy-files}")
private Resource[] resources;

@wilkinsona
Copy link
Member

Thanks, @filiphr. We're already aware that Resource[] works with @Value. The Framework issue that Madhura opened notes this and aims to address the fact that it does not work with Collection<Resource>. We'd like that inconsistency to be addressed one way or another before deciding how to proceed in Boot.

@filiphr
Copy link
Contributor

filiphr commented Jul 1, 2019

Thanks for clearing it up @wilkinsona. I was only trying to bring some more information from the other issue (although I have no doubt that you are on top of it 😄).

@wilkinsona
Copy link
Member

The Framework issue has moved into the 5.x backlog so we're unlikely to be able to tackle this in 2.1.x or even 2.2.x.

@wilkinsona wilkinsona modified the milestones: 2.1.x, 2.x Aug 29, 2019
izeye added a commit to izeye/spring-boot-throwaway-branches that referenced this issue Nov 25, 2019
@LeoFuso
Copy link

LeoFuso commented Feb 26, 2021

Hello team, any news about this issue?

@snicoll
Copy link
Member

snicoll commented Feb 26, 2021

@LeoFuso nothing that wouldn't be available here. As noted above, we've opened a Spring Framework issue that still unresolved, see #15835 (comment)

@vermgit
Copy link

vermgit commented Jun 22, 2021

I think this is related and has a work around in the question.

@philwebb philwebb modified the milestones: 2.x, 3.x Aug 19, 2022
@wilkinsona
Copy link
Member

It looks like there's going to be a change in Framework 6.1. We should see what, if anything, we can do in Boot 3.2.

@wilkinsona wilkinsona modified the milestones: 3.x, 3.2.x Aug 2, 2023
@philwebb philwebb assigned wilkinsona and unassigned mbhave Sep 11, 2023
@wilkinsona
Copy link
Member

Framework now consistently converts a pattern to multiple resources whether it's a Resource[] or a Collection<Resource>. For consistency, we should do the same in Boot for binding. We'll need to figure out the best way to do that with the conversion service used by the binder.

@wilkinsona wilkinsona added for: team-meeting An issue we'd like to discuss as a team to make progress and removed status: blocked An issue that's blocked on an external project change labels Sep 12, 2023
@wilkinsona wilkinsona removed status: pending-design-work Needs design work before any code can be developed for: team-meeting An issue we'd like to discuss as a team to make progress labels Oct 16, 2023
@wilkinsona wilkinsona modified the milestones: 3.2.x, 3.2.0-RC1 Oct 16, 2023
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
Labels
type: regression A regression from a previous release
Projects
None yet
Development

No branches or pull requests

9 participants