Skip to content

Commit

Permalink
Fix/minor binding fixes (#163)
Browse files Browse the repository at this point in the history
* fix(mps-sync-plugin-lib): make the remove method calls in the MpsToModelixMap null-safe

* feat(mps-sync-plugin): do not remove models and modules locally if we deactivate the bindings

the reason for that is, the user may want to push these models/modules into other repositories too, after they disconnected from the previous model server / repository / branch.
  • Loading branch information
benedekh authored Oct 17, 2024
1 parent 1e9c935 commit bfffafe
Show file tree
Hide file tree
Showing 4 changed files with 5 additions and 89 deletions.
Original file line number Diff line number Diff line change
Expand Up @@ -17,7 +17,6 @@
package org.modelix.mps.sync.bindings

import jetbrains.mps.extapi.model.SModelBase
import jetbrains.mps.model.ModelDeleteHelper
import mu.KotlinLogging
import org.modelix.kotlin.utils.UnstableModelixFeature
import org.modelix.model.api.IBranch
Expand Down Expand Up @@ -53,9 +52,6 @@ class ModelBinding(val model: SModelBase, branch: IBranch, serviceLocator: Servi
@Volatile
private var isActivated = false

@Volatile
private var modelDeletedLocally = false

@Synchronized
override fun activate(callback: Runnable?) {
if (isDisposed || isActivated) {
Expand Down Expand Up @@ -101,30 +97,6 @@ class ModelBinding(val model: SModelBase, branch: IBranch, serviceLocator: Servi
isActivated = false
}
}
}.continueWith(linkedSetOf(SyncLock.MPS_WRITE), SyncDirection.MPS_TO_MODELIX) {
synchronized(this) {
try {
// delete model
if (!removeFromServer && !modelDeletedLocally) {
/*
* to delete the files locally, otherwise MPS takes care of calling
* ModelDeleteHelper(model).delete() to delete the model (if removeFromServer is true)
*/
ModelDeleteHelper(model).delete()
modelDeletedLocally = true
}
} catch (ex: Exception) {
logger.error(ex) { "Exception occurred while deactivating ${name()}." }
/*
* if any error occurs, then we put the binding back to let the rest of the application know that
* it exists
*/
bindingsRegistry.addModelBinding(this)
activate()

throw ex
}
}
}.continueWith(linkedSetOf(SyncLock.NONE), SyncDirection.NONE) {
bindingsRegistry.removeModelBinding(model.module!!, this)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -16,7 +16,6 @@

package org.modelix.mps.sync.bindings

import jetbrains.mps.module.ModuleDeleteHelper
import jetbrains.mps.project.AbstractModule
import mu.KotlinLogging
import org.modelix.kotlin.utils.UnstableModelixFeature
Expand Down Expand Up @@ -44,7 +43,6 @@ class ModuleBinding(val module: AbstractModule, branch: IBranch, serviceLocator:

private val bindingsRegistry = serviceLocator.bindingsRegistry
private val notifier = serviceLocator.wrappedNotifier
private val mpsProject = serviceLocator.mpsProject

private val changeListener = ModuleChangeListener(branch, serviceLocator)

Expand All @@ -54,9 +52,6 @@ class ModuleBinding(val module: AbstractModule, branch: IBranch, serviceLocator:
@Volatile
private var isActivated = false

@Volatile
private var moduleDeletedLocally = false

@Synchronized
override fun activate(callback: Runnable?) {
if (isDisposed || isActivated) {
Expand Down Expand Up @@ -111,31 +106,6 @@ class ModuleBinding(val module: AbstractModule, branch: IBranch, serviceLocator:
bindingsRegistry.removeModuleBinding(this)
isActivated = false
}
}.continueWith(linkedSetOf(SyncLock.MPS_WRITE), SyncDirection.MPS_TO_MODELIX) {
synchronized(this) {
// delete module
try {
if (!removeFromServer && !moduleDeletedLocally) {
/*
* if we just delete it locally, then we have to call ModuleDeleteHelper manually.
* otherwise, MPS will call us via the event-handler chain starting from
* ModuleDeleteHelper.deleteModules --> RepositoryChangeListener -->
* moduleListener.deactivate(removeFromServer = true)
*/
ModuleDeleteHelper(mpsProject).deleteModules(listOf(module), false, true)
moduleDeletedLocally = true
}
} catch (ex: Exception) {
logger.error(ex) { "Exception occurred while deactivating ${name()}." }
/*
* if any error occurs, then we put the binding back to let the rest of the application know that
* it exists
*/
bindingsRegistry.addModuleBinding(this)
activate()
throw ex
}
}
}.continueWith(linkedSetOf(SyncLock.NONE), SyncDirection.NONE) {
nodeMap.remove(module)

Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -174,18 +174,13 @@ class MpsToModelixMap : InjectableService {

modelixIdToModel.remove(modelixId)?.let {
modelToModelixId.remove(it)

val model = it.resolve(mpsRepository)
remove(model)
it.resolve(mpsRepository)?.let { model -> remove(model) }
}

modelixIdToModelWithOutgoingModelReference[modelixId]?.let { remove(it) }
modelixIdToModelWithOutgoingModuleReference[modelixId]?.let { remove(it) }

modelixIdToModule.remove(modelixId)?.let {
val module = it.resolve(mpsRepository)!!
remove(module)
}
modelixIdToModule.remove(modelixId)?.let { it.resolve(mpsRepository)?.let { module -> remove(module) } }
modelixIdToModuleWithOutgoingModuleReference[modelixId]?.let { remove(it) }
}

Expand Down Expand Up @@ -219,8 +214,7 @@ class MpsToModelixMap : InjectableService {
val target = ModuleWithModuleReference(module, it)
remove(target)
} else if (it is SModelReference) {
val model = it.resolve(mpsRepository)
remove(model)
it.resolve(mpsRepository)?.let { model -> remove(model) }
}
}
}
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -81,8 +81,6 @@ class ModelSyncGuiFactory : ToolWindowFactory {
companion object {
private const val COMBOBOX_CHANGED_COMMAND = "comboBoxChanged"
private const val TEXTFIELD_WIDTH = 20
private const val DISCONNECT_REMOVES_LOCAL_COPIES =
"By disconnecting, the synchronized modules and models will be removed locally."
}

private val logger = KotlinLogging.logger {}
Expand Down Expand Up @@ -365,16 +363,7 @@ class ModelSyncGuiFactory : ToolWindowFactory {
}

if (connectionsModel.size > 0) {
if (bindingsModel.size == 0) {
disconnectAction()
return
}

AlertNotifier(activeProject).warning(DISCONNECT_REMOVES_LOCAL_COPIES) { response ->
if (UserResponse.USER_ACCEPTED == response) {
disconnectAction()
}
}
disconnectAction()
}
}

Expand All @@ -392,16 +381,7 @@ class ModelSyncGuiFactory : ToolWindowFactory {
}

if (activeBranch != null) {
if (bindingsModel.size == 0) {
disconnectAction()
return
}

AlertNotifier(activeProject).warning(DISCONNECT_REMOVES_LOCAL_COPIES) { response ->
if (UserResponse.USER_ACCEPTED == response) {
disconnectAction()
}
}
disconnectAction()
}
}

Expand Down

0 comments on commit bfffafe

Please sign in to comment.