-
Notifications
You must be signed in to change notification settings - Fork 106
Add Serializable Vars State with nested fields descriptors #314
New issue
Have a question about this project? Sign up for a free GitHub account to open an issue and contact its maintainers and the community.
By clicking “Sign up for GitHub”, you agree to our terms of service and privacy statement. We’ll occasionally send you account related emails.
Already on GitHub? Sign in to your account
base: master
Are you sure you want to change the base?
Changes from all commits
eb38537
e72e266
f3bbb28
0131e41
79bebea
e9aefaa
e5fac02
f3a3ce1
dc305f7
18e844c
42751f2
445583e
a00fcec
0355749
b4ac2ce
7ac866a
d6f6d39
4ddb8ea
f9a67d5
ec951f7
5aebd3d
c1b2344
2a1081a
ef7df1e
File filter
Filter by extension
Conversations
Jump to
Diff view
Diff view
There are no files selected for viewing
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -20,12 +20,88 @@ data class SerializedCompiledScriptsData( | |
} | ||
} | ||
|
||
@Serializable | ||
data class SerializableTypeInfo(val type: Type = Type.Custom, val isPrimitive: Boolean = false, val fullType: String = "") { | ||
companion object { | ||
val ignoreSet = setOf("int", "double", "boolean", "char", "float", "byte", "string", "entry") | ||
|
||
val propertyNamesForNullFilter = setOf("data", "size") | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Аналогично (unused) |
||
|
||
fun makeFromSerializedVariablesState(type: String?, isContainer: Boolean?): SerializableTypeInfo { | ||
val fullType = type.orEmpty() | ||
val enumType = fullType.toTypeEnum() | ||
val isPrimitive = !( | ||
if (fullType != "Entry") (isContainer ?: false) | ||
else true | ||
) | ||
|
||
Comment on lines
+33
to
+37
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. fullType != "Entry" isContainer Result Это таблица для AND, значит всё это выражение можно упростить до isPrimitive = fullType!="Entry" && isContainer != true |
||
return SerializableTypeInfo(enumType, isPrimitive, fullType) | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Зачем нам тип передавать в двух видах? Честно говоря, выглядит стрёмно |
||
} | ||
} | ||
} | ||
|
||
@Serializable | ||
enum class Type { | ||
Map, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Капсом лучше писать |
||
Entry, | ||
Array, | ||
List, | ||
Custom | ||
} | ||
|
||
fun String.toTypeEnum(): Type { | ||
return when (this) { | ||
"Map" -> Type.Map | ||
"Entry" -> Type.Entry | ||
"Array" -> Type.Array | ||
"List" -> Type.List | ||
else -> Type.Custom | ||
} | ||
} | ||
|
||
@Serializable | ||
data class SerializedVariablesState( | ||
val type: SerializableTypeInfo = SerializableTypeInfo(), | ||
val value: String? = null, | ||
val isContainer: Boolean = false, | ||
val stateId: String = "" | ||
) { | ||
// todo: not null | ||
val fieldDescriptor: MutableMap<String, SerializedVariablesState?> = mutableMapOf() | ||
override fun equals(other: Any?): Boolean { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Зачем кастомные equals/hashcode? Ни один тест не упал после их удаления |
||
if (this === other) return true | ||
if (javaClass != other?.javaClass) return false | ||
|
||
other as SerializedVariablesState | ||
|
||
if (type != other.type) return false | ||
if (value != other.value) return false | ||
if (isContainer != other.isContainer) return false | ||
|
||
return true | ||
} | ||
|
||
override fun hashCode(): Int { | ||
var result = type.hashCode() | ||
result = 31 * result + (value?.hashCode() ?: 0) | ||
result = 31 * result + isContainer.hashCode() | ||
return result | ||
} | ||
} | ||
|
||
@Serializable | ||
class SerializationReply( | ||
val cell_id: Int = 1, | ||
val descriptorsState: Map<String, SerializedVariablesState> = emptyMap(), | ||
val comm_id: String = "" | ||
) | ||
|
||
@Serializable | ||
class EvaluatedSnippetMetadata( | ||
val newClasspath: Classpath = emptyList(), | ||
val compiledData: SerializedCompiledScriptsData = SerializedCompiledScriptsData.EMPTY, | ||
val newImports: List<String> = emptyList(), | ||
val evaluatedVariablesState: Map<String, String?> = mutableMapOf() | ||
val evaluatedVariablesState: Map<String, SerializedVariablesState> = emptyMap() | ||
) { | ||
companion object { | ||
val EMPTY = EvaluatedSnippetMetadata() | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -12,6 +12,7 @@ import org.jetbrains.kotlinx.jupyter.api.RenderersProcessor | |
import org.jetbrains.kotlinx.jupyter.api.ResultsAccessor | ||
import org.jetbrains.kotlinx.jupyter.api.VariableState | ||
import org.jetbrains.kotlinx.jupyter.api.libraries.LibraryResolutionRequest | ||
import org.jetbrains.kotlinx.jupyter.repl.InternalEvaluator | ||
import org.jetbrains.kotlinx.jupyter.repl.impl.SharedReplContext | ||
|
||
class DisplayResultWrapper private constructor( | ||
|
@@ -133,7 +134,9 @@ class NotebookImpl( | |
|
||
private val history = arrayListOf<CodeCellImpl>() | ||
private var mainCellCreated = false | ||
private val _unchangedVariables: MutableSet<String> = mutableSetOf() | ||
|
||
val unchangedVariables: Set<String> get() = _unchangedVariables | ||
val displays = DisplayContainerImpl() | ||
|
||
override fun getAllDisplays(): List<DisplayResultWithCell> { | ||
|
@@ -149,6 +152,11 @@ class NotebookImpl( | |
override val jreInfo: JREInfoProvider | ||
get() = JavaRuntime | ||
|
||
fun updateVariablesState(evaluator: InternalEvaluator) { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. It seems like a very internal method. We most likely don't want to expose it to user |
||
_unchangedVariables.clear() | ||
_unchangedVariables.addAll(evaluator.getUnchangedVariables()) | ||
} | ||
|
||
fun variablesReportAsHTML(): String { | ||
return generateHTMLVarsReport(variablesState) | ||
} | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -23,6 +23,7 @@ import kotlinx.serialization.json.decodeFromJsonElement | |
import kotlinx.serialization.json.encodeToJsonElement | ||
import kotlinx.serialization.json.jsonObject | ||
import kotlinx.serialization.serializer | ||
import org.jetbrains.kotlinx.jupyter.compiler.util.SerializedVariablesState | ||
import org.jetbrains.kotlinx.jupyter.config.LanguageInfo | ||
import org.jetbrains.kotlinx.jupyter.exceptions.ReplException | ||
import kotlin.reflect.KClass | ||
|
@@ -87,7 +88,11 @@ enum class MessageType(val contentClass: KClass<out MessageContent>) { | |
COMM_CLOSE(CommClose::class), | ||
|
||
LIST_ERRORS_REQUEST(ListErrorsRequest::class), | ||
LIST_ERRORS_REPLY(ListErrorsReply::class); | ||
LIST_ERRORS_REPLY(ListErrorsReply::class), | ||
|
||
// from Serialization_Request | ||
VARIABLES_VIEW_REQUEST(SerializationRequest::class), | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Тут видимо не допереименовано |
||
VARIABLES_VIEW_REPLY(SerializationReply::class); | ||
|
||
val type: String | ||
get() = name.lowercase() | ||
|
@@ -552,6 +557,22 @@ class ListErrorsReply( | |
val errors: List<ScriptDiagnostic> | ||
) : MessageContent() | ||
|
||
@Serializable | ||
class SerializationRequest( | ||
val cellId: Int, | ||
val descriptorsState: Map<String, SerializedVariablesState>, | ||
val topLevelDescriptorName: String = "", | ||
val pathToDescriptor: List<String> = emptyList(), | ||
val commId: String = "" | ||
) : MessageContent() | ||
|
||
@Serializable | ||
class SerializationReply( | ||
val cell_id: Int = 1, | ||
val descriptorsState: Map<String, SerializedVariablesState> = emptyMap(), | ||
val comm_id: String = "" | ||
) : MessageContent() | ||
|
||
@Serializable(MessageDataSerializer::class) | ||
data class MessageData( | ||
val header: MessageHeader? = null, | ||
|
Original file line number | Diff line number | Diff line change |
---|---|---|
|
@@ -2,8 +2,11 @@ package org.jetbrains.kotlinx.jupyter | |
|
||
import ch.qos.logback.classic.Level | ||
import kotlinx.serialization.json.Json | ||
import kotlinx.serialization.json.JsonElement | ||
import kotlinx.serialization.json.JsonNull | ||
import kotlinx.serialization.json.JsonObject | ||
import kotlinx.serialization.json.encodeToJsonElement | ||
import kotlinx.serialization.json.jsonObject | ||
import org.jetbrains.annotations.TestOnly | ||
import org.jetbrains.kotlinx.jupyter.LoggingManagement.disableLogging | ||
import org.jetbrains.kotlinx.jupyter.LoggingManagement.mainLoggerLevel | ||
|
@@ -82,7 +85,6 @@ class OkResponseWithMessage( | |
) | ||
) | ||
} | ||
|
||
socket.send( | ||
makeReplyMessage( | ||
requestMsg, | ||
|
@@ -92,7 +94,7 @@ class OkResponseWithMessage( | |
"engine" to Json.encodeToJsonElement(requestMsg.data.header?.session), | ||
"status" to Json.encodeToJsonElement("ok"), | ||
"started" to Json.encodeToJsonElement(startedTime), | ||
"eval_metadata" to Json.encodeToJsonElement(metadata), | ||
"eval_metadata" to Json.encodeToJsonElement(metadata.convertToNullIfEmpty()), | ||
), | ||
content = ExecuteReply( | ||
MessageStatus.OK, | ||
|
@@ -307,6 +309,27 @@ fun JupyterConnection.Socket.shellMessagesHandler(msg: Message, repl: ReplForJup | |
is CommInfoRequest -> { | ||
sendWrapped(msg, makeReplyMessage(msg, MessageType.COMM_INFO_REPLY, content = CommInfoReply(mapOf()))) | ||
} | ||
is CommOpen -> { | ||
if (!content.targetName.equals("kotlin_serialization", ignoreCase = true)) { | ||
send(makeReplyMessage(msg, MessageType.NONE)) | ||
return | ||
} | ||
log.debug("Message type in CommOpen: $msg, ${msg.type}") | ||
val data = content.data ?: return sendWrapped(msg, makeReplyMessage(msg, MessageType.VARIABLES_VIEW_REPLY)) | ||
if (data.isEmpty()) return sendWrapped(msg, makeReplyMessage(msg, MessageType.VARIABLES_VIEW_REPLY)) | ||
log.debug("Message data: $data") | ||
val messageContent = getVariablesDescriptorsFromJson(data) | ||
connection.launchJob { | ||
repl.serializeVariables( | ||
messageContent.topLevelDescriptorName, | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Выглядит так, что распаковка-запаковка делается без особого смысла. Можно передать просто messageContent |
||
messageContent.descriptorsState, | ||
content.commId, | ||
messageContent.pathToDescriptor | ||
) { result -> | ||
sendWrapped(msg, makeReplyMessage(msg, MessageType.COMM_MSG, content = result)) | ||
} | ||
} | ||
} | ||
is CompleteRequest -> { | ||
connection.launchJob { | ||
repl.complete(content.code, content.cursorPos) { result -> | ||
|
@@ -321,6 +344,17 @@ fun JupyterConnection.Socket.shellMessagesHandler(msg: Message, repl: ReplForJup | |
} | ||
} | ||
} | ||
is SerializationRequest -> { | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. Are SerializationRequest's are still used? I thought we switched to comm-s |
||
connection.launchJob { | ||
if (content.topLevelDescriptorName.isNotEmpty()) { | ||
repl.serializeVariables(content.topLevelDescriptorName, content.descriptorsState, commID = content.commId, content.pathToDescriptor) { result -> | ||
sendWrapped(msg, makeReplyMessage(msg, MessageType.VARIABLES_VIEW_REPLY, content = result)) | ||
} | ||
} else { | ||
sendWrapped(msg, makeReplyMessage(msg, MessageType.VARIABLES_VIEW_REPLY, content = null)) | ||
} | ||
} | ||
} | ||
is IsCompleteRequest -> { | ||
// We are in console mode, so switch off all the loggers | ||
if (mainLoggerLevel() != Level.OFF) disableLogging() | ||
|
@@ -513,3 +547,8 @@ fun JupyterConnection.evalWithIO(repl: ReplForJupyter, srcMessage: Message, body | |
KernelStreams.setStreams(false, out, err) | ||
} | ||
} | ||
|
||
fun EvaluatedSnippetMetadata?.convertToNullIfEmpty(): JsonElement? { | ||
val jsonNode = Json.encodeToJsonElement(this) | ||
return if (jsonNode is JsonNull || jsonNode?.jsonObject.isEmpty()) null else jsonNode | ||
There was a problem hiding this comment. Choose a reason for hiding this commentThe reason will be displayed to describe this comment to others. Learn more. return Json.encodeToJsonElement(this).takeIf { it !is JsonNull && it.jsonObject.isNotEmpty() } |
||
} |
There was a problem hiding this comment.
Choose a reason for hiding this comment
The reason will be displayed to describe this comment to others. Learn more.
Это поле не используется нигде. Если что-то используется только в плагине, надо написать тесты на то, как это там используется. Если не используется нигде, удалить