keycloak-uncached

For overwrite, do all deletes, then all adds. Minor UI enhancements. Fix

1/8/2016 4:45:22 PM

Changes

Details

diff --git a/core/src/main/java/org/keycloak/representations/idm/PartialImportRepresentation.java b/core/src/main/java/org/keycloak/representations/idm/PartialImportRepresentation.java
index c27000d..9b8c2ae 100644
--- a/core/src/main/java/org/keycloak/representations/idm/PartialImportRepresentation.java
+++ b/core/src/main/java/org/keycloak/representations/idm/PartialImportRepresentation.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -21,9 +21,9 @@ import java.util.List;
 import org.codehaus.jackson.annotate.JsonIgnoreProperties;
 
 /**
- * Used for partial import of users, clients, and identity providers.
+ * Used for partial import of users, clients, roles, and identity providers.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 @JsonIgnoreProperties(ignoreUnknown=true)
 public class PartialImportRepresentation {
@@ -49,11 +49,11 @@ public class PartialImportRepresentation {
     }
 
     public boolean hasRealmRoles() {
-        return (roles.getRealm() != null) && (!roles.getRealm().isEmpty());
+        return (roles != null) && (roles.getRealm() != null) && (!roles.getRealm().isEmpty());
     }
 
     public boolean hasClientRoles() {
-        return (roles.getClient() != null) && (!roles.getClient().isEmpty());
+        return (roles != null) && (roles.getClient() != null) && (!roles.getClient().isEmpty());
     }
 
     public String getIfResourceExists() {
diff --git a/docbook/auth-server-docs/reference/en/en-US/modules/export-import.xml b/docbook/auth-server-docs/reference/en/en-US/modules/export-import.xml
index fbd6016..6ec8660 100755
--- a/docbook/auth-server-docs/reference/en/en-US/modules/export-import.xml
+++ b/docbook/auth-server-docs/reference/en/en-US/modules/export-import.xml
@@ -1,114 +1,134 @@
 <chapter id="export-import">
     <title>Export and Import</title>
-    <para>
-        Export/import is useful especially if you want to migrate your whole Keycloak database from one environment to another or migrate to different database (For example from MySQL to Oracle).
-        You can trigger export/import at startup of Keycloak server and it's configurable with System properties right now. The fact it's done at server startup means that no-one can access Keycloak UI or REST endpoints
-        and edit Keycloak database on the fly when export or import is in progress. Otherwise it could lead to inconsistent results.
-    </para>
-    <para>
-        You can export/import your database either to:
-        <itemizedlist>
-            <listitem>Directory on local filesystem</listitem>
-            <listitem>Single JSON file on your filesystem</listitem>
-        </itemizedlist>
+    <section>
+        <title>Startup export/import</title>
+        <para>
+            Export/import is useful especially if you want to migrate your whole Keycloak database from one environment to another or migrate to different database (For example from MySQL to Oracle).
+            You can trigger export/import at startup of Keycloak server and it's configurable with System properties right now. The fact it's done at server startup means that no-one can access Keycloak UI or REST endpoints
+            and edit Keycloak database on the fly when export or import is in progress. Otherwise it could lead to inconsistent results.
+        </para>
+        <para>
+            You can export/import your database either to:
+            <itemizedlist>
+                <listitem>Directory on local filesystem</listitem>
+                <listitem>Single JSON file on your filesystem</listitem>
+            </itemizedlist>
 
-        When importing using the "dir" strategy, note that the files need to follow the naming convention specified below.
-        If you are importing files which were previously exported, the files already follow this convention.
-        <itemizedlist>
-            <listitem>{REALM_NAME}-realm.json, such as "acme-roadrunner-affairs-realm.json" for the realm named "acme-roadrunner-affairs"</listitem>
-            <listitem>{REALM_NAME}-users-{INDEX}.json, such as "acme-roadrunner-affairs-users-0.json" for the first users file of the realm named "acme-roadrunner-affairs"</listitem>
-        </itemizedlist>
-    </para>
-    <para>
-        If you import to Directory, you can specify also the number of users to be stored in each JSON file. So if you have
-        very large amount of users in your database, you likely don't want to import them into single file as the file might be very big.
-        Processing of each file is done in separate transaction as exporting/importing all users at once could also lead to memory issues.
-    </para>
-    <para>
-        To export into unencrypted directory you can use:
-        <programlisting><![CDATA[
+            When importing using the "dir" strategy, note that the files need to follow the naming convention specified below.
+            If you are importing files which were previously exported, the files already follow this convention.
+            <itemizedlist>
+                <listitem>{REALM_NAME}-realm.json, such as "acme-roadrunner-affairs-realm.json" for the realm named "acme-roadrunner-affairs"</listitem>
+                <listitem>{REALM_NAME}-users-{INDEX}.json, such as "acme-roadrunner-affairs-users-0.json" for the first users file of the realm named "acme-roadrunner-affairs"</listitem>
+            </itemizedlist>
+        </para>
+        <para>
+            If you import to Directory, you can specify also the number of users to be stored in each JSON file. So if you have
+            very large amount of users in your database, you likely don't want to import them into single file as the file might be very big.
+            Processing of each file is done in separate transaction as exporting/importing all users at once could also lead to memory issues.
+        </para>
+        <para>
+            To export into unencrypted directory you can use:
+            <programlisting><![CDATA[
 bin/standalone.sh -Dkeycloak.migration.action=export
 -Dkeycloak.migration.provider=dir -Dkeycloak.migration.dir=<DIR TO EXPORT TO>
 ]]></programlisting>
-        And similarly for import just use <literal>-Dkeycloak.migration.action=import</literal> instead of <literal>export</literal> .
-    </para>
-    <para>
-        To export into single JSON file you can use:
-        <programlisting><![CDATA[
+            And similarly for import just use <literal>-Dkeycloak.migration.action=import</literal> instead of <literal>export</literal> .
+        </para>
+        <para>
+            To export into single JSON file you can use:
+            <programlisting><![CDATA[
 bin/standalone.sh -Dkeycloak.migration.action=export
 -Dkeycloak.migration.provider=singleFile -Dkeycloak.migration.file=<FILE TO EXPORT TO>
 ]]></programlisting>
-    </para>
-    <para>
-        Here's an example of importing:
-        <programlisting><![CDATA[
+        </para>
+        <para>
+            Here's an example of importing:
+            <programlisting><![CDATA[
 bin/standalone.sh -Dkeycloak.migration.action=import
 -Dkeycloak.migration.provider=singleFile -Dkeycloak.migration.file=<FILE TO IMPORT>
 -Dkeycloak.migration.strategy=OVERWRITE_EXISTING
 ]]></programlisting>
-    </para>
-    <para>
-        Other available options are:
-        <variablelist>
-            <varlistentry>
-                <term>-Dkeycloak.migration.realmName</term>
-                <listitem>
-                    <para>
-                        can be used if you want to export just one specified realm instead of all.
-                        If not specified, then all realms will be exported.
-                    </para>
-                </listitem>
-            </varlistentry>
-            <varlistentry>
-                <term>-Dkeycloak.migration.usersExportStrategy</term>
-                <listitem>
-                    <para>
-                        can be used to specify for Directory providers to specify where to import users.
-                        Possible values are:
-                        <itemizedlist>
-                            <listitem>DIFFERENT_FILES - Users will be exported into more different files according to maximum number of users per file. This is default value</listitem>
-                            <listitem>SKIP - exporting of users will be skipped completely</listitem>
-                            <listitem>REALM_FILE - All users will be exported to same file with realm (So file like "foo-realm.json" with both realm data and users)</listitem>
-                            <listitem>SAME_FILE - All users will be exported to same file but different than realm (So file like "foo-realm.json" with realm data and "foo-users.json" with users)</listitem>
-                        </itemizedlist>
-                    </para>
-                </listitem>
-            </varlistentry>
-            <varlistentry>
-                <term>-Dkeycloak.migration.usersPerFile</term>
-                <listitem>
-                    <para>
-                        can be used to specify number of users per file (and also per DB transaction).
-                        It's 5000 by default. It's used only if usersExportStrategy is DIFFERENT_FILES
-                    </para>
-                </listitem>
-            </varlistentry>
-            <varlistentry>
-                <term>-Dkeycloak.migration.strategy</term>
-                <listitem>
-                    <para>
-                        is used during import. It can be used to specify how to proceed if realm with same name
-                        already exists in the database where you are going to import data. Possible values are:
-                        <itemizedlist>
-                            <listitem>IGNORE_EXISTING - Ignore importing if realm of this name already exists</listitem>
-                            <listitem>OVERWRITE_EXISTING - Remove existing realm and import it again with new data from JSON file.
-                                If you want to fully migrate one environment to another and ensure that the new environment will contain same data
-                                like the old one, you can specify this.
-                            </listitem>
-                        </itemizedlist>
-                    </para>
-                </listitem>
-            </varlistentry>
-        </variablelist>
-    </para>
-    <para>
-        When importing realm files that weren't exported before, the option <literal>keycloak.import</literal> can be used. If more than one realm
-        file needs to be imported, a comma separated list of file names can be specified. This is more appropriate than the cases before, as this
-        will happen only after the master realm has been initialized. Examples:
-        <itemizedlist>
-            <listitem>-Dkeycloak.import=/tmp/realm1.json</listitem>
-            <listitem>-Dkeycloak.import=/tmp/realm1.json,/tmp/realm2.json</listitem>
-        </itemizedlist>
-    </para>
-
+        </para>
+        <para>
+            Other available options are:
+            <variablelist>
+                <varlistentry>
+                    <term>-Dkeycloak.migration.realmName</term>
+                    <listitem>
+                        <para>
+                            can be used if you want to export just one specified realm instead of all.
+                            If not specified, then all realms will be exported.
+                        </para>
+                    </listitem>
+                </varlistentry>
+                <varlistentry>
+                    <term>-Dkeycloak.migration.usersExportStrategy</term>
+                    <listitem>
+                        <para>
+                            can be used to specify for Directory providers to specify where to import users.
+                            Possible values are:
+                            <itemizedlist>
+                                <listitem>DIFFERENT_FILES - Users will be exported into more different files according to maximum number of users per file. This is default value</listitem>
+                                <listitem>SKIP - exporting of users will be skipped completely</listitem>
+                                <listitem>REALM_FILE - All users will be exported to same file with realm (So file like "foo-realm.json" with both realm data and users)</listitem>
+                                <listitem>SAME_FILE - All users will be exported to same file but different than realm (So file like "foo-realm.json" with realm data and "foo-users.json" with users)</listitem>
+                            </itemizedlist>
+                        </para>
+                    </listitem>
+                </varlistentry>
+                <varlistentry>
+                    <term>-Dkeycloak.migration.usersPerFile</term>
+                    <listitem>
+                        <para>
+                            can be used to specify number of users per file (and also per DB transaction).
+                            It's 5000 by default. It's used only if usersExportStrategy is DIFFERENT_FILES
+                        </para>
+                    </listitem>
+                </varlistentry>
+                <varlistentry>
+                    <term>-Dkeycloak.migration.strategy</term>
+                    <listitem>
+                        <para>
+                            is used during import. It can be used to specify how to proceed if realm with same name
+                            already exists in the database where you are going to import data. Possible values are:
+                            <itemizedlist>
+                                <listitem>IGNORE_EXISTING - Ignore importing if realm of this name already exists</listitem>
+                                <listitem>OVERWRITE_EXISTING - Remove existing realm and import it again with new data from JSON file.
+                                    If you want to fully migrate one environment to another and ensure that the new environment will contain same data
+                                    like the old one, you can specify this.
+                                </listitem>
+                            </itemizedlist>
+                        </para>
+                    </listitem>
+                </varlistentry>
+            </variablelist>
+        </para>
+        <para>
+            When importing realm files that weren't exported before, the option <literal>keycloak.import</literal> can be used. If more than one realm
+            file needs to be imported, a comma separated list of file names can be specified. This is more appropriate than the cases before, as this
+            will happen only after the master realm has been initialized. Examples:
+            <itemizedlist>
+                <listitem>-Dkeycloak.import=/tmp/realm1.json</listitem>
+                <listitem>-Dkeycloak.import=/tmp/realm1.json,/tmp/realm2.json</listitem>
+            </itemizedlist>
+        </para>
+    </section>
+    <section>
+        <title>Admin console export/import</title>
+        <para>
+            Import of most resources can be performed from the admin console.
+            Exporting resources will be supported in future versions.
+        </para>
+        <para>
+            The files created during a "startup" export can be used to import from
+            the admin UI.  This way, you can export from one realm and import to
+            another realm.  Or, you can export from one server and import to another.
+        </para>
+        <warning>
+            <para>
+                The admin console import allows you to "overwrite" resources if you choose.
+                Use this feature with caution, especially on a production system.
+            </para>
+        </warning>
+    </section>
 </chapter>
\ No newline at end of file
diff --git a/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js b/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js
index 4b73b74..a3c99e8 100755
--- a/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js
+++ b/forms/common-themes/src/main/resources/theme/base/admin/resources/js/controllers/realm.js
@@ -2229,6 +2229,17 @@ module.controller('RealmImportCtrl', function($scope, realm, $route,
         }
     }, true);
     
+    $scope.successMessage = function() {
+        var message = $scope.results.added + ' records added. ';
+        if ($scope.ifResourceExists === 'SKIP') {
+            message += $scope.results.skipped + ' records skipped.'
+        }
+        if ($scope.ifResourceExists === 'OVERWRITE') {
+            message += $scope.results.overwritten + ' records overwritten.';
+        }
+        return message;
+    }
+    
     $scope.save = function() {
         var json = angular.copy($scope.fileContent);
         json.ifResourceExists = $scope.ifResourceExists;
@@ -2243,14 +2254,7 @@ module.controller('RealmImportCtrl', function($scope, realm, $route,
         
         var importFile = $resource(authUrl + '/admin/realms/' + realm.realm + '/partialImport');
         $scope.results = importFile.save(json, function() {
-            var message = $scope.results.added + ' records added. ';
-            if ($scope.ifResourceExists === 'SKIP') {
-                message += $scope.results.skipped + ' records skipped.'
-            }
-            if ($scope.ifResourceExists === 'OVERWRITE') {
-                message += $scope.results.overwritten + ' records overwritten.';
-            }
-            Notifications.success(message);
+            Notifications.success($scope.successMessage());
         }, function(error) {
             if (error.data.errorMessage) {
                 Notifications.error(error.data.errorMessage);
diff --git a/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/partial-import.html b/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/partial-import.html
index 26301c3..d15cd71 100644
--- a/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/partial-import.html
+++ b/forms/common-themes/src/main/resources/theme/base/admin/resources/partials/partial-import.html
@@ -86,6 +86,7 @@
         </div>
 
         <div class="form-group" data-ng-show="hasResults()">
+            {{successMessage()}}
             <table class="table table-striped table-bordered">
                 <thead>
                     <tr>
@@ -95,26 +96,23 @@
                         <th>Id</th>
                     </tr>
                 </thead>
-                <tfoot>
-                    <tr>
-                        <td>
-                            <div class="table-nav">
-                                <button data-ng-click="setFirstPage()" class="first" ng-disabled="">First page</button>
-                                <button data-ng-click="setPreviousPage()" class="prev" ng-disabled="!hasPrevious()">Previous page</button>
-                                <button data-ng-click="setNextPage()" class="next" ng-disabled="!hasNext()">Next page</button>
-                            </div>
-                        </td>
-                    </tr>
-                </tfoot>
                 <tbody>
                     <tr ng-repeat="result in resultsPage()" >
-                        <td>{{result.action}}</td>
+                        <td ng-show="result.action == 'OVERWRITTEN'"><span class="label label-danger">{{result.action}}</span></td>
+                        <td ng-show="result.action == 'SKIPPED'"><span class="label label-warning">{{result.action}}</span></td>
+                        <td ng-show="result.action == 'ADDED'"><span class="label label-success">{{result.action}}</span></td>
                         <td>{{result.resourceType}}</td>
                         <td>{{result.resourceName}}</td>
                         <td>{{result.id}}</td>
                     </tr>
                 </tbody>
             </table>
+            
+            <div class="table-nav">
+                <button data-ng-click="setFirstPage()" class="first" ng-disabled="">First page</button>
+                <button data-ng-click="setPreviousPage()" class="prev" ng-disabled="!hasPrevious()">Previous page</button>
+                <button data-ng-click="setNextPage()" class="next" ng-disabled="!hasNext()">Next page</button>
+            </div>
         </div>
     </form>
 </div>
diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java
index 36fd1fd..f44abe8 100755
--- a/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java
+++ b/model/jpa/src/main/java/org/keycloak/models/jpa/JpaUserProvider.java
@@ -76,9 +76,12 @@ public class JpaUserProvider implements UserProvider {
                 userModel.joinGroup(g);
             }
         }
-        for (RequiredActionProviderModel r : realm.getRequiredActionProviders()) {
-            if (r.isEnabled() && r.isDefaultAction()) {
-                userModel.addRequiredAction(r.getAlias());
+
+        if (addDefaultRequiredActions){
+            for (RequiredActionProviderModel r : realm.getRequiredActionProviders()) {
+                if (r.isEnabled() && r.isDefaultAction()) {
+                    userModel.addRequiredAction(r.getAlias());
+                }
             }
         }
 
diff --git a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java
index ce72bba..d5be946 100755
--- a/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java
+++ b/model/jpa/src/main/java/org/keycloak/models/jpa/RealmAdapter.java
@@ -1059,6 +1059,7 @@ public class RealmAdapter implements RealmModel {
         em.createNamedQuery("deleteGroupRoleMappingsByRole").setParameter("roleId", roleEntity.getId()).executeUpdate();
 
         em.remove(roleEntity);
+        em.flush();
 
         return true;
     }
@@ -1217,7 +1218,7 @@ public class RealmAdapter implements RealmModel {
         realm.setEventsListeners(listeners);
         em.flush();
     }
-    
+
     @Override
     public Set<String> getEnabledEventTypes() {
         return realm.getEnabledEventTypes();
@@ -1228,7 +1229,7 @@ public class RealmAdapter implements RealmModel {
         realm.setEnabledEventTypes(enabledEventTypes);
         em.flush();
     }
-    
+
     @Override
     public boolean isAdminEventsEnabled() {
         return realm.isAdminEventsEnabled();
@@ -1250,7 +1251,7 @@ public class RealmAdapter implements RealmModel {
         realm.setAdminEventsDetailsEnabled(enabled);
         em.flush();
     }
-    
+
     @Override
     public ClientModel getMasterAdminClient() {
         ClientEntity masterAdminClient = realm.getMasterAdminClient();
diff --git a/services/src/main/java/org/keycloak/partialimport/AbstractPartialImport.java b/services/src/main/java/org/keycloak/partialimport/AbstractPartialImport.java
index ad4c203..33f9c12 100644
--- a/services/src/main/java/org/keycloak/partialimport/AbstractPartialImport.java
+++ b/services/src/main/java/org/keycloak/partialimport/AbstractPartialImport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -21,16 +21,19 @@ import java.util.HashSet;
 import java.util.List;
 import java.util.Set;
 import javax.ws.rs.core.Response;
+import org.jboss.logging.Logger;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.representations.idm.PartialImportRepresentation;
 import org.keycloak.services.ErrorResponse;
 
 /**
+ * Base PartialImport for most resource types.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public abstract class AbstractPartialImport<T> implements PartialImport<T> {
+    protected static Logger logger = Logger.getLogger(AbstractPartialImport.class);
 
     protected final Set<T> toOverwrite = new HashSet<>();
     protected final Set<T> toSkip = new HashSet<>();
@@ -41,7 +44,7 @@ public abstract class AbstractPartialImport<T> implements PartialImport<T> {
     public abstract boolean exists(RealmModel realm, KeycloakSession session, T resourceRep);
     public abstract String existsMessage(T resourceRep);
     public abstract ResourceType getResourceType();
-    public abstract void overwrite(RealmModel realm, KeycloakSession session, T resourceRep);
+    public abstract void remove(RealmModel realm, KeycloakSession session, T resourceRep);
     public abstract void create(RealmModel realm, KeycloakSession session, T resourceRep);
 
     @Override
@@ -67,29 +70,36 @@ public abstract class AbstractPartialImport<T> implements PartialImport<T> {
         return new ErrorResponseException(error);
     }
 
-    public PartialImportResult overwritten(String modelId, T resourceRep){
+    protected PartialImportResult overwritten(String modelId, T resourceRep){
         return PartialImportResult.overwritten(getResourceType(), getName(resourceRep), modelId, resourceRep);
     }
 
-    public PartialImportResult skipped(String modelId, T resourceRep) {
+    protected PartialImportResult skipped(String modelId, T resourceRep) {
         return PartialImportResult.skipped(getResourceType(), getName(resourceRep), modelId, resourceRep);
     }
 
-    public PartialImportResult added(String modelId, T resourceRep) {
+    protected PartialImportResult added(String modelId, T resourceRep) {
         return PartialImportResult.added(getResourceType(), getName(resourceRep), modelId, resourceRep);
     }
 
     @Override
+    public void removeOverwrites(RealmModel realm, KeycloakSession session) {
+        for (T resourceRep : toOverwrite) {
+            remove(realm, session, resourceRep);
+        }
+    }
+
+    @Override
     public PartialImportResults doImport(PartialImportRepresentation partialImportRep, RealmModel realm, KeycloakSession session) throws ErrorResponseException {
         PartialImportResults results = new PartialImportResults();
         List<T> repList = getRepList(partialImportRep);
         if ((repList == null) || repList.isEmpty()) return results;
 
-        for (T resourceRep: toOverwrite) {
-            //System.out.println("overwriting " + getResourceType() + " " + getName(resourceRep));
+        for (T resourceRep : toOverwrite) {
             try {
-                overwrite(realm, session, resourceRep);
+                create(realm, session, resourceRep);
             } catch (Exception e) {
+                logger.error("Error overwriting " + getName(resourceRep), e);
                 throw new ErrorResponseException(ErrorResponse.error(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR));
             }
 
@@ -98,7 +108,6 @@ public abstract class AbstractPartialImport<T> implements PartialImport<T> {
         }
 
         for (T resourceRep : toSkip) {
-            //System.out.println("skipping " + getResourceType() + " " + getName(resourceRep));
             String modelId = getModelId(realm, session, resourceRep);
             results.addResult(skipped(modelId, resourceRep));
         }
@@ -108,12 +117,11 @@ public abstract class AbstractPartialImport<T> implements PartialImport<T> {
             if (toSkip.contains(resourceRep)) continue;
 
             try {
-                //System.out.println("adding " + getResourceType() + " " + getName(resourceRep));
                 create(realm, session, resourceRep);
                 String modelId = getModelId(realm, session, resourceRep);
                 results.addResult(added(modelId, resourceRep));
             } catch (Exception e) {
-                //e.printStackTrace();
+                logger.error("Error creating " + getName(resourceRep), e);
                 throw new ErrorResponseException(ErrorResponse.error(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR));
             }
         }
diff --git a/services/src/main/java/org/keycloak/partialimport/Action.java b/services/src/main/java/org/keycloak/partialimport/Action.java
index cead470..86ea54f 100644
--- a/services/src/main/java/org/keycloak/partialimport/Action.java
+++ b/services/src/main/java/org/keycloak/partialimport/Action.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -18,10 +18,10 @@
 package org.keycloak.partialimport;
 
 /**
+ * Enum for actions taken by PartialImport.
  *
- * @author ssilvert
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public enum Action {
     ADDED, SKIPPED, OVERWRITTEN
-
 }
diff --git a/services/src/main/java/org/keycloak/partialimport/ClientRolesPartialImport.java b/services/src/main/java/org/keycloak/partialimport/ClientRolesPartialImport.java
index 00a6c25..d309b51 100644
--- a/services/src/main/java/org/keycloak/partialimport/ClientRolesPartialImport.java
+++ b/services/src/main/java/org/keycloak/partialimport/ClientRolesPartialImport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -32,10 +32,11 @@ import org.keycloak.representations.idm.RoleRepresentation;
 import org.keycloak.services.ErrorResponse;
 
 /**
+ * Partial Import handler for Client Roles.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
-public class ClientRolesPartialImport implements PartialImport<RoleRepresentation> {
+public class ClientRolesPartialImport {
     private final Map<String, Set<RoleRepresentation>> toOverwrite = new HashMap<>();
     private final Map<String, Set<RoleRepresentation>> toSkip = new HashMap<>();
 
@@ -94,44 +95,16 @@ public class ClientRolesPartialImport implements PartialImport<RoleRepresentatio
         return ResourceType.CLIENT_ROLE;
     }
 
-    public void overwrite(RealmModel realm, KeycloakSession session, String clientId, RoleRepresentation roleRep) {
-        ClientModel client = realm.getClientByClientId(clientId);
-        checkForComposite(roleRep);
-        RoleModel role = client.getRole(getName(roleRep));
-        checkForOverwriteComposite(role);
-        RealmRolesPartialImport.RoleHelper helper = new RealmRolesPartialImport.RoleHelper(realm);
-//        helper.updateRole(roleRep, role);
-    }
-
     public void deleteRole(RealmModel realm, String clientId, RoleRepresentation roleRep) {
         ClientModel client = realm.getClientByClientId(clientId);
-        RoleModel role = client.getRole(getName(roleRep));
-        client.removeRole(role);
-    }
-
-    private void checkForComposite(RoleRepresentation roleRep) {
-        if (roleRep.isComposite()) {
-            throw new IllegalArgumentException("Composite role '" + getName(roleRep) + "' can not be partially imported");
-        }
-    }
-
-    private void checkForOverwriteComposite(RoleModel role) {
-        if (role.isComposite()) {
-            throw new IllegalArgumentException("Composite role '" + role.getName() + "' can not be overwritten.");
-        }
-    }
-
-    public void create(RealmModel realm, KeycloakSession session, String clientId, RoleRepresentation roleRep) {
-        ClientModel client = realm.getClientByClientId(clientId);
         if (client == null) {
-            throw new IllegalStateException("Client '" + clientId + "' does not exist for client role " + getName(roleRep));
+            // client might have been removed as part of this partial import
+            return;
         }
-        checkForComposite(roleRep);
-        client.addRole(getName(roleRep));
-        overwrite(realm, session, clientId, roleRep);
+        RoleModel role = client.getRole(getName(roleRep));
+        client.removeRole(role);
     }
 
-    @Override
     public void prepare(PartialImportRepresentation partialImportRep, RealmModel realm, KeycloakSession session) throws ErrorResponseException {
         Map<String, List<RoleRepresentation>> repList = getRepList(partialImportRep);
         if (repList == null || repList.isEmpty()) return;
@@ -183,54 +156,6 @@ public class ClientRolesPartialImport implements PartialImport<RoleRepresentatio
         return PartialImportResult.added(getResourceType(), getCombinedName(clientId, roleRep), modelId, roleRep);
     }
 
-    @Override
-    public PartialImportResults doImport(PartialImportRepresentation partialImportRep, RealmModel realm, KeycloakSession session) throws ErrorResponseException {
-        PartialImportResults results = new PartialImportResults();
-        Map<String, List<RoleRepresentation>> repList = getRepList(partialImportRep);
-        if ((repList == null) || repList.isEmpty()) return results;
-
-        for (String clientId : toOverwrite.keySet()) {
-            for (RoleRepresentation roleRep : toOverwrite.get(clientId)) {
-                System.out.println("overwriting " + getResourceType() + " " + getCombinedName(clientId, roleRep));
-                try {
-                    overwrite(realm, session, clientId, roleRep);
-                } catch (Exception e) {
-                    throw new ErrorResponseException(ErrorResponse.error(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR));
-                }
-
-                String modelId = getModelId(realm, clientId);
-                results.addResult(overwritten(clientId, modelId, roleRep));
-            }
-        }
-
-        for (String clientId : toSkip.keySet()) {
-            for (RoleRepresentation roleRep : toSkip.get(clientId)) {
-                System.out.println("skipping " + getResourceType() + " " + getCombinedName(clientId, roleRep));
-                String modelId = getModelId(realm, clientId);
-                results.addResult(skipped(clientId, modelId, roleRep));
-            }
-        }
-
-        for (String clientId : repList.keySet()) {
-            for (RoleRepresentation roleRep : repList.get(clientId)) {
-                if (toOverwrite.get(clientId).contains(roleRep)) continue;
-                if (toSkip.get(clientId).contains(roleRep)) continue;
-
-                try {
-                    System.out.println("adding " + getResourceType() + " " + getCombinedName(clientId, roleRep));
-                    create(realm, session, clientId, roleRep);
-                    String modelId = getModelId(realm, clientId);
-                    results.addResult(added(clientId, modelId, roleRep));
-                } catch (Exception e) {
-                    //e.printStackTrace();
-                    throw new ErrorResponseException(ErrorResponse.error(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR));
-                }
-            }
-        }
-
-        return results;
-    }
-
     public String getModelId(RealmModel realm, String clientId) {
         return realm.getClientByClientId(clientId).getId();
     }
diff --git a/services/src/main/java/org/keycloak/partialimport/ClientsPartialImport.java b/services/src/main/java/org/keycloak/partialimport/ClientsPartialImport.java
index 940dbe3..c04ab46 100644
--- a/services/src/main/java/org/keycloak/partialimport/ClientsPartialImport.java
+++ b/services/src/main/java/org/keycloak/partialimport/ClientsPartialImport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -30,8 +30,9 @@ import org.keycloak.services.managers.ClientManager;
 import org.keycloak.services.managers.RealmManager;
 
 /**
+ * PartialImport handler for Clients.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public class ClientsPartialImport extends AbstractPartialImport<ClientRepresentation> {
 
@@ -66,12 +67,7 @@ public class ClientsPartialImport extends AbstractPartialImport<ClientRepresenta
     }
 
     @Override
-    public void overwrite(RealmModel realm, KeycloakSession session, ClientRepresentation clientRep) {
-        remove(realm, session, clientRep);
-        create(realm, session, clientRep);
-    }
-
-    protected void remove(RealmModel realm, KeycloakSession session, ClientRepresentation clientRep) {
+    public void remove(RealmModel realm, KeycloakSession session, ClientRepresentation clientRep) {
         ClientModel clientModel = realm.getClientByClientId(getName(clientRep));
         new ClientManager(new RealmManager(session)).removeClient(realm, clientModel);
     }
diff --git a/services/src/main/java/org/keycloak/partialimport/ErrorResponseException.java b/services/src/main/java/org/keycloak/partialimport/ErrorResponseException.java
index 22aa632..b273f56 100644
--- a/services/src/main/java/org/keycloak/partialimport/ErrorResponseException.java
+++ b/services/src/main/java/org/keycloak/partialimport/ErrorResponseException.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -21,11 +21,12 @@ import javax.ws.rs.core.Response;
 
 
 /**
+ * An exception that can hold a Response object.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public class ErrorResponseException extends Exception {
-    private Response response;
+    private final Response response;
 
     public ErrorResponseException(Response response) {
         this.response = response;
diff --git a/services/src/main/java/org/keycloak/partialimport/IdentityProvidersPartialImport.java b/services/src/main/java/org/keycloak/partialimport/IdentityProvidersPartialImport.java
index c17350a..59e1153 100644
--- a/services/src/main/java/org/keycloak/partialimport/IdentityProvidersPartialImport.java
+++ b/services/src/main/java/org/keycloak/partialimport/IdentityProvidersPartialImport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -27,8 +27,9 @@ import org.keycloak.representations.idm.IdentityProviderRepresentation;
 import org.keycloak.representations.idm.PartialImportRepresentation;
 
 /**
+ * PartialImport handler for Identitiy Providers.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public class IdentityProvidersPartialImport extends AbstractPartialImport<IdentityProviderRepresentation> {
 
@@ -63,12 +64,7 @@ public class IdentityProvidersPartialImport extends AbstractPartialImport<Identi
     }
 
     @Override
-    public void overwrite(RealmModel realm, KeycloakSession session, IdentityProviderRepresentation idpRep) {
-        remove(realm, idpRep);
-        create(realm, session, idpRep);
-    }
-
-    protected void remove(RealmModel realm, IdentityProviderRepresentation idpRep) {
+    public void remove(RealmModel realm, KeycloakSession session, IdentityProviderRepresentation idpRep) {
         realm.removeIdentityProviderByAlias(getName(idpRep));
     }
 
diff --git a/services/src/main/java/org/keycloak/partialimport/PartialImport.java b/services/src/main/java/org/keycloak/partialimport/PartialImport.java
index e80368f..eaae106 100644
--- a/services/src/main/java/org/keycloak/partialimport/PartialImport.java
+++ b/services/src/main/java/org/keycloak/partialimport/PartialImport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -22,20 +22,45 @@ import org.keycloak.models.RealmModel;
 import org.keycloak.representations.idm.PartialImportRepresentation;
 
 /**
+ * Main interface for PartialImport handlers.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public interface PartialImport<T> {
 
+    /**
+     * Find which resources will need to be skipped or overwritten.  Also,
+     * do a preliminary check for errors.
+     *
+     * @param rep Everything in the PartialImport request.
+     * @param realm Realm to be imported into.
+     * @param session The KeycloakSession.
+     * @throws ErrorResponseException If the PartialImport can not be performed,
+     *                                throw this exception.
+     */
     public void prepare(PartialImportRepresentation rep,
                          RealmModel realm,
                          KeycloakSession session) throws ErrorResponseException;
 
     /**
-     * @param rep
-     * @param realm
-     * @param session
-     * @return
+     * Delete resources that will be overwritten.  This is done separately so
+     * that it can be called for all resource types before calling all the doImports.
+     *
+     * It was found that doing delete/add per resource causes errors because of
+     * cascading deletes.
+     *
+     * @param realm Realm to be imported into.
+     * @param session The KeycloakSession
+     */
+    public void removeOverwrites(RealmModel realm, KeycloakSession session);
+
+    /**
+     * Create (or re-create) all the imported resources.
+     *
+     * @param rep Everything in the PartialImport request.
+     * @param realm Realm to be imported into.
+     * @param session The KeycloakSession.
+     * @return The final results of the PartialImport request.
      * @throws ErrorResponseException if an error was detected trying to doImport a resource.
      */
     public PartialImportResults doImport(PartialImportRepresentation rep,
diff --git a/services/src/main/java/org/keycloak/partialimport/PartialImportManager.java b/services/src/main/java/org/keycloak/partialimport/PartialImportManager.java
index 7a8eeb0..1bc391f 100644
--- a/services/src/main/java/org/keycloak/partialimport/PartialImportManager.java
+++ b/services/src/main/java/org/keycloak/partialimport/PartialImportManager.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -18,36 +18,21 @@
 package org.keycloak.partialimport;
 
 import java.util.ArrayList;
-import java.util.HashMap;
-import java.util.HashSet;
 import java.util.List;
-import java.util.Map;
-import java.util.Set;
 import javax.ws.rs.core.Response;
-import javax.ws.rs.core.UriInfo;
 import org.keycloak.events.admin.OperationType;
-import org.keycloak.models.ClientModel;
-import org.keycloak.models.IdentityProviderModel;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
-import org.keycloak.models.UserModel;
-import org.keycloak.models.utils.RepresentationToModel;
-import org.keycloak.representations.idm.ClientRepresentation;
-import org.keycloak.representations.idm.IdentityProviderRepresentation;
 import org.keycloak.representations.idm.PartialImportRepresentation;
-import org.keycloak.representations.idm.UserRepresentation;
-import org.keycloak.services.ErrorResponse;
 import org.keycloak.services.resources.admin.AdminEventBuilder;
-import org.keycloak.services.resources.admin.ClientResource;
-import org.keycloak.services.resources.admin.IdentityProviderResource;
-import org.keycloak.services.resources.admin.UsersResource;
 
 /**
+ * This class manages the PartialImport handlers.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public class PartialImportManager {
-    private List<PartialImport> partialImports = new ArrayList<>();
+    private final List<PartialImport> partialImports = new ArrayList<>();
 
     private final PartialImportRepresentation rep;
     private final KeycloakSession session;
@@ -64,8 +49,8 @@ public class PartialImportManager {
         // Do not change the order of these!!!
         partialImports.add(new ClientsPartialImport());
         partialImports.add(new RolesPartialImport());
-        partialImports.add(new UsersPartialImport());
         partialImports.add(new IdentityProvidersPartialImport());
+        partialImports.add(new UsersPartialImport());
     }
 
     public Response saveResources() {
@@ -83,6 +68,7 @@ public class PartialImportManager {
 
         for (PartialImport partialImport : partialImports) {
             try {
+                partialImport.removeOverwrites(realm, session);
                 results.addAllResults(partialImport.doImport(rep, realm, session));
             } catch (ErrorResponseException error) {
                 if (session.getTransaction().isActive()) session.getTransaction().setRollbackOnly();
@@ -118,220 +104,4 @@ public class PartialImportManager {
                   .success();
     }
 
-       /* Response response = prepareForExistingResources();
-        if (response != null) return response;
-
-        response = saveUsers();
-        if (response != null) {
-            session.getTransaction().rollback();
-            return response;
-        }
-
-        response = saveClients();
-        if (response != null) {
-            session.getTransaction().rollback();
-            return response;
-        }
-
-        response = saveIdps();
-        if (response != null) {
-            session.getTransaction().rollback();
-            return response;
-        }
-
-        if (session.getTransaction().isActive()) {
-            session.getTransaction().commit();
-        }
-
-
-        return Response.ok(resultsMap()).build();*/
-    //}
-/*
-    private Map resultsMap() {
-        Map<String, Integer> results = new HashMap<>();
-        results.put("added", added);
-        results.put("skipped", skipped);
-        results.put("overwritten", overwritten);
-        return results;
-    }
-
-    // returns an error response or null
-    private Response prepareForExistingResources() {
-
-        if (rep.hasUsers()) {
-            Response response = prepareUsers();
-            if (response != null) return response;
-        }
-
-        if (rep.hasClients()) {
-            Response response = prepareClients();
-            if (response != null) return response;
-        }
-
-        if (rep.hasIdps()) {
-            Response response = prepareIdps();
-            if (response != null) return response;
-        }
-
-        return null;
-    }
-
-    // returns an error response or null
-    private Response prepareClients() {
-        Set<ClientRepresentation> toSkip = new HashSet<>();
-        for (ClientRepresentation client : rep.getClients()) {
-            if (clientExists(client)) {
-                switch (rep.getPolicy()) {
-                    case SKIP: toSkip.addResult(client); break;
-                    case OVERWRITE: clientsToOverwrite.addResult(client); break;
-                    default: return ErrorResponse.exists("Client id '" + client.getClientId() + "' already exists");
-                }
-            }
-        }
-
-        for (ClientRepresentation client : toSkip) {
-            rep.getClients().remove(client);
-            skipped(client);
-        }
-
-        return null;
-    }
-
-    private boolean clientExists(ClientRepresentation rep) {
-        return realm.getClientByClientId(rep.getClientId()) != null;
-    }
-
-    // returns an error response or null
-    private Response saveClients() {
-        if (!rep.hasClients()) return null;
-
-        for (ClientRepresentation client : clientsToOverwrite) {
-            ClientModel clientModel = realm.getClientByClientId(client.getClientId());
-            ClientResource.updateClientFromRep(client, clientModel, session);
-            adminEvent.operation(OperationType.UPDATE).resourcePath(uriInfo).representation(client).success();
-            overwritten(client);
-        }
-
-        for (ClientRepresentation client : rep.getClients()) {
-            if (clientsToOverwrite.contains(client)) continue;
-
-            try {
-                RepresentationToModel.createClient(session, realm, client, true);
-                added(client);
-            } catch (Exception e) {
-                if (session.getTransaction().isActive()) session.getTransaction().setRollbackOnly();
-                return ErrorResponse.error(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR);
-            }
-        }
-
-        return null;
-    }
-
-    // returns an error response or null
-    private Response prepareIdps() {
-        Set<IdentityProviderRepresentation> toSkip = new HashSet<>();
-        for (IdentityProviderRepresentation idp : rep.getIdentityProviders()) {
-            if (idpExists(idp)) {
-                switch (rep.getPolicy()) {
-                    case SKIP: toSkip.addResult(idp); break;
-                    case OVERWRITE: idpsToOverwrite.addResult(idp); break;
-                    default: return ErrorResponse.exists("Identity Provider '" + idp.getAlias() + "' already exists");
-                }
-            }
-        }
-
-        for (IdentityProviderRepresentation idp : toSkip) {
-            rep.getIdentityProviders().remove(idp);
-            skipped(idp);
-        }
-
-        return null;
-    }
-
-    private boolean idpExists(IdentityProviderRepresentation rep) {
-        return realm.getIdentityProviderByAlias(rep.getAlias()) != null;
-    }
-
-    // returns an error response or null
-    private Response saveIdps() {
-        if (!rep.hasIdps()) return null;
-
-        for (IdentityProviderRepresentation idp : idpsToOverwrite) {
-            IdentityProviderResource.updateIdpFromRep(idp, realm, session);
-            overwritten(idp);
-        }
-
-        for (IdentityProviderRepresentation idp : rep.getIdentityProviders()) {
-            if (idpsToOverwrite.contains(idp)) continue;
-
-            try {
-                IdentityProviderModel identityProvider = RepresentationToModel.toModel(idp);
-                realm.addIdentityProvider(identityProvider);
-                added(idp);
-            } catch (Exception e) {
-                if (session.getTransaction().isActive()) session.getTransaction().setRollbackOnly();
-                return ErrorResponse.error(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR);
-            }
-        }
-
-        return null;
-    }
-
-    // returns an error response or null
-    private Response prepareUsers() {
-        Set<UserRepresentation> toSkip = new HashSet<>();
-        for (UserRepresentation user : rep.getUsers()) {
-            if (session.users().getUserByUsername(user.getUsername(), realm) != null) {
-                switch (rep.getPolicy()) {
-                    case SKIP: toSkip.addResult(user); break;
-                    case OVERWRITE: usersToOverwrite.addResult(user); break;
-                    default: return ErrorResponse.exists("User '" + user.getUsername() + "' already exists");
-                }
-            }
-            if ((user.getEmail() != null) && (session.users().getUserByEmail(user.getEmail(), realm) != null)) {
-                switch (rep.getPolicy()) {
-                    case SKIP: toSkip.addResult(user); break;
-                    case OVERWRITE: usersToOverwrite.addResult(user); break;
-                    default: FAIL: return ErrorResponse.exists("User email '" + user.getEmail() + "' already exists");
-                }
-            }
-        }
-
-        for (UserRepresentation user : toSkip) {
-            rep.getUsers().remove(user);
-            skipped(user);
-        }
-
-        return null;
-    }
-
-    // returns an error response or null
-    private Response saveUsers() {
-        if (!rep.hasUsers()) return null;
-
-        for (UserRepresentation user: usersToOverwrite) {
-            System.out.println("overwriting user " + user.getUsername());
-            UserModel userModel = session.users().getUserByUsername(user.getUsername(), realm);
-            UsersResource.updateUserFromRep(userModel, user, null, realm, session);
-            overwritten(user);
-        }
-
-        for (UserRepresentation user : rep.getUsers()) {
-            if (usersToOverwrite.contains(user)) continue;
-            try {
-                System.out.println("saving user " + user.getUsername());
-                Map<String, ClientModel> apps = realm.getClientNameMap();
-                UserModel userModel = RepresentationToModel.createUser(session, realm, user, apps);
-                added(user);
-            } catch (Exception e) {
-                //e.printStackTrace();
-                if (session.getTransaction().isActive()) session.getTransaction().setRollbackOnly();
-                return ErrorResponse.error(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR);
-            }
-        }
-
-        return null;
-    }
-*/
-
 }
diff --git a/services/src/main/java/org/keycloak/partialimport/PartialImportResult.java b/services/src/main/java/org/keycloak/partialimport/PartialImportResult.java
index 0ed082f..6a732e8 100644
--- a/services/src/main/java/org/keycloak/partialimport/PartialImportResult.java
+++ b/services/src/main/java/org/keycloak/partialimport/PartialImportResult.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -20,8 +20,9 @@ package org.keycloak.partialimport;
 import org.codehaus.jackson.annotate.JsonIgnore;
 
 /**
+ * This class represents a single result for a resource imported.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public class PartialImportResult {
 
diff --git a/services/src/main/java/org/keycloak/partialimport/PartialImportResults.java b/services/src/main/java/org/keycloak/partialimport/PartialImportResults.java
index 9ff95c4..cc372f4 100644
--- a/services/src/main/java/org/keycloak/partialimport/PartialImportResults.java
+++ b/services/src/main/java/org/keycloak/partialimport/PartialImportResults.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -21,15 +21,16 @@ import java.util.HashSet;
 import java.util.Set;
 
 /**
+ * Aggregates all the PartialImportResult objects.
+ * These results are used in the admin UI and for creating admin events.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public class PartialImportResults {
 
     private final Set<PartialImportResult> importResults = new HashSet<>();
 
     public void addResult(PartialImportResult result) {
-        //System.out.println("PartialImportResults: add " + result.getResourceName() + " action=" + result.getAction());
         importResults.add(result);
     }
 
@@ -49,7 +50,6 @@ public class PartialImportResults {
     public int getOverwritten() {
         int overwritten = 0;
         for (PartialImportResult result : importResults) {
-            //System.out.println("action=" + result.getAction());
             if (result.getAction() == Action.OVERWRITTEN) overwritten++;
         }
 
diff --git a/services/src/main/java/org/keycloak/partialimport/RealmRolesPartialImport.java b/services/src/main/java/org/keycloak/partialimport/RealmRolesPartialImport.java
index ec5004a..f7276a1 100644
--- a/services/src/main/java/org/keycloak/partialimport/RealmRolesPartialImport.java
+++ b/services/src/main/java/org/keycloak/partialimport/RealmRolesPartialImport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -26,8 +26,9 @@ import org.keycloak.representations.idm.RoleRepresentation;
 import org.keycloak.services.resources.admin.RoleResource;
 
 /**
+ * PartialImport handler for Realm Roles.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public class RealmRolesPartialImport extends AbstractPartialImport<RoleRepresentation> {
 
@@ -81,13 +82,7 @@ public class RealmRolesPartialImport extends AbstractPartialImport<RoleRepresent
     }
 
     @Override
-    public void overwrite(RealmModel realm, KeycloakSession session, RoleRepresentation roleRep) {
-        //checkForComposite(roleRep);
-        deleteRole(realm, roleRep);
-        create(realm, session, roleRep);
-    }
-
-    public void deleteRole(RealmModel realm, RoleRepresentation roleRep) {
+    public void remove(RealmModel realm, KeycloakSession session, RoleRepresentation roleRep) {
         RoleModel role = realm.getRole(getName(roleRep));
         RoleHelper helper = new RoleHelper(realm);
         helper.deleteRole(role);
@@ -95,9 +90,7 @@ public class RealmRolesPartialImport extends AbstractPartialImport<RoleRepresent
 
     @Override
     public void create(RealmModel realm, KeycloakSession session, RoleRepresentation roleRep) {
-        //checkForComposite(roleRep);
         realm.addRole(getName(roleRep));
-        //overwrite(realm, session, roleRep);
     }
 
     public static class RoleHelper extends RoleResource {
diff --git a/services/src/main/java/org/keycloak/partialimport/ResourceType.java b/services/src/main/java/org/keycloak/partialimport/ResourceType.java
index 3db0486..25a09d2 100644
--- a/services/src/main/java/org/keycloak/partialimport/ResourceType.java
+++ b/services/src/main/java/org/keycloak/partialimport/ResourceType.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -18,12 +18,18 @@
 package org.keycloak.partialimport;
 
 /**
+ * Enum for each resource type that can be partially imported.
  *
- * @author ssilvert
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public enum ResourceType {
     USER, CLIENT, IDP, REALM_ROLE, CLIENT_ROLE;
 
+    /**
+     * Used to create the admin path in events.
+     *
+     * @return The resource portion of the path.
+     */
     public String getPath() {
         switch(this) {
             case USER: return "users";
diff --git a/services/src/main/java/org/keycloak/partialimport/RolesPartialImport.java b/services/src/main/java/org/keycloak/partialimport/RolesPartialImport.java
index c9c4fdb..6bf145e 100644
--- a/services/src/main/java/org/keycloak/partialimport/RolesPartialImport.java
+++ b/services/src/main/java/org/keycloak/partialimport/RolesPartialImport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -19,6 +19,8 @@ package org.keycloak.partialimport;
 import java.util.List;
 import java.util.Map;
 import java.util.Set;
+import javax.ws.rs.core.Response;
+import org.jboss.logging.Logger;
 import org.keycloak.models.KeycloakSession;
 import org.keycloak.models.RealmModel;
 import org.keycloak.models.utils.KeycloakModelUtils;
@@ -26,6 +28,7 @@ import org.keycloak.models.utils.RepresentationToModel;
 import org.keycloak.representations.idm.PartialImportRepresentation;
 import org.keycloak.representations.idm.RoleRepresentation;
 import org.keycloak.representations.idm.RolesRepresentation;
+import org.keycloak.services.ErrorResponse;
 
 /**
  * This class handles both realm roles and client roles.  It delegates to
@@ -38,9 +41,10 @@ import org.keycloak.representations.idm.RolesRepresentation;
  * importRoles() doesn't know about them.  The logic for overwrite needs to delete
  * the overwritten roles before importRoles() is called.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public class RolesPartialImport implements PartialImport<RolesRepresentation> {
+    protected static Logger logger = Logger.getLogger(RolesPartialImport.class);
 
     private Set<RoleRepresentation> realmRolesToOverwrite;
     private Set<RoleRepresentation> realmRolesToSkip;
@@ -74,24 +78,37 @@ public class RolesPartialImport implements PartialImport<RolesRepresentation> {
     }
 
     @Override
+    public void removeOverwrites(RealmModel realm, KeycloakSession session) {
+        deleteClientRoleOverwrites(realm);
+        deleteRealmRoleOverwrites(realm, session);
+    }
+
+    @Override
     public PartialImportResults doImport(PartialImportRepresentation rep, RealmModel realm, KeycloakSession session) throws ErrorResponseException {
         PartialImportResults results = new PartialImportResults();
         if (!rep.hasRealmRoles() && !rep.hasClientRoles()) return results;
 
-        // finalize preparation and add results for skips and overwrites
+        // finalize preparation and add results for skips
         removeRealmRoleSkips(results, rep, realm, session);
         removeClientRoleSkips(results, rep, realm);
-        deleteRealmRoleOverwrites(results, realm, session);
-        deleteClientRoleOverwrites(results, realm);
         if (rep.hasRealmRoles()) setUniqueIds(rep.getRoles().getRealm());
         if (rep.hasClientRoles()) setUniqueIds(rep.getRoles().getClient());
 
-        RepresentationToModel.importRoles(rep.getRoles(), realm);
+        try {
+            RepresentationToModel.importRoles(rep.getRoles(), realm);
+        } catch (Exception e) {
+            logger.error("Error importing roles", e);
+            throw new ErrorResponseException(ErrorResponse.error(e.getMessage(), Response.Status.INTERNAL_SERVER_ERROR));
+        }
 
         // add "add" results for new roles created
         realmRoleAdds(results, rep, realm, session);
         clientRoleAdds(results, rep, realm);
 
+        // add "overwritten" results for roles overwritten
+        addResultsForOverwrittenRealmRoles(results, realm, session);
+        addResultsForOverwrittenClientRoles(results, realm);
+
         return results;
     }
 
@@ -136,22 +153,38 @@ public class RolesPartialImport implements PartialImport<RolesRepresentation> {
         }
     }
 
-    private void deleteRealmRoleOverwrites(PartialImportResults results, RealmModel realm, KeycloakSession session) {
+    private void deleteRealmRoleOverwrites(RealmModel realm, KeycloakSession session) {
+        if (isEmpty(realmRolesToOverwrite)) return;
+
+        for (RoleRepresentation roleRep : realmRolesToOverwrite) {
+            realmRolesPI.remove(realm, session, roleRep);
+        }
+    }
+
+    private void addResultsForOverwrittenRealmRoles(PartialImportResults results, RealmModel realm, KeycloakSession session) {
         if (isEmpty(realmRolesToOverwrite)) return;
 
         for (RoleRepresentation roleRep : realmRolesToOverwrite) {
-            realmRolesPI.deleteRole(realm, roleRep);
             String modelId = realmRolesPI.getModelId(realm, session, roleRep);
             results.addResult(realmRolesPI.overwritten(modelId, roleRep));
         }
     }
 
-    private void deleteClientRoleOverwrites(PartialImportResults results, RealmModel realm) {
+    private void deleteClientRoleOverwrites(RealmModel realm) {
         if (isEmpty(clientRolesToOverwrite)) return;
 
         for (String clientId : clientRolesToOverwrite.keySet()) {
             for (RoleRepresentation roleRep : clientRolesToOverwrite.get(clientId)) {
                 clientRolesPI.deleteRole(realm, clientId, roleRep);
+            }
+        }
+    }
+
+    private void addResultsForOverwrittenClientRoles(PartialImportResults results, RealmModel realm) {
+        if (isEmpty(clientRolesToOverwrite)) return;
+
+        for (String clientId : clientRolesToOverwrite.keySet()) {
+            for (RoleRepresentation roleRep : clientRolesToOverwrite.get(clientId)) {
                 String modelId = clientRolesPI.getModelId(realm, clientId);
                 results.addResult(clientRolesPI.overwritten(clientId, modelId, roleRep));
             }
@@ -176,7 +209,6 @@ public class RolesPartialImport implements PartialImport<RolesRepresentation> {
             if (realmRolesToOverwrite.contains(roleRep)) continue;
             if (realmRolesToSkip.contains(roleRep)) continue;
 
-            //System.out.println("adding " + realmRolesPI.getResourceType() + " " + realmRolesPI.getName(roleRep));
             String modelId = realmRolesPI.getModelId(realm, session, roleRep);
             results.addResult(realmRolesPI.added(modelId, roleRep));
         }
@@ -193,7 +225,6 @@ public class RolesPartialImport implements PartialImport<RolesRepresentation> {
                 if (clientRolesToOverwrite.get(clientId).contains(roleRep)) continue;
                 if (clientRolesToSkip.get(clientId).contains(roleRep)) continue;
 
-                //System.out.println("adding " + clientRolesPI.getResourceType() + " " + clientRolesPI.getCombinedName(clientId, roleRep));
                 String modelId = clientRolesPI.getModelId(realm, clientId);
                 results.addResult(clientRolesPI.added(clientId, modelId, roleRep));
             }
diff --git a/services/src/main/java/org/keycloak/partialimport/UsersPartialImport.java b/services/src/main/java/org/keycloak/partialimport/UsersPartialImport.java
index 68d6b0a..2ae3fc3 100644
--- a/services/src/main/java/org/keycloak/partialimport/UsersPartialImport.java
+++ b/services/src/main/java/org/keycloak/partialimport/UsersPartialImport.java
@@ -1,5 +1,5 @@
 /*
- * Copyright 2015 Red Hat Inc. and/or its affiliates and other contributors
+ * Copyright 2016 Red Hat Inc. and/or its affiliates and other contributors
  * as indicated by the @author tags. All rights reserved.
  *
  * Licensed under the Apache License, Version 2.0 (the "License"); you may not
@@ -31,8 +31,9 @@ import org.keycloak.representations.idm.UserRepresentation;
 import org.keycloak.services.managers.UserManager;
 
 /**
+ * PartialImport handler for users.
  *
- * @author Stan Silvert ssilvert@redhat.com (C) 2015 Red Hat Inc.
+ * @author Stan Silvert ssilvert@redhat.com (C) 2016 Red Hat Inc.
  */
 public class UsersPartialImport extends AbstractPartialImport<UserRepresentation> {
 
@@ -94,12 +95,7 @@ public class UsersPartialImport extends AbstractPartialImport<UserRepresentation
     }
 
     @Override
-    public void overwrite(RealmModel realm, KeycloakSession session, UserRepresentation user) {
-        remove(realm, session, user);
-        create(realm, session, user);
-    }
-
-    protected void remove(RealmModel realm, KeycloakSession session, UserRepresentation user) {
+    public void remove(RealmModel realm, KeycloakSession session, UserRepresentation user) {
         UserModel userModel = session.users().getUserByUsername(user.getUsername(), realm);
         if (userModel == null) {
             userModel = session.users().getUserByEmail(user.getEmail(), realm);