From e18a330163bce7ac2fddd75c32abce9af319532c Mon Sep 17 00:00:00 2001 From: Thomas Vachuska Date: Tue, 23 Jun 2015 12:48:28 -0700 Subject: [PATCH] ONOS-2091 Installing app when already installed will now raise an error Change-Id: I4dacd63bf4a99244b23b932d35dd9cbd088548c1 --- .../common/app/ApplicationArchive.java | 6 +++- .../app/impl/ApplicationManager.java | 6 ---- .../impl/SimpleApplicationStoreTest.java | 28 ++++++++++++++++-- .../IllegalStateExceptionMapper.java | 29 +++++++++++++++++++ web/api/src/main/webapp/WEB-INF/web.xml | 1 + 5 files changed, 60 insertions(+), 10 deletions(-) create mode 100644 web/api/src/main/java/org/onosproject/rest/exceptions/IllegalStateExceptionMapper.java 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 78fd0b9407..1b1c23bf2b 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 @@ -50,6 +50,7 @@ import java.util.Set; import java.util.zip.ZipEntry; import java.util.zip.ZipInputStream; +import static com.google.common.base.Preconditions.checkState; import static com.google.common.io.ByteStreams.toByteArray; import static com.google.common.io.Files.createParentDirs; import static com.google.common.io.Files.write; @@ -108,7 +109,7 @@ public class ApplicationArchive * * @return top-level directory path */ - protected String getRootPath() { + public String getRootPath() { return root.getPath(); } @@ -179,6 +180,8 @@ public class ApplicationArchive boolean plainXml = isPlainXml(cache); ApplicationDescription desc = plainXml ? parsePlainAppDescription(bis) : parseZippedAppDescription(bis); + checkState(!appFile(desc.name(), APP_XML).exists(), + "Application %s already installed", desc.name()); if (plainXml) { expandPlainApplication(cache, desc); @@ -309,6 +312,7 @@ public class ApplicationArchive private void expandPlainApplication(byte[] stream, ApplicationDescription desc) throws IOException { File file = appFile(desc.name(), APP_XML); + checkState(!file.getParentFile().exists(), "Application already installed"); createParentDirs(file); write(stream, file); } diff --git a/core/net/src/main/java/org/onosproject/app/impl/ApplicationManager.java b/core/net/src/main/java/org/onosproject/app/impl/ApplicationManager.java index 989417402e..153b0249d1 100644 --- a/core/net/src/main/java/org/onosproject/app/impl/ApplicationManager.java +++ b/core/net/src/main/java/org/onosproject/app/impl/ApplicationManager.java @@ -93,7 +93,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS @Override public Set getApplications() { checkPermission(Permission.APP_READ); - return store.getApplications(); } @@ -108,7 +107,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS @Override public Application getApplication(ApplicationId appId) { checkPermission(Permission.APP_READ); - checkNotNull(appId, APP_ID_NULL); return store.getApplication(appId); } @@ -116,7 +114,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS @Override public ApplicationState getState(ApplicationId appId) { checkPermission(Permission.APP_READ); - checkNotNull(appId, APP_ID_NULL); return store.getState(appId); } @@ -124,7 +121,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS @Override public Set getPermissions(ApplicationId appId) { checkPermission(Permission.APP_READ); - checkNotNull(appId, APP_ID_NULL); return store.getPermissions(appId); } @@ -167,14 +163,12 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS @Override public void addListener(ApplicationListener listener) { checkPermission(Permission.APP_EVENT); - listenerRegistry.addListener(listener); } @Override public void removeListener(ApplicationListener listener) { checkPermission(Permission.APP_EVENT); - listenerRegistry.removeListener(listener); } diff --git a/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleApplicationStoreTest.java b/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleApplicationStoreTest.java index a6fb24c92b..0e7a16ef03 100644 --- a/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleApplicationStoreTest.java +++ b/core/store/trivial/src/test/java/org/onosproject/store/trivial/impl/SimpleApplicationStoreTest.java @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableSet; import org.junit.After; import org.junit.Before; import org.junit.Test; +import org.onlab.util.Tools; import org.onosproject.app.ApplicationEvent; import org.onosproject.app.ApplicationStoreDelegate; import org.onosproject.common.app.ApplicationArchive; @@ -28,6 +29,10 @@ import org.onosproject.core.Permission; import org.onosproject.core.ApplicationIdStoreAdapter; import org.onosproject.core.DefaultApplicationId; +import java.io.File; +import java.io.IOException; +import java.util.Random; + import static org.junit.Assert.assertEquals; import static org.onosproject.app.ApplicationEvent.Type.APP_INSTALLED; import static org.onosproject.app.ApplicationEvent.Type.APP_DEACTIVATED; @@ -42,23 +47,33 @@ import static org.onosproject.app.ApplicationState.INSTALLED; */ public class SimpleApplicationStoreTest { - private SimpleApplicationStore store = new SimpleApplicationStore(); + static final String ROOT = "/tmp/app-junit/"; + static final String STORE = ROOT + new Random().nextInt(1000) + "/foo"; + + private TestApplicationStore store = new TestApplicationStore(); private TestDelegate delegate = new TestDelegate(); + private static final Object LOCK = new Object(); @Before public void setUp() { store.idStore = new TestIdStore(); + store.setRootPath(STORE); store.setDelegate(delegate); store.activate(); } @After - public void tearDown() { + public void tearDown() throws IOException { + if (new File(ROOT).exists()) { + Tools.removeDirectory(ROOT); + } store.deactivate(); } private Application createTestApp() { - return store.create(ApplicationArchive.class.getResourceAsStream("app.zip")); + synchronized (LOCK) { + return store.create(ApplicationArchive.class.getResourceAsStream("app.zip")); + } } @Test @@ -132,4 +147,11 @@ public class SimpleApplicationStoreTest { this.event = event; } } + + private class TestApplicationStore extends SimpleApplicationStore { + @Override + public void setRootPath(String root) { + super.setRootPath(root); + } + } } \ No newline at end of file diff --git a/web/api/src/main/java/org/onosproject/rest/exceptions/IllegalStateExceptionMapper.java b/web/api/src/main/java/org/onosproject/rest/exceptions/IllegalStateExceptionMapper.java new file mode 100644 index 0000000000..f20daf5861 --- /dev/null +++ b/web/api/src/main/java/org/onosproject/rest/exceptions/IllegalStateExceptionMapper.java @@ -0,0 +1,29 @@ +/* + * Copyright 2015 Open Networking Laboratory + * + * Licensed under the Apache License, Version 2.0 (the "License"); + * you may not use this file except in compliance with the License. + * You may obtain a copy of the License at + * + * http://www.apache.org/licenses/LICENSE-2.0 + * + * Unless required by applicable law or agreed to in writing, software + * distributed under the License is distributed on an "AS IS" BASIS, + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. + * See the License for the specific language governing permissions and + * limitations under the License. + */ +package org.onosproject.rest.exceptions; + +import javax.ws.rs.core.Response; + +/** + * Mapper for illegal state exceptions to the BAD_REQUEST response code. + */ +public class IllegalStateExceptionMapper extends AbstractMapper { + @Override + protected Response.Status responseStatus() { + return Response.Status.CONFLICT; + } +} + diff --git a/web/api/src/main/webapp/WEB-INF/web.xml b/web/api/src/main/webapp/WEB-INF/web.xml index d8759fec93..4a9e5ab77b 100644 --- a/web/api/src/main/webapp/WEB-INF/web.xml +++ b/web/api/src/main/webapp/WEB-INF/web.xml @@ -62,6 +62,7 @@ org.onosproject.rest.exceptions.BadRequestMapper, org.onosproject.rest.exceptions.WebApplicationExceptionMapper, org.onosproject.rest.exceptions.IllegalArgumentExceptionMapper, + org.onosproject.rest.exceptions.IllegalStateExceptionMapper, org.onosproject.rest.resources.JsonBodyWriter, org.onosproject.rest.resources.ApplicationsWebResource,