From 47c954141de2eb1a237d7d024e99bd8bb64857b2 Mon Sep 17 00:00:00 2001 From: Ray Milkey Date: Fri, 15 Sep 2017 10:40:48 -0700 Subject: [PATCH] Refactor construction of application objects - Use a builder and make the constructors private for DefaultApplication - Make DefaultApplication immutable - Use a builder and make the constructors private for DefaultApplicationDescription - Make DefaultApplicationDescription immutable Change-Id: I9499981bd2c0f48aede40682260d51eeae2cab98 --- .../app/DefaultApplicationDescription.java | 271 +++++++++++-- .../onosproject/core/DefaultApplication.java | 358 +++++++++++++++++- .../onosproject/app/ApplicationEventTest.java | 21 +- .../DefaultApplicationDescriptionTest.java | 31 +- .../core/DefaultApplicationTest.java | 82 ++-- .../common/app/ApplicationArchive.java | 20 +- .../store/trivial/SimpleApplicationStore.java | 38 +- .../onosproject/utils/ComparatorsTest.java | 19 +- .../app/impl/ApplicationManagerTest.java | 22 +- .../impl/SecurityModeManagerTest.java | 21 +- .../DistributedSecurityModeStoreTest.java | 21 +- .../app/DistributedApplicationStore.java | 18 +- .../core/ApplicationProtoTranslator.java | 22 +- .../resources/ApplicationsResourceTest.java | 65 +++- 14 files changed, 833 insertions(+), 176 deletions(-) diff --git a/core/api/src/main/java/org/onosproject/app/DefaultApplicationDescription.java b/core/api/src/main/java/org/onosproject/app/DefaultApplicationDescription.java index 2e0988b777..38627185b1 100644 --- a/core/api/src/main/java/org/onosproject/app/DefaultApplicationDescription.java +++ b/core/api/src/main/java/org/onosproject/app/DefaultApplicationDescription.java @@ -15,15 +15,17 @@ */ package org.onosproject.app; -import org.onosproject.core.ApplicationRole; -import org.onosproject.core.Version; -import org.onosproject.security.Permission; - import java.net.URI; import java.util.List; import java.util.Optional; import java.util.Set; +import org.onosproject.core.ApplicationRole; +import org.onosproject.core.Version; +import org.onosproject.security.Permission; + +import com.google.common.collect.ImmutableList; + import static com.google.common.base.MoreObjects.toStringHelper; import static com.google.common.base.Preconditions.checkArgument; import static com.google.common.base.Preconditions.checkNotNull; @@ -31,7 +33,7 @@ import static com.google.common.base.Preconditions.checkNotNull; /** * Default implementation of network control/management application descriptor. */ -public class DefaultApplicationDescription implements ApplicationDescription { +public final class DefaultApplicationDescription implements ApplicationDescription { private final String name; private final Version version; @@ -48,6 +50,14 @@ public class DefaultApplicationDescription implements ApplicationDescription { private final List features; private final List requiredApps; + /** + * Default constructor is hidden to prevent calls to new. + */ + private DefaultApplicationDescription() { + // Should not happen + throw new UnsupportedOperationException(); + } + /** * Creates a new application descriptor using the supplied data. * @@ -66,27 +76,26 @@ public class DefaultApplicationDescription implements ApplicationDescription { * @param features application features * @param requiredApps list of required application names */ - public DefaultApplicationDescription(String name, Version version, String title, - String description, String origin, String category, - String url, String readme, byte[] icon, - ApplicationRole role, Set permissions, - URI featuresRepo, List features, - List requiredApps) { - this.name = checkNotNull(name, "Name cannot be null"); - this.version = checkNotNull(version, "Version cannot be null"); - this.title = checkNotNull(title, "Title cannot be null"); - this.description = checkNotNull(description, "Description cannot be null"); - this.origin = checkNotNull(origin, "Origin cannot be null"); - this.category = checkNotNull(category, "Category cannot be null"); + private DefaultApplicationDescription(String name, Version version, String title, + String description, String origin, String category, + String url, String readme, byte[] icon, + ApplicationRole role, Set permissions, + URI featuresRepo, List features, + List requiredApps) { + this.name = name; + this.version = version; + this.title = title; + this.description = description; + this.origin = origin; + this.category = category; this.url = url; - this.readme = checkNotNull(readme, "Readme cannot be null"); + this.readme = readme; this.icon = icon; - this.role = checkNotNull(role, "Role cannot be null"); - this.permissions = checkNotNull(permissions, "Permissions cannot be null"); + this.role = role; + this.permissions = permissions; this.featuresRepo = Optional.ofNullable(featuresRepo); - this.features = checkNotNull(features, "Features cannot be null"); - this.requiredApps = checkNotNull(requiredApps, "Required apps cannot be null"); - checkArgument(!features.isEmpty(), "There must be at least one feature"); + this.features = ImmutableList.copyOf(features); + this.requiredApps = ImmutableList.copyOf(requiredApps); } @Override @@ -177,4 +186,220 @@ public class DefaultApplicationDescription implements ApplicationDescription { .add("requiredApps", requiredApps) .toString(); } + + /** + * Returns a default application description builder. + * + * @return builder + */ + public static Builder builder() { + return new Builder(); + } + + /** + * Default application description builder. + */ + public static final class Builder { + + private String name; + private Version version; + private String title; + private String description; + private String category; + private String url; + private String readme; + private byte[] icon; + private String origin; + private ApplicationRole role; + private Set permissions; + private URI featuresRepo; + private List features; + private List requiredApps; + + /** + * Default constructor for the builder. + */ + public Builder() {} + + /** + * Adds an application id. + * + * @param name application name + * @return builder + */ + public Builder withName(String name) { + this.name = name; + return this; + } + + /** + * Adds a version string. + * + * @param version version string + * @return builder + */ + public Builder withVersion(Version version) { + this.version = version; + return this; + } + + /** + * Adds a title string. + * + * @param title title string + * @return builder + */ + public Builder withTitle(String title) { + this.title = title; + return this; + } + + /** + * Adds a description string. + * + * @param description description string + * @return builder + */ + public Builder withDescription(String description) { + this.description = description; + return this; + } + + /** + * Adds a category string. + * + * @param category category string + * @return builder + */ + public Builder withCategory(String category) { + this.category = category; + return this; + } + + /** + * Adds a URL string. + * + * @param url url string + * @return builder + */ + public Builder withUrl(String url) { + this.url = url; + return this; + } + + /** + * Adds a readme string. + * + * @param readme readme string + * @return builder + */ + public Builder withReadme(String readme) { + this.readme = readme; + return this; + } + + /** + * Adds an icon. + * + * @param icon icon data + * @return builder + */ + public Builder withIcon(byte[] icon) { + this.icon = icon; + return this; + } + + /** + * Adds an origin string. + * + * @param origin origin string + * @return builder + */ + public Builder withOrigin(String origin) { + this.origin = origin; + return this; + } + + /** + * Adds an application role. + * + * @param role application role + * @return builder + */ + public Builder withRole(ApplicationRole role) { + this.role = role; + return this; + } + + /** + * Adds a permissions set. + * + * @param permissions permissions set + * @return builder + */ + public Builder withPermissions(Set permissions) { + this.permissions = permissions; + return this; + } + + /** + * Adds a URI for a features repository. + * + * @param featuresRepo Optional URI for a features repository + * @return builder + */ + public Builder withFeaturesRepo(URI featuresRepo) { + this.featuresRepo = featuresRepo; + return this; + } + + /** + * Adds a features list. + * + * @param features features list + * @return builder + */ + public Builder withFeatures(List features) { + this.features = features; + return this; + } + + /** + * Adds a list of required applications. + * + * @param requiredApps List of name strings of required applications + * @return builder + */ + public Builder withRequiredApps(List requiredApps) { + this.requiredApps = requiredApps; + return this; + } + + /** + * Builds a default application object from the gathered parameters. + * + * @return new default application + */ + public DefaultApplicationDescription build() { + checkNotNull(name, "Name cannot be null"); + checkNotNull(version, "Version cannot be null"); + checkNotNull(title, "Title cannot be null"); + checkNotNull(description, "Description cannot be null"); + checkNotNull(origin, "Origin cannot be null"); + checkNotNull(category, "Category cannot be null"); + checkNotNull(readme, "Readme cannot be null"); + checkNotNull(role, "Role cannot be null"); + checkNotNull(permissions, "Permissions cannot be null"); + checkNotNull(features, "Features cannot be null"); + checkNotNull(requiredApps, "Required apps cannot be null"); + checkArgument(!features.isEmpty(), "There must be at least one feature"); + + return new DefaultApplicationDescription(name, version, title, + description, origin, category, + url, readme, icon, + role, permissions, + featuresRepo, features, + requiredApps); + } + } } diff --git a/core/api/src/main/java/org/onosproject/core/DefaultApplication.java b/core/api/src/main/java/org/onosproject/core/DefaultApplication.java index f78062325e..1eb83b4b2a 100644 --- a/core/api/src/main/java/org/onosproject/core/DefaultApplication.java +++ b/core/api/src/main/java/org/onosproject/core/DefaultApplication.java @@ -17,6 +17,8 @@ package org.onosproject.core; import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; + +import org.onosproject.app.ApplicationDescription; import org.onosproject.security.Permission; import java.net.URI; @@ -32,7 +34,7 @@ import static com.google.common.base.Preconditions.checkNotNull; /** * Default implementation of network control/management application descriptor. */ -public class DefaultApplication implements Application { +public final class DefaultApplication implements Application { private final ApplicationId appId; private final Version version; @@ -49,6 +51,14 @@ public class DefaultApplication implements Application { private final List features; private final List requiredApps; + /** + * Default constructor is hidden to prevent calls to new. + */ + private DefaultApplication() { + // should never happen + throw new UnsupportedOperationException(); + } + /** * Creates a new application descriptor using the supplied data. * @@ -67,33 +77,26 @@ public class DefaultApplication implements Application { * @param features application features * @param requiredApps list of required application names */ - public DefaultApplication(ApplicationId appId, Version version, String title, + private DefaultApplication(ApplicationId appId, Version version, String title, String description, String origin, String category, String url, String readme, byte[] icon, ApplicationRole role, Set permissions, Optional featuresRepo, List features, List requiredApps) { - this.appId = checkNotNull(appId, "ID cannot be null"); - this.version = checkNotNull(version, "Version cannot be null"); - this.title = checkNotNull(title, "Title cannot be null"); - this.description = checkNotNull(description, "Description cannot be null"); - this.origin = checkNotNull(origin, "Origin cannot be null"); - this.category = checkNotNull(category, "Category cannot be null"); + this.appId = appId; + this.version = version; + this.title = title; + this.description = description; + this.origin = origin; + this.category = category; this.url = url; - this.readme = checkNotNull(readme, "Readme cannot be null"); + this.readme = readme; this.icon = icon == null ? new byte[0] : icon.clone(); - this.role = checkNotNull(role, "Role cannot be null"); - this.permissions = ImmutableSet.copyOf( - checkNotNull(permissions, "Permissions cannot be null") - ); - this.featuresRepo = checkNotNull(featuresRepo, "Features repo cannot be null"); - this.features = ImmutableList.copyOf( - checkNotNull(features, "Features cannot be null") - ); - this.requiredApps = ImmutableList.copyOf( - checkNotNull(requiredApps, "Required apps cannot be null") - ); - checkArgument(!features.isEmpty(), "There must be at least one feature"); + this.role = role; + this.permissions = ImmutableSet.copyOf(permissions); + this.featuresRepo = featuresRepo; + this.features = ImmutableList.copyOf(features); + this.requiredApps = ImmutableList.copyOf(requiredApps); } @Override @@ -217,4 +220,317 @@ public class DefaultApplication implements Application { .add("requiredApps", requiredApps) .toString(); } + + /** + * Returns a default application builder. + * + * @return builder + */ + public static Builder builder() { + return new Builder(); + } + + /** + * Creates a new builder as a copy of an existing builder. + * + * @param builder existing builder to copy + * @return new builder + */ + public static Builder builder(Builder builder) { + return new Builder(builder); + } + + /** + * Creates a new builder as a copy of an existing application. + * + * @param application existing application to copy + * @return new builder + */ + public static Builder builder(Application application) { + return new Builder(application); + } + + /** + * Creates a new builder as a copy of an existing application description. + * + * @param appDesc existing application description + * @return new builder + */ + public static Builder builder(ApplicationDescription appDesc) { + return new Builder(appDesc); + } + + + /** + * Default application builder. + */ + public static final class Builder { + + private ApplicationId appId; + private Version version; + private String title; + private String description; + private String category; + private String url; + private String readme; + private byte[] icon; + private String origin; + private ApplicationRole role; + private Set permissions; + private Optional featuresRepo; + private List features; + private List requiredApps; + + /** + * Default constructor for the builder. + */ + public Builder() {} + + /** + * Updates the builder to be a copy of an existing builder. + * + * @param builder existing builder to copy + */ + public Builder(Builder builder) { + this.appId = builder.appId; + this.version = builder.version; + this.title = builder.title; + this.description = builder.description; + this.category = builder.category; + this.url = builder.url; + this.readme = builder.readme; + this.icon = builder.icon; + this.origin = builder.origin; + this.role = builder.role; + this.permissions = builder.permissions; + this.featuresRepo = builder.featuresRepo; + this.features = builder.features; + this.requiredApps = builder.requiredApps; + } + + /** + * Updates the builder to be a copy of an existing application. + * + * @param application existing application to copy + */ + public Builder(Application application) { + this.appId = application.id(); + this.version = application.version(); + this.title = application.title(); + this.description = application.description(); + this.category = application.category(); + this.url = application.url(); + this.readme = application.readme(); + this.icon = application.icon(); + this.origin = application.origin(); + this.role = application.role(); + this.permissions = application.permissions(); + this.featuresRepo = application.featuresRepo(); + this.features = application.features(); + this.requiredApps = application.requiredApps(); + } + + /** + * Updates the builder to be a copy of an existing application description. + * + * @param appDesc existing application description + */ + public Builder(ApplicationDescription appDesc) { + this.version = appDesc.version(); + this.title = appDesc.title(); + this.description = appDesc.description(); + this.category = appDesc.category(); + this.url = appDesc.url(); + this.readme = appDesc.readme(); + this.icon = appDesc.icon(); + this.origin = appDesc.origin(); + this.role = appDesc.role(); + this.permissions = appDesc.permissions(); + this.featuresRepo = appDesc.featuresRepo(); + this.features = appDesc.features(); + this.requiredApps = appDesc.requiredApps(); + } + + /** + * Adds an application id. + * + * @param appId application id + * @return builder + */ + public Builder withAppId(ApplicationId appId) { + this.appId = appId; + return this; + } + + /** + * Adds a version string. + * + * @param version version string + * @return builder + */ + public Builder withVersion(Version version) { + this.version = version; + return this; + } + + /** + * Adds a title string. + * + * @param title title string + * @return builder + */ + public Builder withTitle(String title) { + this.title = title; + return this; + } + + /** + * Adds a description string. + * + * @param description description string + * @return builder + */ + public Builder withDescription(String description) { + this.description = description; + return this; + } + + /** + * Adds a category string. + * + * @param category category string + * @return builder + */ + public Builder withCategory(String category) { + this.category = category; + return this; + } + + /** + * Adds a URL string. + * + * @param url url string + * @return builder + */ + public Builder withUrl(String url) { + this.url = url; + return this; + } + + /** + * Adds a readme string. + * + * @param readme readme string + * @return builder + */ + public Builder withReadme(String readme) { + this.readme = readme; + return this; + } + + /** + * Adds an icon. + * + * @param icon icon data + * @return builder + */ + public Builder withIcon(byte[] icon) { + this.icon = icon; + return this; + } + + /** + * Adds an origin string. + * + * @param origin origin string + * @return builder + */ + public Builder withOrigin(String origin) { + this.origin = origin; + return this; + } + + /** + * Adds an application role. + * + * @param role application role + * @return builder + */ + public Builder withRole(ApplicationRole role) { + this.role = role; + return this; + } + + /** + * Adds a permissions set. + * + * @param permissions permissions set + * @return builder + */ + public Builder withPermissions(Set permissions) { + this.permissions = permissions; + return this; + } + + /** + * Adds a URI for a features repository. + * + * @param featuresRepo Optional URI for a features repository + * @return builder + */ + public Builder withFeaturesRepo(Optional featuresRepo) { + this.featuresRepo = featuresRepo; + return this; + } + + /** + * Adds a features list. + * + * @param features features list + * @return builder + */ + public Builder withFeatures(List features) { + this.features = features; + return this; + } + + /** + * Adds a list of required applications. + * + * @param requiredApps List of name strings of required applications + * @return builder + */ + public Builder withRequiredApps(List requiredApps) { + this.requiredApps = requiredApps; + return this; + } + + /** + * Builds a default application object from the gathered parameters. + * + * @return new default application + */ + public DefaultApplication build() { + checkNotNull(appId, "ID cannot be null"); + checkNotNull(version, "Version cannot be null"); + checkNotNull(title, "Title cannot be null"); + checkNotNull(description, "Description cannot be null"); + checkNotNull(origin, "Origin cannot be null"); + checkNotNull(category, "Category cannot be null"); + checkNotNull(readme, "Readme cannot be null"); + checkNotNull(role, "Role cannot be null"); + checkNotNull(permissions, "Permissions cannot be null"); + checkNotNull(featuresRepo, "Features repo cannot be null"); + checkNotNull(features, "Features cannot be null"); + checkNotNull(requiredApps, "Required apps cannot be null"); + checkArgument(!features.isEmpty(), "There must be at least one feature"); + + return new DefaultApplication(appId, version, title, + description, origin, category, + url, readme, icon, + role, permissions, + featuresRepo, features, + requiredApps); + } + } } diff --git a/core/api/src/test/java/org/onosproject/app/ApplicationEventTest.java b/core/api/src/test/java/org/onosproject/app/ApplicationEventTest.java index 3cdedd2b34..61538f07ea 100644 --- a/core/api/src/test/java/org/onosproject/app/ApplicationEventTest.java +++ b/core/api/src/test/java/org/onosproject/app/ApplicationEventTest.java @@ -32,9 +32,22 @@ import static org.onosproject.core.DefaultApplicationTest.APP_ID; public class ApplicationEventTest extends AbstractEventTest { private Application createApp() { - return new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, CATEGORY, - URL, README, ICON, ROLE, PERMS, - Optional.of(FURL), FEATURES, APPS); + return DefaultApplication.builder() + .withAppId(APP_ID) + .withVersion(VER) + .withTitle(TITLE) + .withDescription(DESC) + .withOrigin(ORIGIN) + .withCategory(CATEGORY) + .withUrl(URL) + .withReadme(README) + .withIcon(ICON) + .withRole(ROLE) + .withPermissions(PERMS) + .withFeaturesRepo(Optional.of(FURL)) + .withFeatures(FEATURES) + .withRequiredApps(APPS) + .build(); } @Test @@ -53,4 +66,4 @@ public class ApplicationEventTest extends AbstractEventTest { validateEvent(event, APP_ACTIVATED, app, before, after); } -} \ No newline at end of file +} diff --git a/core/api/src/test/java/org/onosproject/app/DefaultApplicationDescriptionTest.java b/core/api/src/test/java/org/onosproject/app/DefaultApplicationDescriptionTest.java index ac8d971992..a935c1bdf6 100644 --- a/core/api/src/test/java/org/onosproject/app/DefaultApplicationDescriptionTest.java +++ b/core/api/src/test/java/org/onosproject/app/DefaultApplicationDescriptionTest.java @@ -29,6 +29,7 @@ import java.util.Set; import static org.junit.Assert.assertEquals; import static org.junit.Assert.assertTrue; +import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable; /** @@ -53,12 +54,34 @@ public class DefaultApplicationDescriptionTest { public static final List FEATURES = ImmutableList.of("foo", "bar"); public static final List APPS = ImmutableList.of("fifi"); + /** + * Checks that the DefaultApplicationDescription class is immutable. + */ + @Test + public void testImmutability() { + assertThatClassIsImmutable(DefaultApplicationDescription.class); + } + @Test public void basics() { ApplicationDescription app = - new DefaultApplicationDescription(APP_NAME, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, ICON, - ROLE, PERMS, FURL, FEATURES, APPS); + DefaultApplicationDescription.builder() + .withName(APP_NAME) + .withVersion(VER) + .withTitle(TITLE) + .withDescription(DESC) + .withOrigin(ORIGIN) + .withCategory(CATEGORY) + .withUrl(URL) + .withReadme(README) + .withIcon(ICON) + .withRole(ROLE) + .withPermissions(PERMS) + .withFeaturesRepo(FURL) + .withFeatures(FEATURES) + .withRequiredApps(APPS) + .build(); + assertEquals("incorrect id", APP_NAME, app.name()); assertEquals("incorrect version", VER, app.version()); assertEquals("incorrect title", TITLE, app.title()); @@ -74,4 +97,4 @@ public class DefaultApplicationDescriptionTest { assertEquals("incorrect apps", APPS, app.requiredApps()); assertTrue("incorrect toString", app.toString().contains(APP_NAME)); } -} \ No newline at end of file +} diff --git a/core/api/src/test/java/org/onosproject/core/DefaultApplicationTest.java b/core/api/src/test/java/org/onosproject/core/DefaultApplicationTest.java index 38113bfe59..b52312608d 100644 --- a/core/api/src/test/java/org/onosproject/core/DefaultApplicationTest.java +++ b/core/api/src/test/java/org/onosproject/core/DefaultApplicationTest.java @@ -28,6 +28,9 @@ import java.util.List; import java.util.Optional; import java.util.Set; +import static org.onlab.junit.ImmutableClassChecker.assertThatClassIsImmutable; +import static org.onosproject.core.DefaultApplication.Builder; + import static org.junit.Assert.*; import static org.onosproject.app.DefaultApplicationDescriptionTest.*; @@ -36,13 +39,35 @@ import static org.onosproject.app.DefaultApplicationDescriptionTest.*; */ public class DefaultApplicationTest { + /** + * Checks that the DefaultApplication class is immutable. + */ + @Test + public void testImmutability() { + assertThatClassIsImmutable(DefaultApplication.class); + } + public static final ApplicationId APP_ID = new DefaultApplicationId(2, APP_NAME); + private Builder baseBuilder = DefaultApplication.builder() + .withAppId(APP_ID) + .withVersion(VER) + .withTitle(TITLE) + .withDescription(DESC) + .withOrigin(ORIGIN) + .withCategory(CATEGORY) + .withUrl(URL) + .withReadme(README) + .withIcon(ICON) + .withRole(ROLE) + .withPermissions(PERMS) + .withFeaturesRepo(Optional.of(FURL)) + .withFeatures(FEATURES) + .withRequiredApps(APPS); @Test public void basics() { - Application app = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, ICON, ROLE, - PERMS, Optional.of(FURL), FEATURES, APPS); + Application app = baseBuilder.build(); + assertEquals("incorrect id", APP_ID, app.id()); assertEquals("incorrect version", VER, app.version()); assertEquals("incorrect title", TITLE, app.title()); @@ -62,20 +87,20 @@ public class DefaultApplicationTest { @Test public void testEquality() { - Application a1 = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, ICON, ROLE, - PERMS, Optional.of(FURL), FEATURES, APPS); - Application a2 = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, ICON, ROLE, - PERMS, Optional.of(FURL), FEATURES, APPS); - Application a3 = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, ICON, ROLE, - PERMS, Optional.empty(), FEATURES, APPS); - Application a4 = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN + "asd", - CATEGORY, URL, README, ICON, ROLE, - PERMS, Optional.of(FURL), FEATURES, APPS); - new EqualsTester().addEqualityGroup(a1, a2) - .addEqualityGroup(a3).addEqualityGroup(a4).testEquals(); + Application a1 = baseBuilder.build(); + Application a2 = DefaultApplication.builder(a1) + .build(); + Application a3 = DefaultApplication.builder(baseBuilder) + .withFeaturesRepo(Optional.empty()) + .build(); + Application a4 = DefaultApplication.builder(baseBuilder) + .withOrigin(ORIGIN + "asd") + .build(); + new EqualsTester() + .addEqualityGroup(a1, a2) + .addEqualityGroup(a3) + .addEqualityGroup(a4) + .testEquals(); } @@ -85,9 +110,8 @@ public class DefaultApplicationTest { public void immutableIcon() { byte[] iconSourceData = ICON_ORIG.clone(); - Application app = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, iconSourceData, ROLE, - PERMS, Optional.of(FURL), FEATURES, APPS); + Application app = DefaultApplication.builder(baseBuilder) + .withIcon(iconSourceData).build(); // can we modify the icon after getting a reference to the app? byte[] icon = app.icon(); @@ -126,9 +150,7 @@ public class DefaultApplicationTest { // Set p = PERMS_ORIG; Set p = PERMS_UNSAFE; - Application app = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, ICON, ROLE, - p, Optional.of(FURL), FEATURES, APPS); + Application app = baseBuilder.build(); Set perms = app.permissions(); try { @@ -168,9 +190,7 @@ public class DefaultApplicationTest { // List f = FEATURES_ORIG; List f = FEATURES_UNSAFE; - Application app = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, ICON, ROLE, - PERMS, Optional.of(FURL), f, APPS); + Application app = DefaultApplication.builder(baseBuilder).withFeatures(f).build(); List features = app.features(); try { @@ -188,9 +208,7 @@ public class DefaultApplicationTest { // List ra = REQ_APPS_ORIG; List ra = REQ_APPS_UNSAFE; - Application app = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, ICON, ROLE, - PERMS, Optional.of(FURL), FEATURES, ra); + Application app = DefaultApplication.builder(baseBuilder).withRequiredApps(ra).build(); List reqApps = app.requiredApps(); try { @@ -204,11 +222,9 @@ public class DefaultApplicationTest { @Test public void nullIcon() { - Application app = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, null, ROLE, - PERMS, Optional.of(FURL), FEATURES, APPS); + Application app = DefaultApplication.builder(baseBuilder).withIcon(null).build(); byte[] icon = app.icon(); assertNotNull("null icon", icon); assertEquals("unexpected size", 0, icon.length); } -} \ No newline at end of file +} diff --git a/core/common/src/main/java/org/onosproject/common/app/ApplicationArchive.java b/core/common/src/main/java/org/onosproject/common/app/ApplicationArchive.java index 92709cbd10..55c198ed03 100644 --- a/core/common/src/main/java/org/onosproject/common/app/ApplicationArchive.java +++ b/core/common/src/main/java/org/onosproject/common/app/ApplicationArchive.java @@ -328,10 +328,22 @@ public class ApplicationArchive // put short description to description field String desc = compactDescription(readme); - return new DefaultApplicationDescription(name, version, title, desc, origin, - category, url, readme, icon, - role, perms, featuresRepo, - features, requiredApps); + return DefaultApplicationDescription.builder() + .withName(name) + .withVersion(version) + .withTitle(title) + .withDescription(desc) + .withOrigin(origin) + .withCategory(category) + .withUrl(url) + .withReadme(readme) + .withIcon(icon) + .withRole(role) + .withPermissions(perms) + .withFeaturesRepo(featuresRepo) + .withFeatures(features) + .withRequiredApps(requiredApps) + .build(); } // Expands the specified ZIP stream into app-specific directory. diff --git a/core/common/src/test/java/org/onosproject/store/trivial/SimpleApplicationStore.java b/core/common/src/test/java/org/onosproject/store/trivial/SimpleApplicationStore.java index ec05afbfc0..c904455bcd 100644 --- a/core/common/src/test/java/org/onosproject/store/trivial/SimpleApplicationStore.java +++ b/core/common/src/test/java/org/onosproject/store/trivial/SimpleApplicationStore.java @@ -76,20 +76,11 @@ public class SimpleApplicationStore extends ApplicationArchive ApplicationId appId = idStore.registerApplication(name); ApplicationDescription appDesc = getApplicationDescription(name); DefaultApplication app = - new DefaultApplication(appId, - appDesc.version(), - appDesc.title(), - appDesc.description(), - appDesc.origin(), - appDesc.category(), - appDesc.url(), - appDesc.readme(), - appDesc.icon(), - appDesc.role(), - appDesc.permissions(), - appDesc.featuresRepo(), - appDesc.features(), - appDesc.requiredApps()); + DefaultApplication + .builder(appDesc) + .withAppId(appId) + .build(); + apps.put(appId, app); states.put(appId, isActive(name) ? INSTALLED : ACTIVE); // load app permissions @@ -129,20 +120,11 @@ public class SimpleApplicationStore extends ApplicationArchive ApplicationDescription appDesc = saveApplication(appDescStream); ApplicationId appId = idStore.registerApplication(appDesc.name()); DefaultApplication app = - new DefaultApplication(appId, - appDesc.version(), - appDesc.title(), - appDesc.description(), - appDesc.origin(), - appDesc.category(), - appDesc.url(), - appDesc.readme(), - appDesc.icon(), - appDesc.role(), - appDesc.permissions(), - appDesc.featuresRepo(), - appDesc.features(), - appDesc.requiredApps()); + DefaultApplication + .builder(appDesc) + .withAppId(appId) + .build(); + apps.put(appId, app); states.put(appId, INSTALLED); delegate.notify(new ApplicationEvent(APP_INSTALLED, app)); diff --git a/core/common/src/test/java/org/onosproject/utils/ComparatorsTest.java b/core/common/src/test/java/org/onosproject/utils/ComparatorsTest.java index 8562d7e1d4..6047719bb3 100644 --- a/core/common/src/test/java/org/onosproject/utils/ComparatorsTest.java +++ b/core/common/src/test/java/org/onosproject/utils/ComparatorsTest.java @@ -161,9 +161,22 @@ public class ComparatorsTest { } private Application app(int id, String name) { - return new DefaultApplication(new DefaultApplicationId(id, name), VER, TITLE, DESC, ORIGIN, - CATEGORY, URL, README, ICON, ROLE, - PERMS, Optional.of(FURL), FEATURES, APPS); + return DefaultApplication.builder() + .withAppId(new DefaultApplicationId(id, name)) + .withVersion(VER) + .withTitle(TITLE) + .withDescription(DESC) + .withOrigin(ORIGIN) + .withCategory(CATEGORY) + .withUrl(URL) + .withReadme(README) + .withIcon(ICON) + .withRole(ROLE) + .withPermissions(PERMS) + .withFeaturesRepo(Optional.of(FURL)) + .withFeatures(FEATURES) + .withRequiredApps(APPS) + .build(); } @Test diff --git a/core/net/src/test/java/org/onosproject/app/impl/ApplicationManagerTest.java b/core/net/src/test/java/org/onosproject/app/impl/ApplicationManagerTest.java index 5fd770168e..5528f269a9 100644 --- a/core/net/src/test/java/org/onosproject/app/impl/ApplicationManagerTest.java +++ b/core/net/src/test/java/org/onosproject/app/impl/ApplicationManagerTest.java @@ -15,7 +15,6 @@ */ package org.onosproject.app.impl; -import com.google.common.collect.ImmutableList; import com.google.common.collect.ImmutableSet; import org.junit.After; import org.junit.Before; @@ -138,9 +137,22 @@ public class ApplicationManagerTest { @Override public Application create(InputStream appDescStream) { - app = new DefaultApplication(APP_ID, VER, TITLE, DESC, ORIGIN, CATEGORY, - URL, README, ICON, ROLE, PERMS, - Optional.of(FURL), FEATURES, ImmutableList.of()); + app = DefaultApplication.builder() + .withAppId(APP_ID) + .withVersion(VER) + .withTitle(TITLE) + .withDescription(DESC) + .withOrigin(ORIGIN) + .withCategory(CATEGORY) + .withUrl(URL) + .withReadme(README) + .withIcon(ICON) + .withRole(ROLE) + .withPermissions(PERMS) + .withFeaturesRepo(Optional.of(FURL)) + .withFeatures(FEATURES) + .withRequiredApps(APPS) + .build(); state = INSTALLED; delegate.notify(new ApplicationEvent(APP_INSTALLED, app)); return app; @@ -211,4 +223,4 @@ public class ApplicationManagerTest { } } -} \ No newline at end of file +} diff --git a/core/security/src/test/java/org/onosproject/security/impl/SecurityModeManagerTest.java b/core/security/src/test/java/org/onosproject/security/impl/SecurityModeManagerTest.java index 15155ff379..328d903bb2 100644 --- a/core/security/src/test/java/org/onosproject/security/impl/SecurityModeManagerTest.java +++ b/core/security/src/test/java/org/onosproject/security/impl/SecurityModeManagerTest.java @@ -69,11 +69,22 @@ public class SecurityModeManagerTest { testFeatures.add("testFeature"); testRequiredApps = new ArrayList(); testRequiredApps.add("testRequiredApp"); - app = new DefaultApplication(appId, Version.version(1, 1, "patch", "build"), "testTitle", - "testDes", "testOri", "testCT", - "testurl", "test", null, - ApplicationRole.ADMIN, testPermissions, - Optional.ofNullable(null), testFeatures, testRequiredApps); + app = DefaultApplication.builder() + .withAppId(appId) + .withVersion(Version.version(1, 1, "patch", "build")) + .withTitle("testTitle") + .withDescription("testDes") + .withOrigin("testOri") + .withCategory("testCT") + .withUrl("testurl") + .withReadme("test") + .withIcon(null) + .withRole(ApplicationRole.ADMIN) + .withPermissions(testPermissions) + .withFeaturesRepo(Optional.ofNullable(null)) + .withFeatures(testFeatures) + .withRequiredApps(testRequiredApps) + .build(); store.registerApplication(appId); } diff --git a/core/security/src/test/java/org/onosproject/security/store/DistributedSecurityModeStoreTest.java b/core/security/src/test/java/org/onosproject/security/store/DistributedSecurityModeStoreTest.java index f3e38bff3b..220b44e974 100644 --- a/core/security/src/test/java/org/onosproject/security/store/DistributedSecurityModeStoreTest.java +++ b/core/security/src/test/java/org/onosproject/security/store/DistributedSecurityModeStoreTest.java @@ -79,11 +79,22 @@ public class DistributedSecurityModeStoreTest { testFeatures.add("testFeature"); testRequiredApps = new ArrayList(); testRequiredApps.add("testRequiredApp"); - app = new DefaultApplication(appId, Version.version(1, 1, "patch", "build"), "testTitle", - "testDes", "testOri", "testCT", - "testurl", "test", null, - ApplicationRole.ADMIN, testPermissions, - Optional.ofNullable(null), testFeatures, testRequiredApps); + app = DefaultApplication.builder() + .withAppId(appId) + .withVersion(Version.version(1, 1, "patch", "build")) + .withTitle("testTitle") + .withDescription("testDes") + .withOrigin("testOri") + .withCategory("testCT") + .withUrl("testurl") + .withReadme("test") + .withIcon(null) + .withRole(ApplicationRole.ADMIN) + .withPermissions(testPermissions) + .withFeaturesRepo(Optional.ofNullable(null)) + .withFeatures(testFeatures) + .withRequiredApps(testRequiredApps) + .build(); testLocations = new HashSet(); testLocations.add("locationA"); diff --git a/core/store/dist/src/main/java/org/onosproject/store/app/DistributedApplicationStore.java b/core/store/dist/src/main/java/org/onosproject/store/app/DistributedApplicationStore.java index 532b279fa5..c65d1aaba0 100644 --- a/core/store/dist/src/main/java/org/onosproject/store/app/DistributedApplicationStore.java +++ b/core/store/dist/src/main/java/org/onosproject/store/app/DistributedApplicationStore.java @@ -571,20 +571,10 @@ public class DistributedApplicationStore extends ApplicationArchive */ private Application registerApp(ApplicationDescription appDesc) { ApplicationId appId = idStore.registerApplication(appDesc.name()); - return new DefaultApplication(appId, - appDesc.version(), - appDesc.title(), - appDesc.description(), - appDesc.origin(), - appDesc.category(), - appDesc.url(), - appDesc.readme(), - appDesc.icon(), - appDesc.role(), - appDesc.permissions(), - appDesc.featuresRepo(), - appDesc.features(), - appDesc.requiredApps()); + return DefaultApplication + .builder(appDesc) + .withAppId(appId) + .build(); } /** diff --git a/incubator/protobuf/models/src/main/java/org/onosproject/incubator/protobuf/models/core/ApplicationProtoTranslator.java b/incubator/protobuf/models/src/main/java/org/onosproject/incubator/protobuf/models/core/ApplicationProtoTranslator.java index a4b2ca6401..eba30ee030 100644 --- a/incubator/protobuf/models/src/main/java/org/onosproject/incubator/protobuf/models/core/ApplicationProtoTranslator.java +++ b/incubator/protobuf/models/src/main/java/org/onosproject/incubator/protobuf/models/core/ApplicationProtoTranslator.java @@ -23,7 +23,6 @@ import org.onosproject.grpc.core.models.ApplicationProtoOuterClass.ApplicationPr import org.onosproject.incubator.protobuf.models.security.PermissionProtoTranslator; import org.onosproject.security.Permission; -import java.util.Optional; import java.util.Set; import static org.onosproject.grpc.core.models.ApplicationProtoOuterClass.ApplicationProto.getDefaultInstance; @@ -46,12 +45,21 @@ public final class ApplicationProtoTranslator { app.getPermissionsList().forEach(p -> permissions.add(PermissionProtoTranslator.translate(p))); - return new DefaultApplication(ApplicationIdProtoTranslator.translate(app.getAppId()), - VersionProtoTranslator.translate(app.getVersion()), app.getTitle(), - app.getDescription(), app.getOrigin(), app.getCategory(), app.getUrl(), - app.getReadme(), app.toByteArray(), - (ApplicationRole) ApplicationEnumsProtoTranslator.translate(app.getRole()).get(), - permissions, Optional.empty(), app.getFeaturesList(), app.getRequiredAppsList()); + return DefaultApplication.builder() + .withAppId(ApplicationIdProtoTranslator.translate(app.getAppId())) + .withVersion(VersionProtoTranslator.translate(app.getVersion())) + .withTitle(app.getTitle()) + .withDescription(app.getDescription()) + .withOrigin(app.getOrigin()) + .withCategory(app.getCategory()) + .withUrl(app.getUrl()) + .withReadme(app.getReadme()) + .withIcon(app.toByteArray()) + .withRole((ApplicationRole) ApplicationEnumsProtoTranslator.translate(app.getRole()).get()) + .withPermissions(permissions) + .withFeatures(app.getFeaturesList()) + .withRequiredApps(app.getRequiredAppsList()) + .build(); } /** diff --git a/web/api/src/test/java/org/onosproject/rest/resources/ApplicationsResourceTest.java b/web/api/src/test/java/org/onosproject/rest/resources/ApplicationsResourceTest.java index 61e717441b..48cbd2d54c 100644 --- a/web/api/src/test/java/org/onosproject/rest/resources/ApplicationsResourceTest.java +++ b/web/api/src/test/java/org/onosproject/rest/resources/ApplicationsResourceTest.java @@ -107,30 +107,55 @@ public class ApplicationsResourceTest extends ResourceTest { private static final URI FURL = URI.create("mvn:org.foo-features/1.2a/xml/features"); private static final Version VER = Version.version(1, 2, "a", null); + private DefaultApplication.Builder baseBuilder = DefaultApplication.builder() + .withVersion(VER) + .withIcon(new byte[0]) + .withRole(ApplicationRole.ADMIN) + .withPermissions(ImmutableSet.of()) + .withFeaturesRepo(Optional.of(FURL)) + .withFeatures(ImmutableList.of("My Feature")) + .withRequiredApps(ImmutableList.of()); + private Application app1 = - new DefaultApplication(id1, VER, "title1", - "desc1", "origin1", "category1", "url1", - "readme1", new byte[0], ApplicationRole.ADMIN, - ImmutableSet.of(), Optional.of(FURL), - ImmutableList.of("My Feature"), ImmutableList.of()); + DefaultApplication.builder(baseBuilder) + .withAppId(id1) + .withTitle("title1") + .withDescription("desc1") + .withOrigin("origin1") + .withCategory("category1") + .withUrl("url1") + .withReadme("readme1") + .build(); private Application app2 = - new DefaultApplication(id2, VER, "title2", - "desc2", "origin2", "category2", "url2", - "readme2", new byte[0], ApplicationRole.ADMIN, - ImmutableSet.of(), Optional.of(FURL), - ImmutableList.of("My Feature"), ImmutableList.of()); + DefaultApplication.builder(baseBuilder) + .withAppId(id2) + .withTitle("title2") + .withDescription("desc2") + .withOrigin("origin2") + .withCategory("category2") + .withUrl("url2") + .withReadme("readme2") + .build(); private Application app3 = - new DefaultApplication(id3, VER, "title3", - "desc3", "origin3", "category3", "url3", - "readme3", new byte[0], ApplicationRole.ADMIN, - ImmutableSet.of(), Optional.of(FURL), - ImmutableList.of("My Feature"), ImmutableList.of()); + DefaultApplication.builder(baseBuilder) + .withAppId(id3) + .withTitle("title3") + .withDescription("desc3") + .withOrigin("origin3") + .withCategory("category3") + .withUrl("url3") + .withReadme("readme3") + .build(); private Application app4 = - new DefaultApplication(id4, VER, "title4", - "desc4", "origin4", "category4", "url4", - "readme4", new byte[0], ApplicationRole.ADMIN, - ImmutableSet.of(), Optional.of(FURL), - ImmutableList.of("My Feature"), ImmutableList.of()); + DefaultApplication.builder(baseBuilder) + .withAppId(id4) + .withTitle("title4") + .withDescription("desc4") + .withOrigin("origin4") + .withCategory("category4") + .withUrl("url4") + .withReadme("readme4") + .build(); /** * Hamcrest matcher to check that an application representation in JSON matches