Skip to content

HTTPS clone URL

Subversion checkout URL

You can clone with HTTPS or Subversion.

Download ZIP

Loading…

NEXUS-4878 - Add reason to not deletable error #340

Merged
merged 6 commits into from

4 participants

...figuration/application/DefaultNexusConfiguration.java
@@ -811,8 +811,8 @@ public synchronized void deleteRepository( String id )
if ( repository.getId().equals( shadow.getMasterRepository().getId() ) )
{
- throw new ConfigurationException( "The repository with ID " + id
- + " is not deletable, it has dependant repositories!" );
+ throw new ConfigurationException( "The repository '" + repository.getName()
+ + "' is not deletable cause '" + shadow.getName() + "' depends on it!" );
}
@peterlynch Owner

Since this exception gets logged for administrators, I think it is important to retain the mention of the repo ID value. Both name and id wouldn't hurt though.

Suggest this should read something more like:

"Repository [id= + id + ",name=" + repository.getName() + "] + "' cannot be deleted due to dependencies: shadow repository[id=" + shadow.getId() + ",name=" +shadow.getName()+"]."

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@peterlynch peterlynch commented on the diff
...nexus/rest/repositories/RepositoryPlexusResource.java
@@ -379,10 +379,9 @@ public void delete( Context context, Request request, Response response )
}
catch ( ConfigurationException e )
{
- getLogger().warn( "Repository not deletable, it has dependants, id=" + repoId );
+ getLogger().warn( e.getMessage() );
@peterlynch Owner

Perhaps creating a more specific ConfigurationException here so in this case it can be logged as INFO?

@velo
velo added a note

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...ain/webapp/js/repoServer/repoServer.RepoMaintPanel.js
@@ -276,6 +276,27 @@ Sonatype.repoServer.RepositoryPanel = function(config) {
Ext.extend(Sonatype.repoServer.RepositoryPanel, Sonatype.panels.GridViewer, {
+ deleteRecord : function(rec) {
+ Ext.Ajax.request({
+ callback : function(options, success, response) {
+ if (success)
+ {
+ this.dataStore.remove(rec);
+ }
+ else
+ {
+ var options = {
+ hideErrorStatus : true
+ };
+ Sonatype.utils.connectionError(response, 'Delete Failed!', false, options);
@peterlynch Owner

Prefer something like:

"Delete request could not be completed."

"Repository XXXX cannot be deleted due to dependencies on shadow repository YYYY."
"Dependencies must be removed in order to complete this operation."

@velo
velo added a note

FWIW, http://screencast.com/t/rtsdXnkjr

it does show the exception content on UI.

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@velo

Applied requested changes.

@nabcos

RepositoryDependendantException is not checked in.

...nexus/rest/repositories/RepositoryPlexusResource.java
@@ -32,6 +32,7 @@
import org.restlet.resource.ResourceException;
import org.restlet.resource.Variant;
import org.sonatype.configuration.ConfigurationException;
+import org.sonatype.nexus.configuration.application.RepositoryDependantException;
@peterlynch Owner

Dependant is a variant of Dependent - use Dependent spelling instead

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@velo

Ouch, fixed.

...uration/application/RepositoryDependentException.java
((5 lines not shown))
+import org.sonatype.configuration.ConfigurationException;
+import org.sonatype.nexus.proxy.repository.Repository;
+
+public class RepositoryDependentException
+ extends ConfigurationException
+{
+
+ private static final long serialVersionUID = -2037859093869479166L;
+
+ private final Repository dependant;
+
+ private final Repository repository;
+
+ public RepositoryDependentException( Repository repository, Repository dependant )
+ {
+ super( format( "Repository [id=%s, name=%s] cannot be deleted due to dependency: repository[id=%s, name=%s].",
@cstamas Owner
cstamas added a note

see org.sonatype.nexus.proxy.utils.RepositoryStringUtils.getHumanizedNameString(Repository)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
...uration/application/RepositoryDependentException.java
((24 lines not shown))
+ this.dependant = dependant;
+ }
+
+ public Repository getDependant()
+ {
+ return dependant;
+ }
+
+ public Repository getRepository()
+ {
+ return repository;
+ }
+
+ public String getUIMessage()
+ {
+ return format( "Repository '%s' cannot be deleted due to dependencies on repository '%s'.\n"
@cstamas Owner
cstamas added a note

see org.sonatype.nexus.proxy.utils.RepositoryStringUtils.getHumanizedNameString(Repository)

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
@cstamas
Owner

Method org.sonatype.nexus.proxy.utils.RepositoryStringUtils.getHumanizedNameString(Repository) should be used in cases like this, to retain same formatting in logs (as almost all the logs already uses it).

@velo

Done

@peterlynch
Owner

+1 update the screenshot if you have time - otherwise I'll do it during testing

@nabcos

+1

@velo velo merged commit d4ac9e8 into from
Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
This page is out of date. Refresh to see the latest.
View
3  nexus/nexus-app/src/main/java/org/sonatype/nexus/configuration/application/DefaultNexusConfiguration.java
@@ -811,8 +811,7 @@ public synchronized void deleteRepository( String id )
if ( repository.getId().equals( shadow.getMasterRepository().getId() ) )
{
- throw new ConfigurationException( "The repository with ID " + id
- + " is not deletable, it has dependant repositories!" );
+ throw new RepositoryDependentException( repository, shadow );
}
}
View
44 ...s/nexus-app/src/main/java/org/sonatype/nexus/configuration/application/RepositoryDependentException.java
@@ -0,0 +1,44 @@
+package org.sonatype.nexus.configuration.application;
+
+import static java.lang.String.format;
+import static org.sonatype.nexus.proxy.utils.RepositoryStringUtils.getHumanizedNameString;
+
+import org.sonatype.configuration.ConfigurationException;
+import org.sonatype.nexus.proxy.repository.Repository;
+
+public class RepositoryDependentException
+ extends ConfigurationException
+{
+
+ private static final long serialVersionUID = -2037859093869479166L;
+
+ private final Repository dependant;
+
+ private final Repository repository;
+
+ public RepositoryDependentException( Repository repository, Repository dependant )
+ {
+ super( format( "Repository %s cannot be deleted due to dependency: repository %s.",
+ getHumanizedNameString( repository ), getHumanizedNameString( dependant ) ) );
+ this.repository = repository;
+ this.dependant = dependant;
+ }
+
+ public Repository getDependant()
+ {
+ return dependant;
+ }
+
+ public Repository getRepository()
+ {
+ return repository;
+ }
+
+ public String getUIMessage()
+ {
+ return format(
+ "Repository '%s' cannot be deleted due to dependencies on repository '%s'.\nDependencies must be removed in order to complete this operation.",
+ getHumanizedNameString( repository ), getHumanizedNameString( dependant ) );
+ }
+
+}
View
12 nexus/nexus-rest-api/src/main/java/org/sonatype/nexus/rest/repositories/RepositoryPlexusResource.java
@@ -32,6 +32,7 @@
import org.restlet.resource.ResourceException;
import org.restlet.resource.Variant;
import org.sonatype.configuration.ConfigurationException;
+import org.sonatype.nexus.configuration.application.RepositoryDependentException;
import org.sonatype.nexus.proxy.AccessDeniedException;
import org.sonatype.nexus.proxy.NoSuchRepositoryException;
import org.sonatype.nexus.proxy.StorageException;
@@ -377,12 +378,17 @@ public void delete( Context context, Request request, Response response )
response.setStatus( Status.SUCCESS_NO_CONTENT );
}
+ catch ( RepositoryDependentException e )
+ {
+ getLogger().info( e.getMessage() );
+
+ throw new ResourceException( Status.CLIENT_ERROR_BAD_REQUEST, e.getUIMessage(), e );
+ }
catch ( ConfigurationException e )
{
- getLogger().warn( "Repository not deletable, it has dependants, id=" + repoId );
+ getLogger().warn( e.getMessage() );
@peterlynch Owner

Perhaps creating a more specific ConfigurationException here so in this case it can be logged as INFO?

@velo
velo added a note

Done

Sign up for free to join this conversation on GitHub. Already have an account? Sign in to comment
- throw new ResourceException( Status.CLIENT_ERROR_BAD_REQUEST,
- "Repository is not deletable, it has dependants." );
+ throw new ResourceException( Status.CLIENT_ERROR_BAD_REQUEST, e.getMessage(), e );
}
catch ( NoSuchRepositoryAccessException e )
{
View
21 nexus/nexus-webapp/src/main/webapp/js/repoServer/repoServer.RepoMaintPanel.js
@@ -276,6 +276,27 @@ Sonatype.repoServer.RepositoryPanel = function(config) {
Ext.extend(Sonatype.repoServer.RepositoryPanel, Sonatype.panels.GridViewer, {
+ deleteRecord : function(rec) {
+ Ext.Ajax.request({
+ callback : function(options, success, response) {
+ if (success)
+ {
+ this.dataStore.remove(rec);
+ }
+ else
+ {
+ var options = {
+ hideErrorStatus : true
+ };
+ Sonatype.utils.connectionError(response, 'Delete request could not be completed!', false, options);
+ }
+ },
+ scope : this,
+ method : 'DELETE',
+ url : rec.data.resourceURI
+ });
+ },
+
applyBookmark : function(bookmark) {
this.updatingBookmark = true;
if (this.groupStore.lastOptions == null)
Something went wrong with that request. Please try again.