Skip to content
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

MODELIX-971 allProperties missing in ModelQl on INode #895

Closed
wants to merge 1 commit into from
Closed
Show file tree
Hide file tree
Changes from all commits
Commits
File filter

Filter by extension

Filter by extension

Conversations
Failed to load comments.
Loading
Jump to
Jump to file
Failed to load files.
Loading
Diff view
Diff view
Original file line number Diff line number Diff line change
Expand Up @@ -244,6 +244,7 @@ interface INode {
// <editor-fold desc="flow API">
fun getParentAsFlow(): Flow<INode> = flowOf(parent).filterNotNull()
fun getPropertyValueAsFlow(role: IProperty): Flow<String?> = flowOf(getPropertyValue(role))
fun getAllPropertiesAsFlow(): Flow<Pair<IProperty, String>> = getAllProperties().asFlow()
fun getAllChildrenAsFlow(): Flow<INode> = allChildren.asFlow()
fun getAllReferenceTargetsAsFlow(): Flow<Pair<IReferenceLink, INode>> = getAllReferenceTargets().asFlow()
fun getAllReferenceTargetRefsAsFlow(): Flow<Pair<IReferenceLink, INodeReference>> = getAllReferenceTargetRefs().asFlow()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -15,6 +15,8 @@

package org.modelix.model.api

import kotlinx.serialization.Serializable

/**
* Representation of a property within an [IConcept].
*/
Expand All @@ -24,6 +26,7 @@ interface IProperty : IRole {
}
}

@Serializable
data class PropertyFromName(override val name: String) : RoleFromName(), IProperty {
override val isOptional: Boolean
get() = throw UnsupportedOperationException()
Expand Down
Original file line number Diff line number Diff line change
Expand Up @@ -19,6 +19,7 @@ import io.ktor.server.testing.testApplication
import kotlinx.coroutines.withTimeout
import org.modelix.model.api.IConceptReference
import org.modelix.model.api.INode
import org.modelix.model.api.IProperty
import org.modelix.model.api.PBranch
import org.modelix.model.api.getRootNode
import org.modelix.model.client.IdGenerator
Expand All @@ -42,6 +43,7 @@ import org.modelix.modelql.core.zip
import org.modelix.modelql.server.ModelQLServer
import org.modelix.modelql.untyped.addNewChild
import org.modelix.modelql.untyped.allChildren
import org.modelix.modelql.untyped.allProperties
import org.modelix.modelql.untyped.allReferences
import org.modelix.modelql.untyped.children
import org.modelix.modelql.untyped.descendants
Expand All @@ -64,6 +66,7 @@ class ModelQLClientTest {
branch.runWrite {
val module1 = rootNode.addNewChild("modules", -1, null as IConceptReference?)
module1.setPropertyValue("name", "abc")
module1.setPropertyValue("description", "xyz")
val model1a = module1.addNewChild("models", -1, null as IConceptReference?)
model1a.setPropertyValue("name", "model1a")
}
Expand Down Expand Up @@ -95,6 +98,21 @@ class ModelQLClientTest {
assertEquals(listOf("abc"), result)
}

@Test
fun test_allProperties() = runTest { httpClient ->
val client = ModelQLClient.builder().url("http://localhost/query").httpClient(httpClient).build()
val result: Set<Pair<IProperty, String?>> = client.query { root ->
root.children("modules").allProperties().toSet()
}
assertEquals(
setOf(
IProperty.fromName("name") to "abc",
IProperty.fromName("description") to "xyz",
),
result,
)
}

@Test
fun test_zip() = runTest { httpClient ->
val client = ModelQLClient.builder().url("http://localhost/query").httpClient(httpClient).build()
Expand Down
Original file line number Diff line number Diff line change
@@ -0,0 +1,70 @@
/*
* Copyright (c) 2024.
*
* Licensed under the Apache License, Version 2.0 (the "License");
* you may not use this file except in compliance with the License.
* You may obtain a copy of the License at
*
* http://www.apache.org/licenses/LICENSE-2.0
*
* Unless required by applicable law or agreed to in writing, software
* distributed under the License is distributed on an "AS IS" BASIS,
* WITHOUT WARRANTIES OR CONDITIONS OF ANY KIND, either express or implied.
* See the License for the specific language governing permissions and
* limitations under the License.
*/

package org.modelix.modelql.untyped

import kotlinx.coroutines.ExperimentalCoroutinesApi
import kotlinx.coroutines.flow.flatMapConcat
import kotlinx.serialization.KSerializer
import kotlinx.serialization.SerialName
import kotlinx.serialization.Serializable
import org.modelix.model.api.INode
import org.modelix.model.api.IProperty
import org.modelix.modelql.core.FluxTransformingStep
import org.modelix.modelql.core.IFlowInstantiationContext
import org.modelix.modelql.core.IFluxStep
import org.modelix.modelql.core.IProducingStep
import org.modelix.modelql.core.IStep
import org.modelix.modelql.core.IStepOutput
import org.modelix.modelql.core.QueryDeserializationContext
import org.modelix.modelql.core.QueryGraphDescriptorBuilder
import org.modelix.modelql.core.SerializationContext
import org.modelix.modelql.core.StepDescriptor
import org.modelix.modelql.core.StepFlow
import org.modelix.modelql.core.asStepFlow
import org.modelix.modelql.core.connect
import org.modelix.modelql.core.stepOutputSerializer

class AllPropertiesTraversalStep : FluxTransformingStep<INode, Pair<IProperty, String?>>() {
@OptIn(ExperimentalCoroutinesApi::class)
override fun createFlow(
input: StepFlow<INode>,
context: IFlowInstantiationContext,
): StepFlow<Pair<IProperty, String?>> {
return input.flatMapConcat { it.value.getAllPropertiesAsFlow() }.asStepFlow(this)
}

override fun getOutputSerializer(serializationContext: SerializationContext): KSerializer<out IStepOutput<Pair<IProperty, String?>>> {
return serializationContext.serializer<Pair<IProperty, String?>>().stepOutputSerializer(this)
Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

This only works, because the test case uses a CLTree with named based roles. Not all implementations of IProperty are serializable. I think we need https://issues.modelix.org/issue/MODELIX-976/Cleanup-relation-between-model-and-meta-model first and then return an IPropertyReference (or IPropertyId, not sure yet) instead, that can be resolved on the client side to an IPropertyDefinition if needed.

Copy link
Contributor Author

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

I tested it with a CLTree that uses useRoleIds = true and it works for that case as well.
But your proposed solution would definitely be cleaner.

Copy link
Member

Choose a reason for hiding this comment

The reason will be displayed to describe this comment to others. Learn more.

Because the role is always wrapped by a PropertyFromName, even if it is an ID. This really needs to be cleaned up.

}

override fun createDescriptor(context: QueryGraphDescriptorBuilder): StepDescriptor = AllPropertiesStepDescriptor()

@Serializable
@SerialName("untyped.allProperties")
class AllPropertiesStepDescriptor : StepDescriptor() {
override fun createStep(context: QueryDeserializationContext): IStep {
return AllPropertiesTraversalStep()
}
}

override fun toString(): String {
return """${getProducer()}.allProperties()"""
}
}

fun IProducingStep<INode>.allProperties(): IFluxStep<Pair<IProperty, String?>> =
AllPropertiesTraversalStep().also { connect(it) }
Original file line number Diff line number Diff line change
Expand Up @@ -18,6 +18,8 @@ import kotlinx.serialization.modules.SerializersModule
import kotlinx.serialization.modules.polymorphic
import kotlinx.serialization.modules.subclass
import org.modelix.model.api.INode
import org.modelix.model.api.IProperty
import org.modelix.model.api.PropertyFromName
import org.modelix.model.api.RoleAccessContext
import org.modelix.modelql.core.IFluxQuery
import org.modelix.modelql.core.IFluxStep
Expand All @@ -34,6 +36,7 @@ object UntypedModelQL {
polymorphic(StepDescriptor::class) {
subclass(AddNewChildNodeStep.Descriptor::class)
subclass(AllChildrenTraversalStep.AllChildrenStepDescriptor::class)
subclass(AllPropertiesTraversalStep.AllPropertiesStepDescriptor::class)
subclass(AllReferencesTraversalStep.Descriptor::class)
subclass(ChildrenTraversalStep.ChildrenStepDescriptor::class)
subclass(ConceptReferenceTraversalStep.Descriptor::class)
Expand All @@ -55,6 +58,9 @@ object UntypedModelQL {
subclass(SetReferenceStep.Descriptor::class)
}
polymorphicDefaultSerializer(INode::class) { NodeKSerializer() }
polymorphic(IProperty::class) {
subclass(PropertyFromName::class)
}
}
val json = Json {
serializersModule = UntypedModelQL.serializersModule
Expand Down
Loading