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

Feature/947 v2 entities #958

Merged
merged 20 commits into from
Jun 9, 2015
Merged
Show file tree
Hide file tree
Changes from 11 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
1 change: 1 addition & 0 deletions CHANGES_NEXT_RELEASE
Original file line number Diff line number Diff line change
@@ -1 +1,2 @@
Fix: Fixed a bug about pagination problem due to missing ErrorCode with details in the case of queries with forwarding to context providers. (Issue #945)
Add: New convop "GET /v2/entities" (Issue #947)
Copy link
Contributor

Choose a reason for hiding this comment

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

It was my understanding that in v2 we are no longer using the term convop, aren't we?

Copy link
Member

Choose a reason for hiding this comment

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

Good point... I'd suggest to be neutral and just say:

Add: New operation "GET /v2/entities" (Issue #947)

Copy link
Member Author

Choose a reason for hiding this comment

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

ok, I wasn't aware of that ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in bb66c46

2 changes: 2 additions & 0 deletions CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -158,10 +158,12 @@ if (error EQUAL 0)
ADD_SUBDIRECTORY(src/lib/orionTypes)
Copy link
Member

Choose a reason for hiding this comment

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

CHANGES_NEXT_RELEASE entry is missing.

Copy link
Member Author

Choose a reason for hiding this comment

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

Fixed in 7bb8611

ADD_SUBDIRECTORY(src/lib/ngsi)
ADD_SUBDIRECTORY(src/lib/serviceRoutines)
ADD_SUBDIRECTORY(src/lib/serviceRoutinesV2)
Copy link
Member

Choose a reason for hiding this comment

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

We were discussing on using lib/serviceRoutines/v2 for v2 service routines and (to keep thing homogenoeus) move existing lib/serviceRoutines to lib/serviceRoutines/v1...

Copy link
Member Author

Choose a reason for hiding this comment

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

We were.
I believe it is better adding the version to the directory name instead.
This way all 'lib directories' are on the same level. More order ...

Copy link
Member

Choose a reason for hiding this comment

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

Thus, lib/serviceRoutines should be renamed to lib/serviceRoutinesV1

Copy link
Member Author

Choose a reason for hiding this comment

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

Not really necessary, but sure, no probs

Copy link
Member Author

Choose a reason for hiding this comment

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

I just found a really good reason to not change the name of /lib/serviceRoutines. Let's see what you think ...

The name is used in hundreds of places, when including headers from the directory:

  #include "serviceRoutines/xxx.h"

Copy link
Member Author

Choose a reason for hiding this comment

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

After some discussion on skype, we've decided to leave the names 'as is'. At least for now.

NTC

Copy link
Member

Choose a reason for hiding this comment

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

Any change related with this has been deferred. An issue has been created for this: #959

ADD_SUBDIRECTORY(src/lib/convenience)
ADD_SUBDIRECTORY(src/lib/ngsi9)
ADD_SUBDIRECTORY(src/lib/ngsi10)
ADD_SUBDIRECTORY(src/lib/ngsiNotify)
ADD_SUBDIRECTORY(src/lib/apiTypesV2)
ADD_SUBDIRECTORY(src/lib/parse)
ADD_SUBDIRECTORY(src/lib/jsonParse)
ADD_SUBDIRECTORY(src/lib/xmlParse)
Expand Down
2 changes: 2 additions & 0 deletions src/app/contextBroker/CMakeLists.txt
Original file line number Diff line number Diff line change
Expand Up @@ -31,10 +31,12 @@ SET (STATIC_LIBS
mongoBackend
ngsiNotify
serviceRoutines
serviceRoutinesV2
mongoBackend
rest
jsonParse
xmlParse
apiTypesV2
convenience
ngsi9
ngsi10
Expand Down
18 changes: 18 additions & 0 deletions src/app/contextBroker/contextBroker.cpp
Original file line number Diff line number Diff line change
Expand Up @@ -171,6 +171,9 @@
#include "serviceRoutines/badNgsi9Request.h"
#include "serviceRoutines/badNgsi10Request.h"
#include "serviceRoutines/badRequest.h"

#include "serviceRoutinesV2/getEntities.h"

#include "contextBroker/version.h"

#include "common/string.h"
Expand Down Expand Up @@ -324,6 +327,13 @@ PaArgument paArgs[] =
*/


//
// /v2 conv-ops
Copy link
Member

Choose a reason for hiding this comment

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

Taking into account @jmcanterafonseca comment, maybe it is better just /v2 operations.

Copy link
Member Author

Choose a reason for hiding this comment

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

/v2 operations 

or

/v2 API

?

Copy link
Member

Choose a reason for hiding this comment

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

Both looks fine :)

Copy link
Member Author

Choose a reason for hiding this comment

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

I vote for /v2 API (here and in the name of the definition).

Copy link
Member Author

Choose a reason for hiding this comment

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

/v2 API in bb66c46

Let's decide ...

Copy link
Member

Choose a reason for hiding this comment

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

For my /v2 is fine. If @jmcanterafonseca agress, then NTC.

//
#define ENT EntitiesRequest
#define ENT_COMPS_V2 2, { "v2", "entities" }
Copy link
Contributor

Choose a reason for hiding this comment

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

probably we can define a constant for 'entities' and use it as well in the entryPoints operation as well, WDYT?

Copy link
Member Author

Choose a reason for hiding this comment

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

What do we win with a change like that?
[ Except that the code gets more difficult to read ... ]

Copy link
Member Author

Choose a reason for hiding this comment

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

NTC

#define ENT_COMPS_WORD ""

//
// NGSI9
//
Expand Down Expand Up @@ -569,6 +579,12 @@ PaArgument paArgs[] =



#define CONVENIENCE_OPERATIONS_V2 \
{ "GET", ENT, ENT_COMPS_V2, ENT_COMPS_WORD, getEntities }, \
{ "*", ENT, ENT_COMPS_V2, ENT_COMPS_WORD, badVerbGetOnly }



#define REGISTRY_STANDARD_REQUESTS_V0 \
{ "POST", RCR, RCR_COMPS_V0, RCR_POST_WORD, postRegisterContext }, \
{ "*", RCR, RCR_COMPS_V0, RCR_POST_WORD, badVerbPostOnly }, \
Expand Down Expand Up @@ -899,6 +915,8 @@ PaArgument paArgs[] =
*/
RestService restServiceV[] =
{
CONVENIENCE_OPERATIONS_V2,
Copy link
Contributor

Choose a reason for hiding this comment

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

drop 'CONVENIENCE' ?

Copy link
Member Author

Choose a reason for hiding this comment

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

API_V2 ?

Copy link
Contributor

Choose a reason for hiding this comment

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

SIMPLE_API?

Copy link
Member

Choose a reason for hiding this comment

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

OPERATIONS_V2 ?

Copy link
Member Author

Choose a reason for hiding this comment

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

API_V2 in bb66c46

Let's decide a name ... :-D

Copy link
Member

Choose a reason for hiding this comment

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

For my API_V2 is fine. If @jmcanterafonseca agress, then NTC.


REGISTRY_STANDARD_REQUESTS_V0,
REGISTRY_STANDARD_REQUESTS_V1,
STANDARD_REQUESTS_V0,
Expand Down
44 changes: 44 additions & 0 deletions src/lib/apiTypesV2/CMakeLists.txt
Original file line number Diff line number Diff line change
@@ -0,0 +1,44 @@
# Copyright 2015 Telefonica Investigacion y Desarrollo, S.A.U
#
# This file is part of Orion Context Broker.
#
# Orion Context Broker is free software: you can redistribute it and/or
# modify it under the terms of the GNU Affero General Public License as
# published by the Free Software Foundation, either version 3 of the
# License, or (at your option) any later version.
#
# Orion Context Broker is distributed in the hope that it will be useful,
# but WITHOUT ANY WARRANTY; without even the implied warranty of
# MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero
# General Public License for more details.
#
# You should have received a copy of the GNU Affero General Public License
# along with Orion Context Broker. If not, see http://www.gnu.org/licenses/.
#
# For those usages not covered by this license please contact with
# iot_support at tid dot es

CMAKE_MINIMUM_REQUIRED(VERSION 2.6)

SET (SOURCES
Entity.cpp
EntityVector.cpp
Entities.cpp
)

SET (HEADERS
Copy link
Member

Choose a reason for hiding this comment

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

Looking to the new 3 classes, Entity seems to be very similar to existing EntityId and EntityVector very similar to existing EntityIdVector. I understand that we need a class for the "outest" data structure for the operation (and that class I undertand is Entities), but why don't base its internal content in already existing classes instead of introducing new classes that are basically identical?

Copy link
Member Author

Choose a reason for hiding this comment

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

First of all, they aren't exactly identical.
Separating them into different classes makes the code easier to read and maintain (talking about EntityId vs Entity).
We have a number of vectors in the system and they are pretty much identical, in that I agree 100%.
A nice refactor would be to replace ALL **Vector with ONE class (or at least make them inherit from a base-vector-class.

However, that's not really part of this PR ...

Copy link
Member Author

Choose a reason for hiding this comment

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

Issue #960

Copy link
Member

Choose a reason for hiding this comment

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

Ok, EntityVector is covered by #960. Regarding, Entity I understand that the main difference comparing with EntityId is that the former includes a ContextAttribute vector while the later doesn't include it. Is that correct?

Copy link
Member Author

Choose a reason for hiding this comment

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

Yes, that is the main difference.
But, also, as it is used solely for V2 we can use toJson directly (without checking version number). The code gets cleaner and easier to understand/maintain.

Copy link
Member

Choose a reason for hiding this comment

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

Ok. NTC.

Entity.h
EntityVector.h
Entities.h
)



# Include directories
# -----------------------------------------------------------------
include_directories("${PROJECT_SOURCE_DIR}/src/lib")


# Library declaration
# -----------------------------------------------------------------
ADD_LIBRARY(apiTypesV2 STATIC ${SOURCES} ${HEADERS})
136 changes: 136 additions & 0 deletions src/lib/apiTypesV2/Entities.cpp
Original file line number Diff line number Diff line change
@@ -0,0 +1,136 @@
/*
*
* Copyright 2015 Telefonica Investigacion y Desarrollo, S.A.U
*
* This file is part of Orion Context Broker.
*
* Orion Context Broker is free software: you can redistribute it and/or
* modify it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* Orion Context Broker is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero
* General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with Orion Context Broker. If not, see http://www.gnu.org/licenses/.
*
* For those usages not covered by this license please contact with
* iot_support at tid dot es
*
* Author: Ken Zangelin
*/
#include <string>
#include <vector>

#include "ngsi10/QueryContextResponse.h"
#include "apiTypesV2/Entities.h"



/* ****************************************************************************
*
* Entities::Entities -
*/
Entities::Entities()
{
errorCode.fill(SccOk);
}



/* ****************************************************************************
*
* Entities::~Entities -
*/
Entities::~Entities()
{
release();
}



/* ****************************************************************************
*
* Entities::render -
*
* If no error reported in errorCode, render the vector of entities.
* Otherwise, render the errorCode.
*/
std::string Entities::render(ConnectionInfo* ciP, RequestType requestType)
{
if ((errorCode.code == SccOk) || (errorCode.code == SccNone))
{
return vec.render(ciP, requestType, false);
}

return errorCode.renderV2();
}



/* ****************************************************************************
*
* Entities::check -
*
* NOTE
* The 'check' method is normally only used to check that incoming payload is correct.
* For now (at least), the Entities type is only used as outgoing payload ...
*/
std::string Entities::check(ConnectionInfo* ciP, RequestType requestType)
{
return vec.check(ciP, requestType);
}



/* ****************************************************************************
*
* Entities::present -
*/
void Entities::present(const std::string& indent)
{
LM_F(("%s%d Entities:", indent.c_str(), vec.size()));
vec.present(indent + " ");
}



/* ****************************************************************************
*
* Entities::release -
*/
void Entities::release(void)
{
vec.release();
}



/* ****************************************************************************
*
* Entities::fill -
*/
void Entities::fill(QueryContextResponse* qcrsP)
{
if (qcrsP->errorCode.code != SccOk)
{
errorCode.fill(qcrsP->errorCode);
return;
}

for (unsigned int ix = 0; ix < qcrsP->contextElementResponseVector.size(); ++ix)
{
ContextElement* ceP = &qcrsP->contextElementResponseVector[ix]->contextElement;
Entity* eP = new Entity();

eP->id = ceP->entityId.id;
eP->type = ceP->entityId.type;
eP->isPattern = ceP->entityId.isPattern;
Copy link
Contributor

Choose a reason for hiding this comment

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

do we need isPattern, as we are not going to render it in the Simple API?

Copy link
Member Author

Choose a reason for hiding this comment

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

The idea is for this method to be generic.
In the case of rendering a GET, we don't want isPattern, but in the case of filling the Entity after parsing payload from a POST, isPattern might have to be there.

Copy link
Member Author

Choose a reason for hiding this comment

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

NTC

Copy link
Contributor

Choose a reason for hiding this comment

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

NTC


eP->attributeVector.fill(&ceP->contextAttributeVector);
vec.push_back(eP);
}
}
57 changes: 57 additions & 0 deletions src/lib/apiTypesV2/Entities.h
Original file line number Diff line number Diff line change
@@ -0,0 +1,57 @@
#ifndef SRC_LIB_APITYPESV2_ENTITIES_H_
#define SRC_LIB_APITYPESV2_ENTITIES_H_

/*
*
* Copyright 2015 Telefonica Investigacion y Desarrollo, S.A.U
*
* This file is part of Orion Context Broker.
*
* Orion Context Broker is free software: you can redistribute it and/or
* modify it under the terms of the GNU Affero General Public License as
* published by the Free Software Foundation, either version 3 of the
* License, or (at your option) any later version.
*
* Orion Context Broker is distributed in the hope that it will be useful,
* but WITHOUT ANY WARRANTY; without even the implied warranty of
* MERCHANTABILITY or FITNESS FOR A PARTICULAR PURPOSE. See the GNU Affero
* General Public License for more details.
*
* You should have received a copy of the GNU Affero General Public License
* along with Orion Context Broker. If not, see http://www.gnu.org/licenses/.
*
* For those usages not covered by this license please contact with
* iot_support at tid dot es
*
* Author: Ken Zangelin
*/
#include <string>
#include <vector>

#include "apiTypesV2/EntityVector.h"
#include "ngsi/StatusCode.h"


struct QueryContextResponse;

/* ****************************************************************************
*
* Entities -
*/
class Entities
{
public:
EntityVector vec; // Optional - mandatory if 200-OK
StatusCode errorCode; // Optional - mandatory if not 200-OK

Entities();
~Entities();

std::string render(ConnectionInfo* ciP, RequestType requestType);
std::string check(ConnectionInfo* ciP, RequestType requestType);
void present(const std::string& indent);
void release(void);
void fill(QueryContextResponse* qcrsP);
};

#endif // SRC_LIB_APITYPESV2_ENTITIES_H_
Loading