Committed by
Gerrit Code Review
ONOS-2091 Installing app when already installed will now raise an error
Change-Id: I4dacd63bf4a99244b23b932d35dd9cbd088548c1
Showing
5 changed files
with
59 additions
and
9 deletions
| ... | @@ -50,6 +50,7 @@ import java.util.Set; | ... | @@ -50,6 +50,7 @@ import java.util.Set; |
| 50 | import java.util.zip.ZipEntry; | 50 | import java.util.zip.ZipEntry; |
| 51 | import java.util.zip.ZipInputStream; | 51 | import java.util.zip.ZipInputStream; |
| 52 | 52 | ||
| 53 | +import static com.google.common.base.Preconditions.checkState; | ||
| 53 | import static com.google.common.io.ByteStreams.toByteArray; | 54 | import static com.google.common.io.ByteStreams.toByteArray; |
| 54 | import static com.google.common.io.Files.createParentDirs; | 55 | import static com.google.common.io.Files.createParentDirs; |
| 55 | import static com.google.common.io.Files.write; | 56 | import static com.google.common.io.Files.write; |
| ... | @@ -108,7 +109,7 @@ public class ApplicationArchive | ... | @@ -108,7 +109,7 @@ public class ApplicationArchive |
| 108 | * | 109 | * |
| 109 | * @return top-level directory path | 110 | * @return top-level directory path |
| 110 | */ | 111 | */ |
| 111 | - protected String getRootPath() { | 112 | + public String getRootPath() { |
| 112 | return root.getPath(); | 113 | return root.getPath(); |
| 113 | } | 114 | } |
| 114 | 115 | ||
| ... | @@ -179,6 +180,8 @@ public class ApplicationArchive | ... | @@ -179,6 +180,8 @@ public class ApplicationArchive |
| 179 | boolean plainXml = isPlainXml(cache); | 180 | boolean plainXml = isPlainXml(cache); |
| 180 | ApplicationDescription desc = plainXml ? | 181 | ApplicationDescription desc = plainXml ? |
| 181 | parsePlainAppDescription(bis) : parseZippedAppDescription(bis); | 182 | parsePlainAppDescription(bis) : parseZippedAppDescription(bis); |
| 183 | + checkState(!appFile(desc.name(), APP_XML).exists(), | ||
| 184 | + "Application %s already installed", desc.name()); | ||
| 182 | 185 | ||
| 183 | if (plainXml) { | 186 | if (plainXml) { |
| 184 | expandPlainApplication(cache, desc); | 187 | expandPlainApplication(cache, desc); |
| ... | @@ -309,6 +312,7 @@ public class ApplicationArchive | ... | @@ -309,6 +312,7 @@ public class ApplicationArchive |
| 309 | private void expandPlainApplication(byte[] stream, ApplicationDescription desc) | 312 | private void expandPlainApplication(byte[] stream, ApplicationDescription desc) |
| 310 | throws IOException { | 313 | throws IOException { |
| 311 | File file = appFile(desc.name(), APP_XML); | 314 | File file = appFile(desc.name(), APP_XML); |
| 315 | + checkState(!file.getParentFile().exists(), "Application already installed"); | ||
| 312 | createParentDirs(file); | 316 | createParentDirs(file); |
| 313 | write(stream, file); | 317 | write(stream, file); |
| 314 | } | 318 | } | ... | ... |
| ... | @@ -93,7 +93,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS | ... | @@ -93,7 +93,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS |
| 93 | @Override | 93 | @Override |
| 94 | public Set<Application> getApplications() { | 94 | public Set<Application> getApplications() { |
| 95 | checkPermission(Permission.APP_READ); | 95 | checkPermission(Permission.APP_READ); |
| 96 | - | ||
| 97 | return store.getApplications(); | 96 | return store.getApplications(); |
| 98 | } | 97 | } |
| 99 | 98 | ||
| ... | @@ -108,7 +107,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS | ... | @@ -108,7 +107,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS |
| 108 | @Override | 107 | @Override |
| 109 | public Application getApplication(ApplicationId appId) { | 108 | public Application getApplication(ApplicationId appId) { |
| 110 | checkPermission(Permission.APP_READ); | 109 | checkPermission(Permission.APP_READ); |
| 111 | - | ||
| 112 | checkNotNull(appId, APP_ID_NULL); | 110 | checkNotNull(appId, APP_ID_NULL); |
| 113 | return store.getApplication(appId); | 111 | return store.getApplication(appId); |
| 114 | } | 112 | } |
| ... | @@ -116,7 +114,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS | ... | @@ -116,7 +114,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS |
| 116 | @Override | 114 | @Override |
| 117 | public ApplicationState getState(ApplicationId appId) { | 115 | public ApplicationState getState(ApplicationId appId) { |
| 118 | checkPermission(Permission.APP_READ); | 116 | checkPermission(Permission.APP_READ); |
| 119 | - | ||
| 120 | checkNotNull(appId, APP_ID_NULL); | 117 | checkNotNull(appId, APP_ID_NULL); |
| 121 | return store.getState(appId); | 118 | return store.getState(appId); |
| 122 | } | 119 | } |
| ... | @@ -124,7 +121,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS | ... | @@ -124,7 +121,6 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS |
| 124 | @Override | 121 | @Override |
| 125 | public Set<Permission> getPermissions(ApplicationId appId) { | 122 | public Set<Permission> getPermissions(ApplicationId appId) { |
| 126 | checkPermission(Permission.APP_READ); | 123 | checkPermission(Permission.APP_READ); |
| 127 | - | ||
| 128 | checkNotNull(appId, APP_ID_NULL); | 124 | checkNotNull(appId, APP_ID_NULL); |
| 129 | return store.getPermissions(appId); | 125 | return store.getPermissions(appId); |
| 130 | } | 126 | } |
| ... | @@ -167,14 +163,12 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS | ... | @@ -167,14 +163,12 @@ public class ApplicationManager implements ApplicationService, ApplicationAdminS |
| 167 | @Override | 163 | @Override |
| 168 | public void addListener(ApplicationListener listener) { | 164 | public void addListener(ApplicationListener listener) { |
| 169 | checkPermission(Permission.APP_EVENT); | 165 | checkPermission(Permission.APP_EVENT); |
| 170 | - | ||
| 171 | listenerRegistry.addListener(listener); | 166 | listenerRegistry.addListener(listener); |
| 172 | } | 167 | } |
| 173 | 168 | ||
| 174 | @Override | 169 | @Override |
| 175 | public void removeListener(ApplicationListener listener) { | 170 | public void removeListener(ApplicationListener listener) { |
| 176 | checkPermission(Permission.APP_EVENT); | 171 | checkPermission(Permission.APP_EVENT); |
| 177 | - | ||
| 178 | listenerRegistry.removeListener(listener); | 172 | listenerRegistry.removeListener(listener); |
| 179 | } | 173 | } |
| 180 | 174 | ... | ... |
| ... | @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableSet; | ... | @@ -19,6 +19,7 @@ import com.google.common.collect.ImmutableSet; |
| 19 | import org.junit.After; | 19 | import org.junit.After; |
| 20 | import org.junit.Before; | 20 | import org.junit.Before; |
| 21 | import org.junit.Test; | 21 | import org.junit.Test; |
| 22 | +import org.onlab.util.Tools; | ||
| 22 | import org.onosproject.app.ApplicationEvent; | 23 | import org.onosproject.app.ApplicationEvent; |
| 23 | import org.onosproject.app.ApplicationStoreDelegate; | 24 | import org.onosproject.app.ApplicationStoreDelegate; |
| 24 | import org.onosproject.common.app.ApplicationArchive; | 25 | import org.onosproject.common.app.ApplicationArchive; |
| ... | @@ -28,6 +29,10 @@ import org.onosproject.core.Permission; | ... | @@ -28,6 +29,10 @@ import org.onosproject.core.Permission; |
| 28 | import org.onosproject.core.ApplicationIdStoreAdapter; | 29 | import org.onosproject.core.ApplicationIdStoreAdapter; |
| 29 | import org.onosproject.core.DefaultApplicationId; | 30 | import org.onosproject.core.DefaultApplicationId; |
| 30 | 31 | ||
| 32 | +import java.io.File; | ||
| 33 | +import java.io.IOException; | ||
| 34 | +import java.util.Random; | ||
| 35 | + | ||
| 31 | import static org.junit.Assert.assertEquals; | 36 | import static org.junit.Assert.assertEquals; |
| 32 | import static org.onosproject.app.ApplicationEvent.Type.APP_INSTALLED; | 37 | import static org.onosproject.app.ApplicationEvent.Type.APP_INSTALLED; |
| 33 | import static org.onosproject.app.ApplicationEvent.Type.APP_DEACTIVATED; | 38 | import static org.onosproject.app.ApplicationEvent.Type.APP_DEACTIVATED; |
| ... | @@ -42,24 +47,34 @@ import static org.onosproject.app.ApplicationState.INSTALLED; | ... | @@ -42,24 +47,34 @@ import static org.onosproject.app.ApplicationState.INSTALLED; |
| 42 | */ | 47 | */ |
| 43 | public class SimpleApplicationStoreTest { | 48 | public class SimpleApplicationStoreTest { |
| 44 | 49 | ||
| 45 | - private SimpleApplicationStore store = new SimpleApplicationStore(); | 50 | + static final String ROOT = "/tmp/app-junit/"; |
| 51 | + static final String STORE = ROOT + new Random().nextInt(1000) + "/foo"; | ||
| 52 | + | ||
| 53 | + private TestApplicationStore store = new TestApplicationStore(); | ||
| 46 | private TestDelegate delegate = new TestDelegate(); | 54 | private TestDelegate delegate = new TestDelegate(); |
| 55 | + private static final Object LOCK = new Object(); | ||
| 47 | 56 | ||
| 48 | @Before | 57 | @Before |
| 49 | public void setUp() { | 58 | public void setUp() { |
| 50 | store.idStore = new TestIdStore(); | 59 | store.idStore = new TestIdStore(); |
| 60 | + store.setRootPath(STORE); | ||
| 51 | store.setDelegate(delegate); | 61 | store.setDelegate(delegate); |
| 52 | store.activate(); | 62 | store.activate(); |
| 53 | } | 63 | } |
| 54 | 64 | ||
| 55 | @After | 65 | @After |
| 56 | - public void tearDown() { | 66 | + public void tearDown() throws IOException { |
| 67 | + if (new File(ROOT).exists()) { | ||
| 68 | + Tools.removeDirectory(ROOT); | ||
| 69 | + } | ||
| 57 | store.deactivate(); | 70 | store.deactivate(); |
| 58 | } | 71 | } |
| 59 | 72 | ||
| 60 | private Application createTestApp() { | 73 | private Application createTestApp() { |
| 74 | + synchronized (LOCK) { | ||
| 61 | return store.create(ApplicationArchive.class.getResourceAsStream("app.zip")); | 75 | return store.create(ApplicationArchive.class.getResourceAsStream("app.zip")); |
| 62 | } | 76 | } |
| 77 | + } | ||
| 63 | 78 | ||
| 64 | @Test | 79 | @Test |
| 65 | public void create() { | 80 | public void create() { |
| ... | @@ -132,4 +147,11 @@ public class SimpleApplicationStoreTest { | ... | @@ -132,4 +147,11 @@ public class SimpleApplicationStoreTest { |
| 132 | this.event = event; | 147 | this.event = event; |
| 133 | } | 148 | } |
| 134 | } | 149 | } |
| 150 | + | ||
| 151 | + private class TestApplicationStore extends SimpleApplicationStore { | ||
| 152 | + @Override | ||
| 153 | + public void setRootPath(String root) { | ||
| 154 | + super.setRootPath(root); | ||
| 155 | + } | ||
| 156 | + } | ||
| 135 | } | 157 | } |
| ... | \ No newline at end of file | ... | \ No newline at end of file | ... | ... |
| 1 | +/* | ||
| 2 | + * Copyright 2015 Open Networking Laboratory | ||
| 3 | + * | ||
| 4 | + * Licensed under the Apache License, Version 2.0 (the "License"); | ||
| 5 | + * you may not use this file except in compliance with the License. | ||
| 6 | + * You may obtain a copy of the License at | ||
| 7 | + * | ||
| 8 | + * http://www.apache.org/licenses/LICENSE-2.0 | ||
| 9 | + * | ||
| 10 | + * Unless required by applicable law or agreed to in writing, software | ||
| 11 | + * distributed under the License is distributed on an "AS IS" BASIS, | ||
| 12 | + * WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied. | ||
| 13 | + * See the License for the specific language governing permissions and | ||
| 14 | + * limitations under the License. | ||
| 15 | + */ | ||
| 16 | +package org.onosproject.rest.exceptions; | ||
| 17 | + | ||
| 18 | +import javax.ws.rs.core.Response; | ||
| 19 | + | ||
| 20 | +/** | ||
| 21 | + * Mapper for illegal state exceptions to the BAD_REQUEST response code. | ||
| 22 | + */ | ||
| 23 | +public class IllegalStateExceptionMapper extends AbstractMapper<IllegalStateException> { | ||
| 24 | + @Override | ||
| 25 | + protected Response.Status responseStatus() { | ||
| 26 | + return Response.Status.CONFLICT; | ||
| 27 | + } | ||
| 28 | +} | ||
| 29 | + |
| ... | @@ -62,6 +62,7 @@ | ... | @@ -62,6 +62,7 @@ |
| 62 | org.onosproject.rest.exceptions.BadRequestMapper, | 62 | org.onosproject.rest.exceptions.BadRequestMapper, |
| 63 | org.onosproject.rest.exceptions.WebApplicationExceptionMapper, | 63 | org.onosproject.rest.exceptions.WebApplicationExceptionMapper, |
| 64 | org.onosproject.rest.exceptions.IllegalArgumentExceptionMapper, | 64 | org.onosproject.rest.exceptions.IllegalArgumentExceptionMapper, |
| 65 | + org.onosproject.rest.exceptions.IllegalStateExceptionMapper, | ||
| 65 | org.onosproject.rest.resources.JsonBodyWriter, | 66 | org.onosproject.rest.resources.JsonBodyWriter, |
| 66 | 67 | ||
| 67 | org.onosproject.rest.resources.ApplicationsWebResource, | 68 | org.onosproject.rest.resources.ApplicationsWebResource, | ... | ... |
-
Please register or login to post a comment