Skip to content

Commit

Permalink
Retrieve app version without downloading
Browse files Browse the repository at this point in the history
 - Instead of invoking `getResource` via the underlying LRUCleaningResourceLoader, construct the resource based on the schemes used in the URI
  - For maven resource, parse the co-ordinates and construct MavenResource with the appropriate MavenProperties (configured in the server)
   - Make sure to inject the maven properties

 - Remove `getResource` from AppRegistryService as we no longer need to download the resource at the registry
 - Use DefaultResourceLoader in AppRegistry for the use case where the `--uri` option is used to download the http resource when `registerAll`
 - The delegating resource loader is now used only when downloading the metadata resource at the AppRegistry

 - Add tests for resource version retrieval

Resolves spring-cloud#1843
  • Loading branch information
ilayaperumalg committed Dec 28, 2017
1 parent b34e35f commit 4fd7bfd
Show file tree
Hide file tree
Showing 9 changed files with 128 additions and 44 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,6 +17,7 @@
import org.springframework.cloud.dataflow.core.ApplicationType;
import org.springframework.cloud.dataflow.registry.domain.AppRegistration;
import org.springframework.cloud.dataflow.registry.support.ResourceUtils;
import org.springframework.cloud.deployer.resource.maven.MavenProperties;
import org.springframework.core.io.Resource;
import org.springframework.core.io.ResourceLoader;
import org.springframework.core.io.support.PropertiesLoaderUtils;
Expand All @@ -27,26 +28,30 @@
* {@link AppRegistryCommon} implementation common for the Classic and the Skipper modes.
*
* @author Christian Tzolov
* @author Ilayaperumal Gopinathan
*/
public abstract class AbstractAppRegistryCommon implements AppRegistryCommon {

private static final Logger logger = LoggerFactory.getLogger(AbstractAppRegistryCommon.class);

public static final String METADATA_KEY_SUFFIX = "metadata";

private ResourceLoader resourceLoader;
protected ResourceLoader resourceLoader;

protected MavenProperties mavenProperties;

public AbstractAppRegistryCommon(ResourceLoader resourceLoader) {
this.resourceLoader = resourceLoader;
}

public ResourceLoader getResourceLoader() {
return this.resourceLoader;
public AbstractAppRegistryCommon(ResourceLoader resourceLoader, MavenProperties mavenProperties) {
this.resourceLoader = resourceLoader;
this.mavenProperties = mavenProperties;
}

@Override
public Resource getAppResource(AppRegistration appRegistration) {
return this.resourceLoader.getResource(appRegistration.getUri().toString());
return ResourceUtils.getResource(appRegistration.getUri().toString(), this.mavenProperties);
}

@Override
Expand Down Expand Up @@ -121,9 +126,7 @@ protected Stream<AppRegistration> toValidAppRegistration(Map.Entry<String, URI>

private String getVersionOrBroken(String uri) {
try {
Resource appResource = resourceLoader.getResource(uri);

return ResourceUtils.getResourceVersion(appResource);
return ResourceUtils.getResourceVersion(uri, this.mavenProperties);
}
catch (IllegalStateException ise) {
logger.warn("", ise);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -31,6 +31,7 @@
import org.springframework.cloud.dataflow.core.ApplicationType;
import org.springframework.cloud.dataflow.registry.domain.AppRegistration;
import org.springframework.cloud.dataflow.registry.support.NoSuchAppRegistrationException;
import org.springframework.cloud.deployer.resource.maven.MavenProperties;
import org.springframework.cloud.deployer.resource.registry.UriRegistry;
import org.springframework.core.io.Resource;
import org.springframework.core.io.ResourceLoader;
Expand Down Expand Up @@ -66,8 +67,6 @@ public class AppRegistry extends AbstractAppRegistryCommon implements AppRegistr

private final UriRegistry uriRegistry;

private final ResourceLoader resourceLoader;

private static final Function<Map.Entry<Object, Object>, AbstractMap.SimpleImmutableEntry<String, URI>> toStringAndUriFUNC = kv -> {
try {
return new AbstractMap.SimpleImmutableEntry<>((String) kv.getKey(), new URI((String) kv.getValue()));
Expand All @@ -80,9 +79,14 @@ public class AppRegistry extends AbstractAppRegistryCommon implements AppRegistr
public AppRegistry(UriRegistry uriRegistry, ResourceLoader resourceLoader) {
super(resourceLoader);
Assert.notNull(uriRegistry, "'uriRegistry' must not be null");
this.uriRegistry = uriRegistry;
}

public AppRegistry(UriRegistry uriRegistry, ResourceLoader resourceLoader, MavenProperties mavenProperties) {
super(resourceLoader, mavenProperties);
Assert.notNull(uriRegistry, "'uriRegistry' must not be null");
Assert.notNull(resourceLoader, "'resourceLoader' must not be null");
this.uriRegistry = uriRegistry;
this.resourceLoader = resourceLoader;
}

public AppRegistration find(String name, ApplicationType type) {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -5,7 +5,6 @@
import org.springframework.cloud.dataflow.core.ApplicationType;
import org.springframework.cloud.dataflow.registry.AppRegistryCommon;
import org.springframework.cloud.dataflow.registry.domain.AppRegistration;
import org.springframework.core.io.Resource;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.Pageable;

Expand Down Expand Up @@ -37,8 +36,6 @@ public interface AppRegistryService extends AppRegistryCommon {

void delete(String name, ApplicationType type, String version);

Resource getResource(String uri);

boolean appExist(String name, ApplicationType type, String version);

Page<AppRegistration> findAllByTypeAndNameIsLike(ApplicationType type, String name, Pageable pageable);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -27,6 +27,7 @@
import org.springframework.cloud.dataflow.registry.domain.AppRegistration;
import org.springframework.cloud.dataflow.registry.repository.AppRegistrationRepository;
import org.springframework.cloud.dataflow.registry.support.NoSuchAppRegistrationException;
import org.springframework.cloud.deployer.resource.maven.MavenProperties;
import org.springframework.core.io.Resource;
import org.springframework.core.io.ResourceLoader;
import org.springframework.data.domain.Page;
Expand Down Expand Up @@ -64,18 +65,13 @@ public class DefaultAppRegistryService extends AbstractAppRegistryCommon impleme
private final AppRegistrationRepository appRegistrationRepository;

public DefaultAppRegistryService(AppRegistrationRepository appRegistrationRepository,
ResourceLoader resourceLoader) {
super(resourceLoader);
ResourceLoader resourceLoader, MavenProperties mavenProperties) {
super(resourceLoader, mavenProperties);
Assert.notNull(appRegistrationRepository, "'appRegistrationRepository' must not be null");
Assert.notNull(resourceLoader, "'resourceLoader' must not be null");
this.appRegistrationRepository = appRegistrationRepository;
}

@Override
public Resource getResource(String uri) {
return (uri == null) ? null : this.getResourceLoader().getResource(uri);
}

@Override
public AppRegistration find(String name, ApplicationType type) {
return this.getDefaultApp(name, type);
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -18,19 +18,25 @@
import java.io.File;
import java.io.IOException;
import java.net.URI;
import java.net.URISyntaxException;
import java.util.function.BiFunction;
import java.util.regex.Matcher;
import java.util.regex.Pattern;

import org.springframework.cloud.deployer.resource.docker.DockerResource;
import org.springframework.cloud.deployer.resource.maven.MavenProperties;
import org.springframework.cloud.deployer.resource.maven.MavenResource;
import org.springframework.cloud.deployer.resource.support.DownloadingUrlResourceLoader;
import org.springframework.core.io.DefaultResourceLoader;
import org.springframework.core.io.Resource;
import org.springframework.core.io.ResourceLoader;
import org.springframework.core.io.UrlResource;
import org.springframework.util.Assert;
import org.springframework.util.StringUtils;

/**
* @author Mark Pollack
* @author Ilayaperumal Gopinathan
*/
public class ResourceUtils {

Expand Down Expand Up @@ -86,6 +92,44 @@ private static URI getUri(UrlResource urlResource) {
return uri;
}

/**
* Retrieve the corresponding {@link Resource} instance based on the URI String.
* Maven properties are used if the URI corresponds to maven resource.
*
* @param uriString String representation of the resource URI
* @param mavenProperties the maven properties to use in case of maven resource
* @return the resource instance
*/
public static Resource getResource(String uriString, MavenProperties mavenProperties) {
Assert.isTrue(StringUtils.hasText(uriString), "Resource URI must not be empty");
try {
URI uri = new URI(uriString);
String scheme = uri.getScheme();
Assert.notNull(scheme, "a scheme (prefix) is required");
if (scheme.equals("maven")) {
String coordinates = uriString.replaceFirst("maven:\\/*", "");
MavenResource mavenResource = MavenResource.parse(coordinates, mavenProperties);
return mavenResource;
}
else if (scheme.equals("docker")) {
return new DockerResource(uriString);
}
else {
ResourceLoader resourceLoader = null;
if (!scheme.equalsIgnoreCase("http") && !scheme.equalsIgnoreCase("https")) {
resourceLoader = new DefaultResourceLoader();
}
else {
resourceLoader = new DownloadingUrlResourceLoader();
}
return resourceLoader.getResource(uriString);
}
}
catch (URISyntaxException e) {
throw new RuntimeException(e);
}
}

/**
* Extracts the version from the resource. Supported resource types are {@link
* MavenResource}, {@link DockerResource}, and {@link UrlResource}. @param resource to be
Expand All @@ -110,6 +154,17 @@ else if (resource instanceof UrlResource) {
}
}

/**
* Returns the version for the given resource URI string.
*
* @param uriString String representation of the resource URI
* @param mavenProperties the maven properties to use in case of maven resource
* @return the resource version
*/
public static String getResourceVersion(String uriString, MavenProperties mavenProperties) {
return ResourceUtils.getResourceVersion(getResource(uriString, mavenProperties));
}

private static String formatDockerResource(DockerResource dockerResource,
BiFunction<String, Integer, String> function) {
try {
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,14 +15,18 @@
*/
package org.springframework.cloud.dataflow.registry.support;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;

import java.net.MalformedURLException;

import org.junit.Test;

import org.springframework.cloud.deployer.resource.maven.MavenProperties;
import org.springframework.cloud.deployer.resource.maven.MavenResource;
import org.springframework.core.io.Resource;
import org.springframework.core.io.UrlResource;

import static org.assertj.core.api.Assertions.assertThat;
import static org.assertj.core.api.Assertions.fail;

/**
* @author Mark Pollack
* @author Ilayaperumal Gopinathan
Expand Down Expand Up @@ -115,6 +119,19 @@ public void testJars() throws MalformedURLException {
assertThat(version).isEqualTo("1.2.0-SNAPSHOT");
theRest = ResourceUtils.getResourceWithoutVersion(urlResource);
assertThat(theRest).isEqualTo("http://repo.spring.io/libs-release/org/springframework/cloud/stream/app/file-sink-rabbit/file-sink-rabbit");
}

@Test
public void testGetResource() {
String mavenUri = "maven://org.springframework.cloud.stream.app:aggregate-counter-sink-rabbit:1.3.0.RELEASE";
Resource resource = ResourceUtils.getResource(mavenUri, new MavenProperties());
assertThat(resource).isInstanceOf(MavenResource.class);
}

@Test
public void testGetResourceVersion() {
String mavenUri = "maven://org.springframework.cloud.stream.app:aggregate-counter-sink-rabbit:1.3.0.RELEASE";
String version = ResourceUtils.getResourceVersion(ResourceUtils.getResource(mavenUri, new MavenProperties()));
assertThat(version).isEqualTo("1.3.0.RELEASE");
}
}
Original file line number Diff line number Diff line change
Expand Up @@ -147,24 +147,25 @@ public UriRegistry uriRegistry(DataSource dataSource) {
@Bean
@ConditionalOnExpression("#{'${" + FeaturesProperties.FEATURES_PREFIX + "." + FeaturesProperties.SKIPPER_ENABLED
+ ":false}'.equalsIgnoreCase('false')}")
public AppRegistry appRegistry(UriRegistry uriRegistry, DelegatingResourceLoader resourceLoader) {
return new AppRegistry(uriRegistry, resourceLoader);
public AppRegistry appRegistry(UriRegistry uriRegistry, DelegatingResourceLoader resourceLoader,
MavenProperties mavenProperties) {
return new AppRegistry(uriRegistry, resourceLoader, mavenProperties);
}

@Bean
@ConditionalOnExpression("#{'${" + FeaturesProperties.FEATURES_PREFIX + "." + FeaturesProperties.SKIPPER_ENABLED
+ ":false}'.equalsIgnoreCase('true')}")
public AppRegistryService appRegistryService(AppRegistrationRepository appRegistrationRepository,
DelegatingResourceLoader resourceLoader) {
return new DefaultAppRegistryService(appRegistrationRepository, resourceLoader);
return new DefaultAppRegistryService(appRegistrationRepository, resourceLoader, mavenProperties());
}

@Bean
@ConditionalOnExpression("#{'${" + FeaturesProperties.FEATURES_PREFIX + "." + FeaturesProperties.SKIPPER_ENABLED
+ ":false}'.equalsIgnoreCase('true')}")
public VersionedAppRegistryController appRegistryController2(AppRegistryService appRegistry,
ApplicationConfigurationMetadataResolver metadataResolver) {
return new VersionedAppRegistryController(appRegistry, metadataResolver, appRegistryFJPFB().getObject());
return new VersionedAppRegistryController(appRegistry, metadataResolver, appRegistryFJPFB().getObject(), mavenProperties());
}

@Bean
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -40,8 +40,10 @@
import org.springframework.cloud.dataflow.registry.support.ResourceUtils;
import org.springframework.cloud.dataflow.rest.resource.AppRegistrationResource;
import org.springframework.cloud.dataflow.rest.resource.DetailedAppRegistrationResource;
import org.springframework.cloud.deployer.resource.maven.MavenProperties;
import org.springframework.core.io.ByteArrayResource;
import org.springframework.core.io.Resource;
import org.springframework.core.io.DefaultResourceLoader;
import org.springframework.core.io.ResourceLoader;
import org.springframework.data.domain.Page;
import org.springframework.data.domain.PageImpl;
import org.springframework.data.domain.Pageable;
Expand Down Expand Up @@ -85,12 +87,17 @@ public class VersionedAppRegistryController {

private ForkJoinPool forkJoinPool;

private final MavenProperties mavenProperties;

private ResourceLoader resourceLoader = new DefaultResourceLoader();

public VersionedAppRegistryController(AppRegistryService appRegistryService,
ApplicationConfigurationMetadataResolver metadataResolver,
ForkJoinPool forkJoinPool) {
ForkJoinPool forkJoinPool, MavenProperties mavenProperties) {
this.appRegistryService = appRegistryService;
this.metadataResolver = metadataResolver;
this.forkJoinPool = forkJoinPool;
this.mavenProperties = mavenProperties;
}

/**
Expand Down Expand Up @@ -193,9 +200,7 @@ public void register(@PathVariable("type") ApplicationType type, @PathVariable("
public void register(@PathVariable("type") ApplicationType type, @PathVariable("name") String name,
@RequestParam("uri") String uri, @RequestParam(name = "metadata-uri", required = false) String metadataUri,
@RequestParam(value = "force", defaultValue = "false") boolean force) {

Resource resource = this.appRegistryService.getResource(uri);
String version = ResourceUtils.getResourceVersion(resource);
String version = ResourceUtils.getResourceVersion(uri, this.mavenProperties);
this.register(type, name, version, uri, metadataUri, force);
}

Expand Down Expand Up @@ -263,7 +268,7 @@ public PagedResources<? extends AppRegistrationResource> registerAll(
List<AppRegistration> registrations = new ArrayList<>();

if (StringUtils.hasText(uri)) {
registrations.addAll(appRegistryService.importAll(force, this.appRegistryService.getResource(uri)));
registrations.addAll(appRegistryService.importAll(force, this.resourceLoader.getResource(uri)));
}
else if (!CollectionUtils.isEmpty(apps)) {
ByteArrayOutputStream baos = new ByteArrayOutputStream();
Expand Down
Loading

0 comments on commit 4fd7bfd

Please sign in to comment.